Skip to content

Commit 8dcaded

Browse files
bsboddenbsbodden
committed
refactor: ignore explicit ':' terminator in entity prefixes (resolves gh-386)
-m
1 parent f5e6c1d commit 8dcaded

17 files changed

+377
-75
lines changed

‎redis-om-spring/src/main/java/com/redis/om/spring/RediSearchIndexer.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public void createIndexFor(Class<?> cl) {
117117
}
118118

119119
String entityPrefix = maybeEntityPrefix.orElse(getEntityPrefix(cl));
120+
entityPrefix = entityPrefix.endsWith(":") ? entityPrefix : entityPrefix + ":";
120121
params.prefix(entityPrefix);
121122
addKeySpaceMapping(entityPrefix, cl);
122123
updateTTLSettings(cl, entityPrefix, isDocument, document, allClassFields);

‎redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java‎

Lines changed: 76 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ public RedisEnhancedKeyValueAdapter( //
119119
this.redisOMProperties = redisOMProperties;
120120
}
121121

122+
private static String sanitizeKeyspace(String keyspace) {
123+
return keyspace.endsWith(":") ? keyspace.substring(0, keyspace.length() - 1) : keyspace;
124+
}
125+
122126
/*
123127
* (non-Javadoc)
124128
*
@@ -132,7 +136,8 @@ public Object put(Object id, Object item, String keyspace) {
132136
if (item instanceof RedisData redisData) {
133137
rdo = redisData;
134138
} else {
135-
byte[] redisKey = createKey(keyspace, converter.getConversionService().convert(id, String.class));
139+
byte[] redisKey = createKey(sanitizeKeyspace(keyspace),
140+
converter.getConversionService().convert(id, String.class));
136141
auditor.processEntity(redisKey, item);
137142
featureExtractor.processEntity(item);
138143

@@ -144,7 +149,7 @@ public Object put(Object id, Object item, String keyspace) {
144149
rdo.setId(converter.getConversionService().convert(id, String.class));
145150
}
146151

147-
byte[] objectKey = createKey(rdo.getKeyspace(), rdo.getId());
152+
byte[] objectKey = createKey(sanitizeKeyspace(rdo.getKeyspace()), rdo.getId());
148153
redisOperations.execute((RedisCallback<Boolean>) connection -> connection.keyCommands().del(objectKey) == 0);
149154

150155
redisOperations.executePipelined((RedisCallback<Object>) connection -> {
@@ -173,7 +178,7 @@ public Object put(Object id, Object item, String keyspace) {
173178
public <T> T get(Object id, String keyspace, Class<T> type) {
174179

175180
String stringId = asStringValue(id);
176-
String stringKeyspace = asStringValue(keyspace);
181+
String stringKeyspace = sanitizeKeyspace(asStringValue(keyspace));
177182

178183
byte[] binId = createKey(stringKeyspace, stringId);
179184

@@ -191,30 +196,6 @@ public <T> T get(Object id, String keyspace, Class<T> type) {
191196
return readTimeToLiveIfSet(binId, converter.read(type, data));
192197
}
193198

194-
/*
195-
* (non-Javadoc)
196-
*
197-
* @see
198-
* org.springframework.data.keyvalue.core.AbstractKeyValueAdapter#delete(java.
199-
* lang.Object, java.lang.String, java.lang.Class)
200-
*/
201-
@Override
202-
public <T> T delete(Object id, String keyspace, Class<T> type) {
203-
T o = get(id, keyspace, type);
204-
205-
if (o != null) {
206-
207-
byte[] keyToDelete = createKey(asStringValue(keyspace), asStringValue(id));
208-
209-
redisOperations.execute((RedisCallback<Void>) connection -> {
210-
connection.keyCommands().unlink(keyToDelete);
211-
return null;
212-
});
213-
}
214-
215-
return o;
216-
}
217-
218199
/*
219200
* (non-Javadoc)
220201
*
@@ -291,13 +272,75 @@ public <T> List<T> getAllOf(String keyspace, Class<T> type, long offset, int row
291272
return result;
292273
}
293274

275+
/*
276+
* (non-Javadoc)
277+
*
278+
* @see
279+
* org.springframework.data.keyvalue.core.AbstractKeyValueAdapter#delete(java.
280+
* lang.Object, java.lang.String, java.lang.Class)
281+
*/
282+
@Override
283+
public <T> T delete(Object id, String keyspace, Class<T> type) {
284+
T o = get(id, keyspace, type);
285+
286+
if (o != null) {
287+
288+
byte[] keyToDelete = createKey(sanitizeKeyspace(asStringValue(keyspace)), asStringValue(id));
289+
290+
redisOperations.execute((RedisCallback<Void>) connection -> {
291+
connection.keyCommands().unlink(keyToDelete);
292+
return null;
293+
});
294+
}
295+
296+
return o;
297+
}
298+
299+
/*
300+
* (non-Javadoc)
301+
*
302+
* @see org.springframework.data.keyvalue.core.KeyValueAdapter#count(java.lang.
303+
* String)
304+
*/
305+
@Override
306+
public long count(String keyspace) {
307+
long count = 0L;
308+
Optional<String> maybeIndexName = indexer.getIndexName(keyspace);
309+
if (maybeIndexName.isPresent()) {
310+
SearchOperations<String> search = modulesOperations.opsForSearch(maybeIndexName.get());
311+
// FT.SEARCH index * LIMIT 0 0
312+
Query query = new Query("*");
313+
query.limit(0, 0);
314+
315+
SearchResult result = search.search(query);
316+
317+
count = result.getTotalResults();
318+
}
319+
return count;
320+
}
321+
322+
/*
323+
* (non-Javadoc)
324+
*
325+
* @see
326+
* org.springframework.data.keyvalue.core.KeyValueAdapter#contains(java.lang.
327+
* Object, java.lang.String)
328+
*/
329+
@Override
330+
public boolean contains(Object id, String keyspace) {
331+
Boolean exists = redisOperations
332+
.execute((RedisCallback<Boolean>) connection -> connection.keyCommands().exists(toBytes(getKey(keyspace, id))));
333+
334+
return exists != null && exists;
335+
}
336+
294337
@Override
295338
public void update(PartialUpdate<?> update) {
296339

297340
RedisPersistentEntity<?> entity = this.converter.getMappingContext()
298341
.getRequiredPersistentEntity(update.getTarget());
299342

300-
String keyspace = entity.getKeySpace();
343+
String keyspace = sanitizeKeyspace(entity.getKeySpace());
301344
Object id = update.getId();
302345

303346
byte[] redisKey = createKey(keyspace, converter.getConversionService().convert(id, String.class));
@@ -348,48 +391,6 @@ public void update(PartialUpdate<?> update) {
348391
});
349392
}
350393

