From 1664c1a600bc7adb104389748d105d30bce1843b Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Sun, 7 Dec 2025 22:20:05 -0700 Subject: [PATCH] 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. --- .../repository/query/RediSearchQuery.java | 15 +++ .../repository/query/RedisEnhancedQuery.java | 12 ++ .../document/model/AutoIndexedIdEntity.java | 34 +++++ .../AutoIndexedIdEntityRepository.java | 19 +++ .../indexing/AutoIndexedIdFieldTest.java | 126 ++++++++++++++++++ 5 files changed, 206 insertions(+) create mode 100644 tests/src/test/java/com/redis/om/spring/fixtures/document/model/AutoIndexedIdEntity.java create mode 100644 tests/src/test/java/com/redis/om/spring/fixtures/document/repository/AutoIndexedIdEntityRepository.java create mode 100644 tests/src/test/java/com/redis/om/spring/indexing/AutoIndexedIdFieldTest.java diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RediSearchQuery.java b/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RediSearchQuery.java index 2177267a..854d0a95 100644 --- a/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RediSearchQuery.java +++ b/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RediSearchQuery.java @@ -17,6 +17,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.data.annotation.Id; import org.springframework.data.core.PropertyPath; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; @@ -628,6 +629,20 @@ private List> extractQueryFields(Class type, Part p NumericIndexed indexAnnotation = field.getAnnotation(NumericIndexed.class); String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias(); qf.add(Pair.of(actualKey, QueryClause.get(FieldType.NUMERIC, part.getType()))); + } else if (field.isAnnotationPresent(Id.class)) { + // Handle @Id fields that are auto-indexed (without explicit index annotation) + // @Id fields are automatically indexed as NUMERIC for Number types, TAG for String/others + Class fieldType = ClassUtils.resolvePrimitiveIfNecessary(field.getType()); + FieldType redisFieldType = getRedisFieldType(fieldType); + + if (redisFieldType == FieldType.NUMERIC) { + qf.add(Pair.of(key, QueryClause.get(FieldType.NUMERIC, part.getType()))); + } else if (redisFieldType == FieldType.TAG) { + qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType()))); + } else { + // Fallback to TAG for other types (String, UUID, etc.) + qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType()))); + } } else if (field.isAnnotationPresent(Indexed.class)) { Indexed indexAnnotation = field.getAnnotation(Indexed.class); String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias(); diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RedisEnhancedQuery.java b/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RedisEnhancedQuery.java index 3ed17ce0..b40cec33 100644 --- a/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RedisEnhancedQuery.java +++ b/redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RedisEnhancedQuery.java @@ -14,6 +14,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.data.annotation.Id; import org.springframework.data.core.PropertyPath; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; @@ -399,6 +400,17 @@ private List> extractQueryFields(Class type, Part p NumericIndexed indexAnnotation = field.getAnnotation(NumericIndexed.class); String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias(); qf.add(Pair.of(actualKey, QueryClause.get(FieldType.NUMERIC, part.getType()))); + } else if (field.isAnnotationPresent(Id.class)) { + // Handle @Id fields that are auto-indexed (without explicit index annotation) + // @Id fields are automatically indexed as NUMERIC for Number types, TAG for String/others + Class fieldType = ClassUtils.resolvePrimitiveIfNecessary(field.getType()); + + if (Number.class.isAssignableFrom(fieldType)) { + qf.add(Pair.of(key, QueryClause.get(FieldType.NUMERIC, part.getType()))); + } else { + // Fallback to TAG for String, UUID, Ulid, etc. + qf.add(Pair.of(key, QueryClause.get(FieldType.TAG, part.getType()))); + } } else if (field.isAnnotationPresent(Indexed.class)) { Indexed indexAnnotation = field.getAnnotation(Indexed.class); String actualKey = indexAnnotation.alias().isBlank() ? key : indexAnnotation.alias(); diff --git a/tests/src/test/java/com/redis/om/spring/fixtures/document/model/AutoIndexedIdEntity.java b/tests/src/test/java/com/redis/om/spring/fixtures/document/model/AutoIndexedIdEntity.java new file mode 100644 index 00000000..b1baddaa --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/fixtures/document/model/AutoIndexedIdEntity.java @@ -0,0 +1,34 @@ +package com.redis.om.spring.fixtures.document.model; + +import org.springframework.data.annotation.Id; + +import com.redis.om.spring.annotations.Document; +import com.redis.om.spring.annotations.TagIndexed; + +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; +import lombok.NonNull; + +/** + * Test entity with only @Id annotation (no @NumericIndexed). + * This tests that auto-indexed ID fields work correctly with findByIdIn queries. + */ +@Data +@NoArgsConstructor +@RequiredArgsConstructor +@AllArgsConstructor(access = AccessLevel.PROTECTED) +@Document +public class AutoIndexedIdEntity { + @Id // Only @Id, no @NumericIndexed - should be auto-indexed as NUMERIC for Long type + private Long id; + + @NonNull + @TagIndexed + private String name; + + @NonNull + private Integer value; +} diff --git a/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/AutoIndexedIdEntityRepository.java b/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/AutoIndexedIdEntityRepository.java new file mode 100644 index 00000000..aa391854 --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/AutoIndexedIdEntityRepository.java @@ -0,0 +1,19 @@ +package com.redis.om.spring.fixtures.document.repository; + +import java.util.Collection; +import java.util.List; + +import com.redis.om.spring.fixtures.document.model.AutoIndexedIdEntity; +import com.redis.om.spring.repository.RedisDocumentRepository; + +/** + * Repository for testing auto-indexed ID fields with findByIdIn queries. + */ +public interface AutoIndexedIdEntityRepository extends RedisDocumentRepository { + // Test querying by auto-indexed numeric ID (no explicit @NumericIndexed) + List findByIdIn(Collection ids); + List findByIdNotIn(Collection ids); + + // Combined queries + List findByIdInAndName(Collection ids, String name); +} diff --git a/tests/src/test/java/com/redis/om/spring/indexing/AutoIndexedIdFieldTest.java b/tests/src/test/java/com/redis/om/spring/indexing/AutoIndexedIdFieldTest.java new file mode 100644 index 00000000..dd4bda8e --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/indexing/AutoIndexedIdFieldTest.java @@ -0,0 +1,126 @@ +package com.redis.om.spring.indexing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Arrays; +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import com.redis.om.spring.AbstractBaseDocumentTest; +import com.redis.om.spring.fixtures.document.model.AutoIndexedIdEntity; +import com.redis.om.spring.fixtures.document.repository.AutoIndexedIdEntityRepository; + +/** + * Tests for auto-indexed @Id fields (without explicit @NumericIndexed annotation). + * + * This tests GitHub issue #699 - findByXXXIn should work with auto-indexed ID fields. + * The ID field with Long type should be automatically indexed as NUMERIC, + * and findByIdIn queries should work without requiring explicit @NumericIndexed. + */ +class AutoIndexedIdFieldTest extends AbstractBaseDocumentTest { + + @Autowired + AutoIndexedIdEntityRepository repository; + + private AutoIndexedIdEntity entity1; + private AutoIndexedIdEntity entity2; + private AutoIndexedIdEntity entity3; + + @BeforeEach + void setUp() { + // Create test entities with numeric IDs (no @NumericIndexed on ID field) + entity1 = new AutoIndexedIdEntity("Entity One", 100); + entity1.setId(1001L); + + entity2 = new AutoIndexedIdEntity("Entity Two", 200); + entity2.setId(1002L); + + entity3 = new AutoIndexedIdEntity("Entity Three", 100); + entity3.setId(1003L); + + repository.saveAll(Arrays.asList(entity1, entity2, entity3)); + } + + @AfterEach + void tearDown() { + repository.deleteAll(); + } + + @Test + void testAutoIndexedIdFieldDoesNotCauseDuplicateSchema() { + // Verify that having just @Id on a Long field doesn't cause issues + // The entities should be saved and queryable without issues + + // Verify entities were saved + assertThat(repository.count()).isEqualTo(3); + + // Verify we can query by ID using standard findById + assertThat(repository.findById(1001L)).isPresent(); + assertThat(repository.findById(1002L)).isPresent(); + assertThat(repository.findById(1003L)).isPresent(); + } + + @Test + void testFindByIdInWithAutoIndexedId() { + // This is the key test for issue #699 + // findByIdIn should work with auto-indexed ID fields (no @NumericIndexed needed) + List ids = Arrays.asList(1001L, 1003L); + List results = repository.findByIdIn(ids); + + assertThat(results).hasSize(2); + assertThat(results).contains(entity1, entity3); + assertThat(results).doesNotContain(entity2); + } + + @Test + void testFindByIdNotInWithAutoIndexedId() { + List ids = Arrays.asList(1001L, 1002L); + List results = repository.findByIdNotIn(ids); + + assertThat(results).hasSize(1); + assertThat(results).contains(entity3); + assertThat(results).doesNotContain(entity1, entity2); + } + + @Test + void testFindByIdInAndNameWithAutoIndexedId() { + List ids = Arrays.asList(1001L, 1002L, 1003L); + List results = repository.findByIdInAndName(ids, "Entity Two"); + + assertThat(results).hasSize(1); + assertThat(results).contains(entity2); + } + + @Test + void testFindByIdInWithEmptyCollection() { + // Edge case: empty collection should return empty results + List ids = List.of(); + List results = repository.findByIdIn(ids); + + assertThat(results).isEmpty(); + } + + @Test + void testFindByIdInWithSingleId() { + // Single ID in collection + List ids = List.of(1002L); + List results = repository.findByIdIn(ids); + + assertThat(results).hasSize(1); + assertThat(results).contains(entity2); + } + + @Test + void testFindByIdInWithNonExistentIds() { + // IDs that don't exist + List ids = Arrays.asList(9999L, 8888L); + List results = repository.findByIdIn(ids); + + assertThat(results).isEmpty(); + } +}