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 index b984c379c6..df4d9d75f6 100644 --- 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,12 +1683,6 @@ 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) @@ -1695,7 +1690,7 @@ protected UpdateResult doUpdate(String collectionName, Query query, UpdateDefini 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 index aca19a12ed..b5bcf909e9 100644 --- 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 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/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index da3066fd19..ea427a3e1f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-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,12 +1731,6 @@ public Mono updateMulti(Query query, UpdateDefinition update, Clas protected Mono 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) @@ -1743,7 +1738,7 @@ protected Mono doUpdate(String collectionName, Query query, @Nulla updateContext.increaseVersionForUpdateIfNecessary(entity); Document queryObj = updateContext.getMappedQuery(entity); - UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass); + UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass, query); Flux result; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 36467f5ee2..cf9fc09cd2 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-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 createAfterSaveReference() { AtomicReference saved = new AtomicReference<>(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java index 80dd584b9e..c130436a55 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java +++ b/spring-data-mongodb/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();