351-
/*
352-
* (non-Javadoc)
353-
*
354-
* @see org.springframework.data.keyvalue.core.KeyValueAdapter#count(java.lang.
355-
* String)
356-
*/
357-
@Override
358-
public long count(String keyspace) {
359-
long count = 0L;
360-
Optional<String> maybeIndexName = indexer.getIndexName(keyspace);
361-
if (maybeIndexName.isPresent()) {
362-
SearchOperations<String> search = modulesOperations.opsForSearch(maybeIndexName.get());
363-
// FT.SEARCH index * LIMIT 0 0
364-
Query query = new Query("*");
365-
query.limit(0, 0);
366-
367-
SearchResult result = search.search(query);
368-
369-
count = result.getTotalResults();
370-
}
371-
return count;
372-
}
373-
374-
/*
375-
* (non-Javadoc)
376-
*
377-
* @see
378-
* org.springframework.data.keyvalue.core.KeyValueAdapter#contains(java.lang.
379-
* Object, java.lang.String)
380-
*/
381-
@Override
382-
public boolean contains(Object id, String keyspace) {
383-
Boolean exists = redisOperations
384-
.execute((RedisCallback<Boolean>) connection -> connection.keyCommands().exists(toBytes(getKey(keyspace, id))));
385-
386-
return exists != null && exists;
387-
}
388-
389-
protected String getKey(String keyspace, Object id) {
390-
return String.format("%s:%s", keyspace, id);
391-
}
392-
393394
private RedisUpdateObject fetchDeletePathsFromHash(RedisUpdateObject redisUpdateObject, String path,
394395
RedisConnection connection) {
395396

@@ -500,4 +501,9 @@ void addFieldToRemove(byte[] field) {
500501
fieldsToRemove.add(field);
501502
}
502503
}
504+
505+
protected String getKey(String keyspace, Object id) {
506+
String sanitizedKeyspace = sanitizeKeyspace(keyspace);
507+
return String.format("%s:%s", sanitizedKeyspace, id);
508+
}
503509
}

