From 2fa3795379f83ea58ba290b04d4307ccafab7c59 Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Tue, 16 Sep 2025 13:37:05 -0700 Subject: [PATCH] fix: handle predicate negation and TAG field notEq operations in EntityStream - Add NegatedPredicate class to properly handle SearchFieldPredicate negation - Override negate() in BaseAbstractPredicate to preserve SearchFieldPredicate type - Fix ClassCastException in NotEqualPredicate.getValues() for single values - Add comprehensive tests for isMissing().negate() and notEq("") operations - Document missing field queries in EntityStream documentation The issue occurred when isMissing().negate() was called, as it used Java's default Predicate.negate() which wrapped the predicate in a generic negation, losing the SearchFieldPredicate type needed for proper query generation. This caused the EntityStream to generate wildcard "*" queries instead of proper "-ismissing(@field)" queries. Additionally fixed a ClassCastException in NotEqualPredicate when using notEq("") with single string values by properly checking if the value is already iterable before casting. --- .../modules/ROOT/pages/entity-streams.adoc | 46 +++++ .../predicates/BaseAbstractPredicate.java | 5 + .../stream/predicates/NegatedPredicate.java | 81 +++++++++ .../predicates/tag/NotEqualPredicate.java | 11 +- .../fixtures/document/model/RxDocument.java | 29 ++++ .../repository/RxDocumentRepository.java | 9 + .../stream/EntityStreamMissingFieldTest.java | 158 ++++++++++++++++++ 7 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/NegatedPredicate.java create mode 100644 tests/src/test/java/com/redis/om/spring/fixtures/document/model/RxDocument.java create mode 100644 tests/src/test/java/com/redis/om/spring/fixtures/document/repository/RxDocumentRepository.java create mode 100644 tests/src/test/java/com/redis/om/spring/search/stream/EntityStreamMissingFieldTest.java diff --git a/docs/content/modules/ROOT/pages/entity-streams.adoc b/docs/content/modules/ROOT/pages/entity-streams.adoc index e19e860d..da08a650 100644 --- a/docs/content/modules/ROOT/pages/entity-streams.adoc +++ b/docs/content/modules/ROOT/pages/entity-streams.adoc @@ -227,6 +227,52 @@ List exactLocation = entityStream.of(Company.class) .collect(Collectors.toList()); ---- +=== Missing Field Queries + +For fields with `indexMissing = true` and `indexEmpty = true` (typically TAG fields), you can filter for missing or present values: + +[source,java] +---- +@Document +public class RxDocument { + @Id + private String id; + + @Indexed + private String rxNumber; + + @Indexed(indexEmpty = true, indexMissing = true) + private String lock; + + @Indexed + private String status; +} + +// Find documents where lock field is missing (null or not set) +List unlockedDocs = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing()) + .collect(Collectors.toList()); + +// Find documents where lock field exists (not null, may be empty string) +List lockedDocs = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing().negate()) + .collect(Collectors.toList()); + +// Combine with other conditions +List activeAndLocked = entityStream.of(RxDocument.class) + .filter(RxDocument$.STATUS.eq("ACTIVE")) + .filter(RxDocument$.LOCK.isMissing().negate()) + .collect(Collectors.toList()); + +// Find non-empty and non-missing values +List hasContent = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing().negate()) // Not missing + .filter(RxDocument$.LOCK.notEq("")) // Not empty string + .collect(Collectors.toList()); +---- + +NOTE: The `isMissing()` predicate checks if a field value is null or not set in Redis. The `.negate()` method inverts this check to find documents where the field exists. This is particularly useful for TAG fields with `indexEmpty = true` and `indexMissing = true` settings. + === Tag and Collection Queries [source,java] diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/BaseAbstractPredicate.java b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/BaseAbstractPredicate.java index 3db723a5..973c0145 100644 --- a/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/BaseAbstractPredicate.java +++ b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/BaseAbstractPredicate.java @@ -162,4 +162,9 @@ public SearchFieldAccessor getSearchFieldAccessor() { return field; } + @Override + public SearchFieldPredicate negate() { + return new NegatedPredicate<>(this); + } + } \ No newline at end of file diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/NegatedPredicate.java b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/NegatedPredicate.java new file mode 100644 index 00000000..2b45ff0b --- /dev/null +++ b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/NegatedPredicate.java @@ -0,0 +1,81 @@ +package com.redis.om.spring.search.stream.predicates; + +import java.lang.reflect.Field; + +import redis.clients.jedis.search.Schema.FieldType; +import redis.clients.jedis.search.querybuilder.Node; + +/** + * A predicate that negates another search field predicate. + * This class wraps a {@link SearchFieldPredicate} and applies logical negation to it + * in Redis search queries. + * + *

