Add query sort parameter for updateFirst method.

Closes spring-projects#4797 Signed-off-by: Florian Lüdiger <[email protected]>
florianluediger · Jan 30, 2025 · d890694 · d890694
1 parent dd4579c
commit d890694
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 23 deletions.
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java
@@ -181,6 +181,7 @@
  * @author Bartłomiej Mazur
  * @author Michael Krog
  * @author Jakub Zurawa
+ * @author Florian Lüdiger
  */
 public class MongoTemplate
 		implements MongoOperations, ApplicationContextAware, IndexOperationsProvider, ReadPreferenceAware {
@@ -1682,20 +1683,14 @@ protected UpdateResult doUpdate(String collectionName, Query query, UpdateDefini
 		Assert.notNull(query, "Query must not be null");
 		Assert.notNull(update, "Update must not be null");
 
-		if (query.isSorted() && LOGGER.isWarnEnabled()) {
-
-			LOGGER.warn(String.format("%s does not support sort ('%s'); Please use findAndModify() instead",
-					upsert ? "Upsert" : "UpdateFirst", serializeToJsonSafely(query.getSortObject())));
-		}
-
 		MongoPersistentEntity<?> entity = entityClass == null ? null : getPersistentEntity(entityClass);
 
 		UpdateContext updateContext = multi ? queryOperations.updateContext(update, query, upsert)
 				: queryOperations.updateSingleContext(update, query, upsert);
 		updateContext.increaseVersionForUpdateIfNecessary(entity);
 
 		Document queryObj = updateContext.getMappedQuery(entity);
-		UpdateOptions opts = updateContext.getUpdateOptions(entityClass);
+		UpdateOptions opts = updateContext.getUpdateOptions(entityClass, query);
 
 		if (updateContext.isAggregationUpdate()) {
 

diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java
@@ -77,6 +77,7 @@
  *
  * @author Christoph Strobl
  * @author Mark Paluch
+ * @author Florian Lüdiger
  * @since 3.0
  */
 class QueryOperations {
@@ -740,7 +741,7 @@ class UpdateContext extends QueryContext {
 		/**
 		 * Get the {@link UpdateOptions} applicable for the {@link Query}.
 		 *
-		 * @param domainType must not be {@literal null}.
+		 * @param domainType can be {@literal null}.
 		 * @return never {@literal null}.
 		 */
 		UpdateOptions getUpdateOptions(@Nullable Class<?> domainType) {
@@ -751,11 +752,10 @@ UpdateOptions getUpdateOptions(@Nullable Class<?> domainType) {
 		 * Get the {@link UpdateOptions} applicable for the {@link Query}.
 		 *
 		 * @param domainType can be {@literal null}.
-		 * @param callback a callback to modify the generated options. Can be {@literal null}.
-		 * @return
+		 * @param query can be {@literal null}
+		 * @return never {@literal null}.
 		 */
-		UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Consumer<UpdateOptions> callback) {
-
+		UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Query query) {
 			UpdateOptions options = new UpdateOptions();
 			options.upsert(upsert);
 
@@ -764,13 +764,13 @@ UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Consumer
 						.arrayFilters(update.getArrayFilters().stream().map(ArrayFilter::asDocument).collect(Collectors.toList()));
 			}
 
+			if (query != null && query.isSorted()) {
+				options.sort(query.getSortObject());
+			}
+
 			HintFunction.from(getQuery().getHint()).ifPresent(codecRegistryProvider, options::hintString, options::hint);
 			applyCollation(domainType, options::collation);
 
-			if (callback != null) {
-				callback.accept(options);
-			}
-
 			return options;
 		}
 

diff --git a/...ta-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/...ta-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java
@@ -183,6 +183,7 @@
  * @author Roman Puchkovskiy
  * @author Mathieu Ouellet
  * @author Yadhukrishna S Pai
+ * @author Florian Lüdiger
  * @since 2.0
  */
 public class ReactiveMongoTemplate implements ReactiveMongoOperations, ApplicationContextAware {
@@ -1730,20 +1731,14 @@ public Mono<UpdateResult> updateMulti(Query query, UpdateDefinition update, Clas
 	protected Mono<UpdateResult> doUpdate(String collectionName, Query query, @Nullable UpdateDefinition update,
 			@Nullable Class<?> entityClass, boolean upsert, boolean multi) {
 
-		if (query.isSorted() && LOGGER.isWarnEnabled()) {
-
-			LOGGER.warn(String.format("%s does not support sort ('%s'); Please use findAndModify() instead",
-					upsert ? "Upsert" : "UpdateFirst", serializeToJsonSafely(query.getSortObject())));
-		}
-
 		MongoPersistentEntity<?> entity = entityClass == null ? null : getPersistentEntity(entityClass);
 
 		UpdateContext updateContext = multi ? queryOperations.updateContext(update, query, upsert)
 				: queryOperations.updateSingleContext(update, query, upsert);
 		updateContext.increaseVersionForUpdateIfNecessary(entity);
 
 		Document queryObj = updateContext.getMappedQuery(entity);
-		UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass);
+		UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass, query);
 
 		Flux<UpdateResult> result;
 

diff --git a/...-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/...-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java
@@ -126,6 +126,7 @@
  * @author Laszlo Csontos
  * @author duozhilin
  * @author Jakub Zurawa
+ * @author Florian Lüdiger
  */
 @ExtendWith(MongoClientExtension.class)
 public class MongoTemplateTests {
@@ -4065,6 +4066,64 @@ void readsMapWithDotInKey() {
 		assertThat(loaded.mapValue).isEqualTo(sourceMap);
 	}
 
+	@Test // GH-4797
+	public void updateFirstWithSortingAscendingShouldUpdateCorrectEntities() {
+
+		PersonWithIdPropertyOfTypeObjectId youngPerson = new PersonWithIdPropertyOfTypeObjectId();
+		youngPerson.setId(new ObjectId());
+		youngPerson.setAge(27);
+		youngPerson.setFirstName("Dave");
+		template.save(youngPerson);
+
+		PersonWithIdPropertyOfTypeObjectId oldPerson = new PersonWithIdPropertyOfTypeObjectId();
+		oldPerson.setId(new ObjectId());
+		oldPerson.setAge(34);
+		oldPerson.setFirstName("Dave");
+		template.save(oldPerson);
+
+		template.updateFirst(query(where("firstName").is("Dave")).with(Sort.by(Direction.ASC, "age")),
+				update("firstName", "Mike"), PersonWithIdPropertyOfTypeObjectId.class);
+
+		PersonWithIdPropertyOfTypeObjectId oldPersonResult = template.findById(oldPerson.getId(),
+				PersonWithIdPropertyOfTypeObjectId.class);
+		assertThat(oldPersonResult).isNotNull();
+		assertThat(oldPersonResult.getFirstName()).isEqualTo("Dave");
+
+		PersonWithIdPropertyOfTypeObjectId youngPersonResult = template.findById(youngPerson.getId(),
+				PersonWithIdPropertyOfTypeObjectId.class);
+		assertThat(youngPersonResult).isNotNull();
+		assertThat(youngPersonResult.getFirstName()).isEqualTo("Mike");
+	}
+
+	@Test // GH-4797
+	public void updateFirstWithSortingDescendingShouldUpdateCorrectEntities() {
+
+		PersonWithIdPropertyOfTypeObjectId youngPerson = new PersonWithIdPropertyOfTypeObjectId();
+		youngPerson.setId(new ObjectId());
+		youngPerson.setAge(27);
+		youngPerson.setFirstName("Dave");
+		template.save(youngPerson);
+
+		PersonWithIdPropertyOfTypeObjectId oldPerson = new PersonWithIdPropertyOfTypeObjectId();
+		oldPerson.setId(new ObjectId());
+		oldPerson.setAge(34);
+		oldPerson.setFirstName("Dave");
+		template.save(oldPerson);
+
+		template.updateFirst(query(where("firstName").is("Dave")).with(Sort.by(Direction.DESC, "age")),
+				update("firstName", "Mike"), PersonWithIdPropertyOfTypeObjectId.class);
+
+		PersonWithIdPropertyOfTypeObjectId oldPersonResult = template.findById(oldPerson.getId(),
+				PersonWithIdPropertyOfTypeObjectId.class);
+		assertThat(oldPersonResult).isNotNull();
+		assertThat(oldPersonResult.getFirstName()).isEqualTo("Mike");
+
+		PersonWithIdPropertyOfTypeObjectId youngPersonResult = template.findById(youngPerson.getId(),
+				PersonWithIdPropertyOfTypeObjectId.class);
+		assertThat(youngPersonResult).isNotNull();
+		assertThat(youngPersonResult.getFirstName()).isEqualTo("Dave");
+	}
+
 	private AtomicReference<ImmutableVersioned> createAfterSaveReference() {
 
 		AtomicReference<ImmutableVersioned> saved = new AtomicReference<>();

diff --git a/...ngodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java b/...ngodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java
@@ -1846,6 +1846,42 @@ void resumesAtBsonTimestampCorrectly() throws InterruptedException {
 				.verify();
 	}
 
+	@Test // GH-4797
+	public void updateFirstWithSortingAscendingShouldUpdateCorrectEntities() {
+
+		Person youngPerson = new Person("Dave", 27);
+		Person oldPerson = new Person("Dave", 34);
+
+		template.insertAll(List.of(youngPerson, oldPerson))
+				.then(template.updateFirst(new Query(where("firstName").is("Dave")).with(Sort.by(Direction.ASC, "age")),
+						new Update().set("firstName", "Carter"), Person.class))
+				.flatMapMany(p -> template.find(new Query().with(Sort.by(Direction.ASC, "age")), Person.class))
+				.as(StepVerifier::create) //
+				.consumeNextWith(actual -> {
+					assertThat(actual.getFirstName()).isEqualTo("Carter");
+				}).consumeNextWith(actual -> {
+					assertThat(actual.getFirstName()).isEqualTo("Dave");
+				}).verifyComplete();
+	}
+
+	@Test // GH-4797
+	public void updateFirstWithSortingDescendingShouldUpdateCorrectEntities() {
+
+		Person youngPerson = new Person("Dave", 27);
+		Person oldPerson = new Person("Dave", 34);
+
+		template.insertAll(List.of(youngPerson, oldPerson))
+				.then(template.updateFirst(new Query(where("firstName").is("Dave")).with(Sort.by(Direction.DESC, "age")),
+						new Update().set("firstName", "Carter"), Person.class))
+				.flatMapMany(p -> template.find(new Query().with(Sort.by(Direction.ASC, "age")), Person.class))
+				.as(StepVerifier::create) //
+				.consumeNextWith(actual -> {
+					assertThat(actual.getFirstName()).isEqualTo("Dave");
+				}).consumeNextWith(actual -> {
+					assertThat(actual.getFirstName()).isEqualTo("Carter");
+				}).verifyComplete();
+	}
+
 	private PersonWithAList createPersonWithAList(String firstname, int age) {
 
 		PersonWithAList p = new PersonWithAList();