Skip to content

Commit aae38a1

Browse files
bsboddenbsbodden
committed
fix: support findByIdIn queries on auto-indexed @id fields (#699)
The extractQueryFields method in RediSearchQuery and RedisEnhancedQuery only checked for explicit index annotations (@NumericIndexed, @TagIndexed, etc.) but did not handle @id fields that are automatically indexed by RediSearchIndexer. This caused findByIdIn queries to return incorrect results (all records or no filtering) when the @id field had no explicit index annotation. The fix adds handling for @id fields in extractQueryFields(): - Number types (Long, Integer, etc.) -> NUMERIC field type - String, UUID, Ulid, etc. -> TAG field type This matches the auto-indexing behavior in RediSearchIndexer where @id fields are automatically indexed based on their Java type.
1 parent 71281a2 commit aae38a1

File tree

5 files changed

+206
-0
lines changed

5 files changed

+206
-0
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.apache.commons.logging.Log;
1818
import org.apache.commons.logging.LogFactory;
1919
import org.springframework.beans.factory.annotation.Value;
20+
import org.springframework.data.annotation.Id;
2021
import org.springframework.data.core.PropertyPath;
2122
import org.springframework.data.domain.PageImpl;
2223
import org.springframework.data.domain.Pageable;
@@ -628,6 +629,20 @@ private List<Pair<String, QueryClause>> extractQueryFields(Class<?> type, Part p
628629
NumericIndexed indexAnnotation = field.getAnnotation(NumericIndexed.class);
629630
String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias();
630631
qf.add(Pair.of(actualKey, QueryClause.get(FieldType.NUMERIC, part.getType())));
632+
} else if (field.isAnnotationPresent(Id.class)) {
633+
// Handle @Id fields that are auto-indexed (without explicit index annotation)
634+
// @Id fields are automatically indexed as NUMERIC for Number types, TAG for String/others
635+
Class<?> fieldType = ClassUtils.resolvePrimitiveIfNecessary(field.getType());
636+
FieldType redisFieldType = getRedisFieldType(fieldType);
637+
638+
if (redisFieldType == FieldType.NUMERIC) {
639+
qf.add(Pair.of(key, QueryClause.get(FieldType.NUMERIC, part.getType())));
640+
} else if (redisFieldType == FieldType.TAG) {
641+
qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType())));
642+
} else {
643+
// Fallback to TAG for other types (String, UUID, etc.)
644+
qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType())));
645+
}
631646
} else if (field.isAnnotationPresent(Indexed.class)) {
632647
Indexed indexAnnotation = field.getAnnotation(Indexed.class);
633648
String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias();

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.apache.commons.logging.Log;
1515
import org.apache.commons.logging.LogFactory;
1616
import org.springframework.beans.factory.annotation.Value;
17+
import org.springframework.data.annotation.Id;
1718
import org.springframework.data.core.PropertyPath;
1819
import org.springframework.data.domain.PageImpl;
1920
import org.springframework.data.domain.Pageable;
@@ -399,6 +400,17 @@ private List<Pair<String, QueryClause>> extractQueryFields(Class<?> type, Part p
399400
NumericIndexed indexAnnotation = field.getAnnotation(NumericIndexed.class);
400401
String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias();
401402
qf.add(Pair.of(actualKey, QueryClause.get(FieldType.NUMERIC, part.getType())));
403+
} else if (field.isAnnotationPresent(Id.class)) {
404+
// Handle @Id fields that are auto-indexed (without explicit index annotation)
405+
// @Id fields are automatically indexed as NUMERIC for Number types, TAG for String/others
406+
Class<?> fieldType = ClassUtils.resolvePrimitiveIfNecessary(field.getType());
407+
408+
if (Number.class.isAssignableFrom(fieldType)) {
409+
qf.add(Pair.of(key, QueryClause.get(FieldType.NUMERIC, part.getType())));
410+
} else {
411+
// Fallback to TAG for String, UUID, Ulid, etc.
412+
qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType())));
413+
}
402414
} else if (field.isAnnotationPresent(Indexed.class)) {
403415
Indexed indexAnnotation = field.getAnnotation(Indexed.class);
404416
String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias();

‎tests/src/test/java/com/redis/om/spring/fixtures/document/model/AutoIndexedIdEntity.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.redis.om.spring.fixtures.document.model;
2+
3+
import org.springframework.data.annotation.Id;
4+
5+
import com.redis.om.spring.annotations.Document;
6+
import com.redis.om.spring.annotations.TagIndexed;
7+
8+
import lombok.AccessLevel;
9+
import lombok.AllArgsConstructor;
10+
import lombok.Data;
11+
import lombok.NoArgsConstructor;
12+
import lombok.RequiredArgsConstructor;
13+
import lombok.NonNull;
14+
15+
/**
16+
* Test entity with only @Id annotation (no @NumericIndexed).
17+
* This tests that auto-indexed ID fields work correctly with findByIdIn queries.
18+
*/
19+
@Data
20+
@NoArgsConstructor
21+
@RequiredArgsConstructor
22+
@AllArgsConstructor(access = AccessLevel.PROTECTED)
23+
@Document
24+
public class AutoIndexedIdEntity {
25+
@Id // Only @Id, no @NumericIndexed - should be auto-indexed as NUMERIC for Long type
26+
private Long id;
27+
28+
@NonNull
29+
@TagIndexed
30+
private String name;
31+
32+
@NonNull
33+
private Integer value;
34+
}

‎tests/src/test/java/com/redis/om/spring/fixtures/document/repository/AutoIndexedIdEntityRepository.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.redis.om.spring.fixtures.document.repository;
2+
3+
import java.util.Collection;
4+
import java.util.List;
5+
6+
import com.redis.om.spring.fixtures.document.model.AutoIndexedIdEntity;
7+
import com.redis.om.spring.repository.RedisDocumentRepository;
8+
9+
/**
10+
* Repository for testing auto-indexed ID fields with findByIdIn queries.
11+
*/
12+
public interface AutoIndexedIdEntityRepository extends RedisDocumentRepository<AutoIndexedIdEntity, Long> {
13+
// Test querying by auto-indexed numeric ID (no explicit @NumericIndexed)
14+
List<AutoIndexedIdEntity> findByIdIn(Collection<Long> ids);
15+
List<AutoIndexedIdEntity> findByIdNotIn(Collection<Long> ids);
16+
17+
// Combined queries
18+
List<AutoIndexedIdEntity> findByIdInAndName(Collection<Long> ids, String name);
19+
}

‎tests/src/test/java/com/redis/om/spring/indexing/AutoIndexedIdFieldTest.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package com.redis.om.spring.indexing;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.junit.jupiter.api.Assertions.*;
5+
6+
import java.util.Arrays;
7+
import java.util.List;
8+
9+
import org.junit.jupiter.api.AfterEach;
10+
import org.junit.jupiter.api.BeforeEach;
11+
import org.junit.jupiter.api.Test;
12+
import org.springframework.beans.factory.annotation.Autowired;
13+
14+
import com.redis.om.spring.AbstractBaseDocumentTest;
15+
import com.redis.om.spring.fixtures.document.model.AutoIndexedIdEntity;
16+
import com.redis.om.spring.fixtures.document.repository.AutoIndexedIdEntityRepository;
17+
18+
/**
19+
* Tests for auto-indexed @Id fields (without explicit @NumericIndexed annotation).
20+
*
21+
* This tests GitHub issue #699 - findByXXXIn should work with auto-indexed ID fields.
22+
* The ID field with Long type should be automatically indexed as NUMERIC,
23+
* and findByIdIn queries should work without requiring explicit @NumericIndexed.
24+
*/
25+
class AutoIndexedIdFieldTest extends AbstractBaseDocumentTest {
26+
27+
@Autowired
28+
AutoIndexedIdEntityRepository repository;
29+
30+
private AutoIndexedIdEntity entity1;
31+
private AutoIndexedIdEntity entity2;
32+
private AutoIndexedIdEntity entity3;
33+
34+
@BeforeEach
35+
void setUp() {
36+
// Create test entities with numeric IDs (no @NumericIndexed on ID field)
37+
entity1 = new AutoIndexedIdEntity("Entity One", 100);
38+
entity1.setId(1001L);
39+
40+
entity2 = new AutoIndexedIdEntity("Entity Two", 200);
41+
entity2.setId(1002L);
42+
43+
entity3 = new AutoIndexedIdEntity("Entity Three", 100);
44+
entity3.setId(1003L);
45+
46+
repository.saveAll(Arrays.asList(entity1, entity2, entity3));
47+
}
48+
49+
@AfterEach
50+
void tearDown() {
51+
repository.deleteAll();
52+
}
53+
54+
@Test
55+
void testAutoIndexedIdFieldDoesNotCauseDuplicateSchema() {
56+
// Verify that having just @Id on a Long field doesn't cause issues
57+
// The entities should be saved and queryable without issues
58+
59+
// Verify entities were saved
60+
assertThat(repository.count()).isEqualTo(3);
61+
62+
// Verify we can query by ID using standard findById
63+
assertThat(repository.findById(1001L)).isPresent();
64+
assertThat(repository.findById(1002L)).isPresent();
65+
assertThat(repository.findById(1003L)).isPresent();
66+
}
67+
68+
@Test
69+
void testFindByIdInWithAutoIndexedId() {
70+
// This is the key test for issue #699
71+
// findByIdIn should work with auto-indexed ID fields (no @NumericIndexed needed)
72+
List<Long> ids = Arrays.asList(1001L, 1003L);
73+
List<AutoIndexedIdEntity> results = repository.findByIdIn(ids);
74+
75+
assertThat(results).hasSize(2);
76+
assertThat(results).contains(entity1, entity3);
77+
assertThat(results).doesNotContain(entity2);
78+
}
79+
80+
@Test
81+
void testFindByIdNotInWithAutoIndexedId() {
82+
List<Long> ids = Arrays.asList(1001L, 1002L);
83+
List<AutoIndexedIdEntity> results = repository.findByIdNotIn(ids);
84+
85+
assertThat(results).hasSize(1);
86+
assertThat(results).contains(entity3);
87+
assertThat(results).doesNotContain(entity1, entity2);
88+
}
89+
90+
@Test
91+
void testFindByIdInAndNameWithAutoIndexedId() {
92+
List<Long> ids = Arrays.asList(1001L, 1002L, 1003L);
93+
List<AutoIndexedIdEntity> results = repository.findByIdInAndName(ids, "Entity Two");
94+
95+
assertThat(results).hasSize(1);
96+
assertThat(results).contains(entity2);
97+
}
98+
99+
@Test
100+
void testFindByIdInWithEmptyCollection() {
101+
// Edge case: empty collection should return empty results
102+
List<Long> ids = List.of();
103+
List<AutoIndexedIdEntity> results = repository.findByIdIn(ids);
104+
105+
assertThat(results).isEmpty();
106+
}
107+
108+
@Test
109+
void testFindByIdInWithSingleId() {
110+
// Single ID in collection
111+
List<Long> ids = List.of(1002L);
112+
List<AutoIndexedIdEntity> results = repository.findByIdIn(ids);
113+
114+
assertThat(results).hasSize(1);
115+
assertThat(results).contains(entity2);
116+
}
117+
118+
@Test
119+
void testFindByIdInWithNonExistentIds() {
120+
// IDs that don't exist
121+
List<Long> ids = Arrays.asList(9999L, 8888L);
122+
List<AutoIndexedIdEntity> results = repository.findByIdIn(ids);
123+
124+
assertThat(results).isEmpty();
125+
}
126+
}

0 commit comments

Comments
 (0)