‎redis-om-spring/src/main/java/com/redis/om/spring/repository/query/Sort.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ public static org.springframework.data.domain.Sort by(Direction direction, Metam
2525
return org.springframework.data.domain.Sort.by(direction, properties);
2626
}
2727

28+
public static org.springframework.data.domain.Sort by(MetamodelField<?, ?>... fields) {
29+
String[] properties = Arrays.stream(fields).map(MetamodelField::getSearchAlias).toArray(String[]::new);
30+
return org.springframework.data.domain.Sort.by(properties);
31+
}
32+
2833
@Serial private static final long serialVersionUID = 7789210988714363618L;
2934

3035
}

‎redis-om-spring/src/main/java/com/redis/om/spring/repository/support/SimpleRedisDocumentRepository.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ private String getKey(Object id) {
246246
}
247247

248248
public byte[] createKey(String keyspace, String id) {
249-
return this.mappingConverter.toBytes(keyspace + ":" + id);
249+
return this.mappingConverter.toBytes(keyspace.endsWith(":") ? keyspace + id : keyspace + ":" + id);
250250
}
251251

252252
private void processReferenceAnnotations(byte[] objectKey, Object entity, Pipeline pipeline) {

‎redis-om-spring/src/main/java/com/redis/om/spring/repository/support/SimpleRedisEnhancedRepository.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public <S extends T> List<S> saveAll(Iterable<S> entities) {
271271
}
272272

273273
public byte[] createKey(String keyspace, String id) {
274-
return this.mappingConverter.toBytes(keyspace + ":" + id);
274+
return this.mappingConverter.toBytes(keyspace.endsWith(":") ? keyspace + id : keyspace + ":" + id);
275275
}
276276

277277
private boolean expires(RedisData data) {

‎redis-om-spring/src/main/java/com/redis/om/spring/search/stream/ReturnFieldsSearchStreamImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public ReturnFieldsSearchStreamImpl( //
5757
this.returning = returning;
5858
this.gson = gson;
5959
this.mappingConverter = mappingConverter;
60-
this.useNoContent = returning.size() == 1 && returning.get(0).getSearchFieldAccessor().getField().isAnnotationPresent(Id.class);
60+
this.useNoContent = returning.size() == 1 && returning.get(0).getSearchFieldAccessor() != null && returning.get(0)
61+
.getSearchFieldAccessor().getField().isAnnotationPresent(Id.class);
6162
this.isDocument = isDocument;
6263
}
6364

‎redis-om-spring/src/test/java/com/redis/om/spring/annotations/document/BasicRedisDocumentMappingTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.springframework.data.domain.PageRequest;
1212
import org.springframework.data.domain.Pageable;
1313
import org.springframework.data.domain.Sort;
14+
import org.springframework.data.domain.Sort.Direction;
1415
import org.springframework.data.geo.Distance;
1516
import org.springframework.data.geo.Point;
1617
import org.springframework.data.redis.connection.RedisGeoCommands;
@@ -715,4 +716,40 @@ void testFindAllWithSorting() {
715716
.collect(Collectors.toList());
716717
assertThat(Ordering.<String>natural().isOrdered(companyNames)).isTrue();
717718
}
719+
720+
@Test
721+
void testFindAllWithSortingById() {
722+
final List<Company> bunchOfCompanies = new ArrayList<>();
723+
IntStream.range(1, 25).forEach(i -> {
724+
Company c = Company.of("Company" + i, 2022, LocalDate.of(2021, 5, 1), new Point(-122.066540, 37.377690),
725+
"company" + i + "@inc.com");
726+
bunchOfCompanies.add(c);
727+
});
728+
repository.saveAll(bunchOfCompanies);
729+
730+
Iterable<Company> result = repository.findAll(Sort.by("id").ascending());
731+
732+
List<String> ids = StreamSupport.stream(result.spliterator(), false)
733+
.map(Company::getId)
734+
.toList();
735+
assertThat(Ordering.<String>natural().isOrdered(ids)).isTrue();
736+
}
737+
738+
@Test
739+
void testFindAllWithSortingByIdUsingMetamodel() {
740+
final List<Company> bunchOfCompanies = new ArrayList<>();
741+
IntStream.range(1, 25).forEach(i -> {
742+
Company c = Company.of("Company" + i, 2022, LocalDate.of(2021, 5, 1), new Point(-122.066540, 37.377690),
743+
"company" + i + "@inc.com");
744+
bunchOfCompanies.add(c);
745+
});
746+
repository.saveAll(bunchOfCompanies);
747+
748+
Iterable<Company> result = repository.findAll(com.redis.om.spring.repository.query.Sort.by(Company$.ID).ascending());
749+
750+
List<String> ids = StreamSupport.stream(result.spliterator(), false)
751+
.map(Company::getId)
752+
.toList();
753+
assertThat(Ordering.<String>natural().isOrdered(ids)).isTrue();
754+
}
718755
}

‎redis-om-spring/src/test/java/com/redis/om/spring/annotations/document/ExplicitPrefixesDocumentTest.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package com.redis.om.spring.annotations.document;
2+
3+
import com.redis.om.spring.AbstractBaseDocumentTest;
4+
import com.redis.om.spring.annotations.document.fixtures.*;
5+
import com.redis.om.spring.search.stream.EntityStream;
6+
import com.redis.om.spring.search.stream.SearchStream;
7+
import org.junit.jupiter.api.BeforeEach;
8+
import org.junit.jupiter.api.Test;
9+
import org.springframework.beans.factory.annotation.Autowired;
10+
11+
import java.util.Optional;
12+
import java.util.Set;
13+
import java.util.stream.Collectors;
14+
15+
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.junit.jupiter.api.Assertions.assertAll;
17+
18+
public class ExplicitPrefixesDocumentTest extends AbstractBaseDocumentTest {
19+
@Autowired
20+
CountryRepository countryRepository;
21+
22+
@Autowired
23+
DocWithColonInPrefixRepository docWithColonInPrefixRepository;
24+
25+
@Autowired
26+
EntityStream entityStream;
27+
28+
@BeforeEach
29+
void prepare() {
30+
countryRepository.deleteAll();
31+
32+
var countriesInBulk = Set.of( //
33+
Country.of("Mexico"), Country.of("Canada"), //
34+
Country.of("Panama"), Country.of("Venezuela") //
35+
);
36+
countryRepository.saveAll(countriesInBulk);
37+
countryRepository.save(Country.of("USA"));
38+
39+
docWithColonInPrefixRepository.deleteAll();
40+
var countriesInBulk2 = Set.of( //
41+
DocWithColonInPrefix.of("Mexico"), DocWithColonInPrefix.of("Canada"), //
42+
DocWithColonInPrefix.of("Panama"), DocWithColonInPrefix.of("Venezuela") //
43+
);
44+
docWithColonInPrefixRepository.saveAll(countriesInBulk2);
45+
docWithColonInPrefixRepository.save(DocWithColonInPrefix.of("USA"));
46+
}
47+
48+
@Test
49+
void testKeyGenerationWithCustomPrefix() {
50+
try (SearchStream<Country> stream = entityStream.of(Country.class)) {
51+
var countries = stream.map(Country$._KEY).collect(Collectors.toSet());
52+
assertThat(countries).containsExactlyInAnyOrder( //
53+
"country:Mexico", "country:Canada", //
54+
"country:Panama", "country:Venezuela", //
55+
"country:USA" //
56+
);
57+
}
58+
}
59+
60+
@Test
61+
void testKeyGenerationWithCustomPrefixWithColon() {
62+
try (SearchStream<DocWithColonInPrefix> stream = entityStream.of(DocWithColonInPrefix.class)) {
63+
var countries = stream.map(DocWithColonInPrefix$._KEY).collect(Collectors.toSet());
64+
assertThat(countries).containsExactlyInAnyOrder( //
65+
"dwcip:Mexico", "dwcip:Canada", //
66+
"dwcip:Panama", "dwcip:Venezuela", //
67+
"dwcip:USA" //
68+
);
69+
}
70+
}
71+
72+
@Test void testFindByIdForCustomPrefixWithColon() {
73+
Optional<DocWithColonInPrefix> maybePanama = docWithColonInPrefixRepository.findById("Panama");
74+
assertThat(maybePanama).isPresent();
75+
}
76+
}

‎redis-om-spring/src/test/java/com/redis/om/spring/annotations/document/fixtures/DocWithColonInPrefix.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.redis.om.spring.annotations.document.fixtures;
2+
3+
import com.redis.om.spring.annotations.Document;
4+
import lombok.Data;
5+
import lombok.NonNull;
6+
import lombok.RequiredArgsConstructor;
7+
import org.springframework.data.annotation.Id;
8+
9+
@Data
10+
@RequiredArgsConstructor(staticName = "of")
11+
@Document("dwcip:")
12+
public class DocWithColonInPrefix {
13+
@Id
14+
@NonNull
15+
private String id;
16+
}

0 commit comments

Comments
 (0)