This predicate is used when calling {@code negate()} on a search predicate, + * ensuring that the negation is properly handled in the Redis search query.

+ * + * @param the entity type being filtered + * @param the field type of the predicate + */ +public class NegatedPredicate implements SearchFieldPredicate { + + private final SearchFieldPredicate predicate; + + /** + * Creates a new negated predicate. + * + * @param predicate the predicate to negate + */ + public NegatedPredicate(SearchFieldPredicate predicate) { + this.predicate = predicate; + } + + @Override + public boolean test(T t) { + return !predicate.test(t); + } + + @Override + public FieldType getSearchFieldType() { + return predicate.getSearchFieldType(); + } + + @Override + public Field getField() { + return predicate.getField(); + } + + @Override + public String getSearchAlias() { + return predicate.getSearchAlias(); + } + + @Override + public Node apply(Node root) { + // Get the node from the wrapped predicate + Node predicateNode = predicate.apply(root); + + // If the predicate generates a custom query string, negate it + String query = predicateNode.toString(); + + // For special queries like "ismissing", add the negation operator + String negatedQuery = "-" + query; + + return new Node() { + @Override + public String toString() { + return negatedQuery; + } + + @Override + public String toString(Parenthesize mode) { + return negatedQuery; + } + }; + } + + @Override + public SearchFieldPredicate negate() { + // Double negation returns the original predicate + return predicate; + } +} \ No newline at end of file diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/tag/NotEqualPredicate.java b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/tag/NotEqualPredicate.java index c69f976a..78096210 100644 --- a/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/tag/NotEqualPredicate.java +++ b/redis-om-spring/src/main/java/com/redis/om/spring/search/stream/predicates/tag/NotEqualPredicate.java @@ -55,7 +55,16 @@ public NotEqualPredicate(SearchFieldAccessor field, List list) { * @return the tag values to exclude from matches, either as a single value or collection */ public Iterable getValues() { - return value != null ? (Iterable) value : values; + if (value != null) { + // Check if value is already iterable (e.g., a List or Collection) + if (value instanceof Iterable) { + return (Iterable) value; + } else { + // Wrap single values in a List + return List.of(value); + } + } + return values; } /** diff --git a/tests/src/test/java/com/redis/om/spring/fixtures/document/model/RxDocument.java b/tests/src/test/java/com/redis/om/spring/fixtures/document/model/RxDocument.java new file mode 100644 index 00000000..6a002410 --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/fixtures/document/model/RxDocument.java @@ -0,0 +1,29 @@ +package com.redis.om.spring.fixtures.document.model; + +import com.redis.om.spring.annotations.Document; +import com.redis.om.spring.annotations.Indexed; +import org.springframework.data.annotation.Id; + +import lombok.Data; +import lombok.NoArgsConstructor; +import lombok.AllArgsConstructor; +import lombok.Builder; + +@Data +@NoArgsConstructor +@AllArgsConstructor +@Builder +@Document +public class RxDocument { + @Id + private String id; + + @Indexed + private String rxNumber; + + @Indexed(indexEmpty = true, indexMissing = true) + private String lock; + + @Indexed + private String status; +} \ No newline at end of file diff --git a/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/RxDocumentRepository.java b/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/RxDocumentRepository.java new file mode 100644 index 00000000..a2deab4d --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/fixtures/document/repository/RxDocumentRepository.java @@ -0,0 +1,9 @@ +package com.redis.om.spring.fixtures.document.repository; + +import com.redis.om.spring.fixtures.document.model.RxDocument; +import com.redis.om.spring.repository.RedisDocumentRepository; +import org.springframework.stereotype.Repository; + +@Repository +public interface RxDocumentRepository extends RedisDocumentRepository { +} \ No newline at end of file diff --git a/tests/src/test/java/com/redis/om/spring/search/stream/EntityStreamMissingFieldTest.java b/tests/src/test/java/com/redis/om/spring/search/stream/EntityStreamMissingFieldTest.java new file mode 100644 index 00000000..22fd16aa --- /dev/null +++ b/tests/src/test/java/com/redis/om/spring/search/stream/EntityStreamMissingFieldTest.java @@ -0,0 +1,158 @@ +package com.redis.om.spring.search.stream; + +import com.redis.om.spring.AbstractBaseDocumentTest; +import com.redis.om.spring.fixtures.document.model.RxDocument; +import com.redis.om.spring.fixtures.document.model.RxDocument$; +import com.redis.om.spring.fixtures.document.repository.RxDocumentRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.List; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertAll; + +class EntityStreamMissingFieldTest extends AbstractBaseDocumentTest { + + @Autowired + private RxDocumentRepository rxDocumentRepository; + + @Autowired + private EntityStream entityStream; + + @BeforeEach + void beforeEach() { + rxDocumentRepository.deleteAll(); + + // Create test data with various lock states + RxDocument rx1 = RxDocument.builder() + .rxNumber("RX001") + .lock("LOCKED") + .status("ACTIVE") + .build(); + + RxDocument rx2 = RxDocument.builder() + .rxNumber("RX002") + .lock("") // Empty string + .status("ACTIVE") + .build(); + + RxDocument rx3 = RxDocument.builder() + .rxNumber("RX003") + .lock(null) // Null value (will be missing in Redis) + .status("ACTIVE") + .build(); + + RxDocument rx4 = RxDocument.builder() + .rxNumber("RX004") + .lock("PROCESSING") + .status("ACTIVE") + .build(); + + RxDocument rx5 = RxDocument.builder() + .rxNumber("RX005") + // lock field not set at all (will be missing) + .status("INACTIVE") + .build(); + + rxDocumentRepository.saveAll(List.of(rx1, rx2, rx3, rx4, rx5)); + } + + @Test + void testIsMissingNegateProducesWildcardQuery_ReproducesIssue() { + // This test reproduces the issue where isMissing().negate() produces "*" query + // instead of the proper filter "-ismissing(@lock)" + // Capture the actual query that gets executed + String indexName = RxDocument.class.getName() + "Idx"; + + // Try using isMissing().negate() as user reported + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing().negate()) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + System.out.println("Results from isMissing().negate(): " + results); + + // After fix: should return RX001, RX002, RX004 + // RX001: lock = "LOCKED" (not missing) + // RX002: lock = "" (not missing, just empty) + // RX004: lock = "PROCESSING" (not missing) + // RX003 and RX005 have null/missing lock fields and should be filtered out + + assertThat(results).as("isMissing().negate() should filter out documents with missing lock field") + .containsExactlyInAnyOrder("RX001", "RX002", "RX004"); + } + + @Test + void testIsMissingAlone() { + // Test that isMissing() works correctly on its own + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing()) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + // Should return RX003 and RX005 (where lock is null/missing) + assertThat(results).as("isMissing() should find documents with missing lock field") + .containsExactlyInAnyOrder("RX003", "RX005"); + } + + @Test + void testNotEmptyQuery() { + // Test filtering for non-empty lock values + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.notEq("")) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + // Should return documents where lock is not an empty string + // This includes RX001, RX003, RX004, RX005 (excludes only RX002 with empty string) + assertThat(results).as("notEq('') should filter out only empty strings") + .containsExactlyInAnyOrder("RX001", "RX003", "RX004", "RX005"); + } + + @Test + void testCombinedFiltersWithNegatedMissing() { + // Test combining multiple filters with negated missing + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.STATUS.eq("ACTIVE")) + .filter(RxDocument$.LOCK.isMissing().negate()) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + // Should return only ACTIVE documents with non-missing lock + // RX001, RX002, RX004 are ACTIVE and have non-missing lock + assertThat(results).as("Combined filter should work correctly") + .containsExactlyInAnyOrder("RX001", "RX002", "RX004"); + } + + @Test + void testDoubleNegation() { + // Test that double negation returns to original + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing().negate().negate()) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + // Double negation should return to original isMissing() + assertThat(results).as("Double negation should return to original predicate") + .containsExactlyInAnyOrder("RX003", "RX005"); + } + + @Test + void testFindNonEmptyAndNonMissingValues() { + // Test finding values that are both not empty AND not missing + + // This combines two conditions: not missing AND not empty + List results = entityStream.of(RxDocument.class) + .filter(RxDocument$.LOCK.isMissing().negate()) + .filter(RxDocument$.LOCK.notEq("")) + .map(RxDocument$.RX_NUMBER) + .collect(Collectors.toList()); + + // Should return only RX001 and RX004 (not missing AND not empty) + assertThat(results).as("Should find only non-empty and non-missing values") + .containsExactlyInAnyOrder("RX001", "RX004"); + } +} \ No newline at end of file