diff --git a/pom.xml b/pom.xml index 19ab7f04c7..85929823dc 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 5.2.0-SNAPSHOT + 5.2.0-5212-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 747b3f0d79..020d839498 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 5.2.0-SNAPSHOT + 5.2.0-5212-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 3bdffedb8c..dc30a7cbc9 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 5.2.0-SNAPSHOT + 5.2.0-5212-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ScrollUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ScrollUtils.java index 2965ddc53c..c9a30ccadc 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ScrollUtils.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ScrollUtils.java @@ -37,6 +37,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author Jens Schauder * @since 4.1 */ class ScrollUtils { @@ -160,20 +161,24 @@ public Document createQuery(KeysetScrollPosition keyset, Document queryObject, D throw new IllegalStateException("KeysetScrollPosition does not contain all keyset values"); } - List or = getKeysetCriteria(queryObject, sortObject, sortKeys, keysetValues); + List or = getKeysetCriteria(sortObject, sortKeys, keysetValues); if (or.isEmpty()) { return queryObject; } + if (queryObject.containsKey("$or")) { + return new Document("$and", List.of(queryObject, new Document("$or", or))); + } + Document filterQuery = new Document(queryObject); filterQuery.put("$or", or); return filterQuery; } - private List getKeysetCriteria(Document queryObject, Document sortObject, List sortKeys, + private List getKeysetCriteria(Document sortObject, List sortKeys, Map keysetValues) { - List or = new ArrayList<>((List) queryObject.getOrDefault("$or", Collections.emptyList())); + List or = new ArrayList<>(); // build matrix query for keyset paging that contains sort^2 queries // reflecting a query that follows sort order semantics starting from the last returned keyset diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateScrollTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateScrollTests.java index 1068b909a7..50fd15d8b6 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateScrollTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateScrollTests.java @@ -21,6 +21,7 @@ import java.lang.reflect.Proxy; import java.util.Arrays; import java.util.Comparator; +import java.util.List; import java.util.Objects; import java.util.function.Function; import java.util.stream.Stream; @@ -44,6 +45,7 @@ import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.mongodb.core.MongoTemplateTests.PersonWithIdPropertyOfTypeUUIDListener; import org.springframework.data.mongodb.core.mapping.Field; +import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.test.util.Client; import org.springframework.data.mongodb.test.util.MongoTestTemplate; @@ -56,6 +58,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author Jens Schauder */ class MongoTemplateScrollTests { @@ -106,6 +109,7 @@ void setUp() { template.remove(Person.class).all(); template.remove(WithNestedDocument.class).all(); template.remove(WithRenamedField.class).all(); + template.remove(AccessibleDocument.class).all(); } @Test // GH-4308 @@ -288,6 +292,39 @@ void scrollThroughResultsWithRenamedField(Class resultType, Function page1 = template.scroll(q, AccessibleDocument.class); + + assertThat(page1).hasSize(3); + assertThat(page1).allSatisfy(doc -> assertThat("user1".equals(doc.owner) || doc.accessible) + .as("page 1 element %s must satisfy the visibility filter", doc).isTrue()); + + Window page2 = template.scroll(q.with(page1.positionAt(page1.size() - 1)), + AccessibleDocument.class); + + List unauthorized = page2.stream() + .filter(doc -> !"user1".equals(doc.owner) && !doc.accessible).toList(); + + assertThat(unauthorized) + .as("keyset scroll must not return unauthorized documents on page 2 when base query uses $or" + + " – keyset predicates must be ANDed with the access-control filter, not OR-ed") + .isEmpty(); + } + static Stream positions() { return Stream.of(args(ScrollPosition.keyset(), Person.class, Function.identity()), // @@ -487,6 +524,27 @@ public String toString() { } } + static class AccessibleDocument { + + String id; + String owner; + boolean accessible; + int score; + + AccessibleDocument() {} + + AccessibleDocument(String owner, boolean accessible, int score) { + this.owner = owner; + this.accessible = accessible; + this.score = score; + } + + @Override + public String toString() { + return "AccessibleDocument{owner='%s', accessible=%s, score=%d}".formatted(owner, accessible, score); + } + } + class WithNestedDocument { String id; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ScrollUtilsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ScrollUtilsUnitTests.java index 1b6ad03cfe..8ddef458f8 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ScrollUtilsUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ScrollUtilsUnitTests.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; +import org.bson.Document; import org.junit.jupiter.api.Test; import org.springframework.data.domain.KeysetScrollPosition; import org.springframework.data.domain.ScrollPosition; @@ -33,9 +34,34 @@ * Unit tests for {@link ScrollUtils}. * * @author Mark Paluch + * @author Jens Schauder */ class ScrollUtilsUnitTests { + @Test // GH-5212 + void keysetCriteriaShouldNotMergeIntoExistingTopLevelOrPredicate() { + + Document baseQuery = new Document("$or", + List.of(new Document("owner", "user1"), new Document("visible", true))); + Document sortObject = new Document("score", 1).append("_id", 1); + KeysetScrollPosition keyset = (KeysetScrollPosition) ScrollPosition.of(Map.of("score", 10, "_id", "abc"), + ScrollPosition.Direction.FORWARD); + + Document result = ScrollUtils.KeysetScrollDirector.of(ScrollPosition.Direction.FORWARD).createQuery(keyset, + baseQuery, sortObject); + + assertThat(result.containsKey("$and")) + .as("query with a top-level $or must combine base criteria and keyset criteria with $and, " + + "not merge them into a single $or") + .isTrue(); + + @SuppressWarnings("unchecked") + List andClauses = (List) result.get("$and"); + assertThat(andClauses).isNotNull(); + assertThat(andClauses.contains(baseQuery)) + .as("$and must contain the original base query as one of its arms").isTrue(); + } + @Test // GH-4413 void positionShouldRetainScrollDirection() {