Fix predicate construction on keyset scrolling. @schauder

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>5.2.0-SNAPSHOT</version>
+	<version>5.2.0-5212-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

spring-data-mongodb-distribution/pom.xml

@@ -13,7 +13,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>5.2.0-SNAPSHOT</version>
+		<version>5.2.0-5212-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -11,7 +11,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>5.2.0-SNAPSHOT</version>
+		<version>5.2.0-5212-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

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<Document> or = getKeysetCriteria(queryObject, sortObject, sortKeys, keysetValues);
+			List<Document> 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<Document> getKeysetCriteria(Document queryObject, Document sortObject, List<String> sortKeys,
+		private List<Document> getKeysetCriteria(Document sortObject, List<String> sortKeys,
 				Map<String, Object> keysetValues) {
 
-			List<Document> or = new ArrayList<>((List<Document>) queryObject.getOrDefault("$or", Collections.emptyList()));
+			List<Document> 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

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 @@ <T> void scrollThroughResultsWithRenamedField(Class<T> resultType, Function<With
 		assertThat(window).containsOnly(assertionConverter.apply(one));
 	}
 
+	@Test // GH-5212
+	void keysetScrollShouldRespectOrPredicateOnSubsequentPages() {
+
+		template.insertAll(Arrays.asList(
+				new AccessibleDocument("user1", false, 10), new AccessibleDocument("user1", false, 20),
+				new AccessibleDocument("user1", false, 30), new AccessibleDocument("anyone", true, 5),
+				new AccessibleDocument("anyone", true, 15), new AccessibleDocument("anyone", true, 25),
+				new AccessibleDocument("other", false, 12), new AccessibleDocument("other", false, 22),
+				new AccessibleDocument("other", false, 32)));
+
+		Criteria visibilityFilter = new Criteria().orOperator(where("owner").is("user1"),
+				where("accessible").is(true));
+		Query q = new Query(visibilityFilter).with(Sort.by("score")).limit(3);
+		q.with(ScrollPosition.keyset());
+
+		Window<AccessibleDocument> 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<AccessibleDocument> page2 = template.scroll(q.with(page1.positionAt(page1.size() - 1)),
+				AccessibleDocument.class);
+
+		List<AccessibleDocument> 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<Arguments> 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;

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<Document> andClauses = (List<Document>) 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() {