Skip to content

Commit 58001ee

Browse files
Add query sort parameter for updateFirst method.
Closes spring-projects#4797 Signed-off-by: Florian Lüdiger <[email protected]>
1 parent dd4579c commit 58001ee

File tree

5 files changed

+108
-23
lines changed

5 files changed

+108
-23
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

+2-7
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@
181181
* @author Bartłomiej Mazur
182182
* @author Michael Krog
183183
* @author Jakub Zurawa
184+
* @author Florian Lüdiger
184185
*/
185186
public class MongoTemplate
186187
implements MongoOperations, ApplicationContextAware, IndexOperationsProvider, ReadPreferenceAware {
@@ -1682,20 +1683,14 @@ protected UpdateResult doUpdate(String collectionName, Query query, UpdateDefini
16821683
Assert.notNull(query, "Query must not be null");
16831684
Assert.notNull(update, "Update must not be null");
16841685

1685-
if (query.isSorted() && LOGGER.isWarnEnabled()) {
1686-
1687-
LOGGER.warn(String.format("%s does not support sort ('%s'); Please use findAndModify() instead",
1688-
upsert ? "Upsert" : "UpdateFirst", serializeToJsonSafely(query.getSortObject())));
1689-
}
1690-
16911686
MongoPersistentEntity<?> entity = entityClass == null ? null : getPersistentEntity(entityClass);
16921687

16931688
UpdateContext updateContext = multi ? queryOperations.updateContext(update, query, upsert)
16941689
: queryOperations.updateSingleContext(update, query, upsert);
16951690
updateContext.increaseVersionForUpdateIfNecessary(entity);
16961691

16971692
Document queryObj = updateContext.getMappedQuery(entity);
1698-
UpdateOptions opts = updateContext.getUpdateOptions(entityClass);
1693+
UpdateOptions opts = updateContext.getUpdateOptions(entityClass, query);
16991694

17001695
if (updateContext.isAggregationUpdate()) {
17011696

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
*
7878
* @author Christoph Strobl
7979
* @author Mark Paluch
80+
* @author Florian Lüdiger
8081
* @since 3.0
8182
*/
8283
class QueryOperations {
@@ -740,7 +741,7 @@ class UpdateContext extends QueryContext {
740741
/**
741742
* Get the {@link UpdateOptions} applicable for the {@link Query}.
742743
*
743-
* @param domainType must not be {@literal null}.
744+
* @param domainType can be {@literal null}.
744745
* @return never {@literal null}.
745746
*/
746747
UpdateOptions getUpdateOptions(@Nullable Class<?> domainType) {
@@ -751,11 +752,10 @@ UpdateOptions getUpdateOptions(@Nullable Class<?> domainType) {
751752
* Get the {@link UpdateOptions} applicable for the {@link Query}.
752753
*
753754
* @param domainType can be {@literal null}.
754-
* @param callback a callback to modify the generated options. Can be {@literal null}.
755-
* @return
755+
* @param query can be {@literal null}
756+
* @return never {@literal null}.
756757
*/
757-
UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Consumer<UpdateOptions> callback) {
758-
758+
UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Query query) {
759759
UpdateOptions options = new UpdateOptions();
760760
options.upsert(upsert);
761761

@@ -764,13 +764,13 @@ UpdateOptions getUpdateOptions(@Nullable Class<?> domainType, @Nullable Consumer
764764
.arrayFilters(update.getArrayFilters().stream().map(ArrayFilter::asDocument).collect(Collectors.toList()));
765765
}
766766

767+
if (query != null && query.isSorted()) {
768+
options.sort(query.getSortObject());
769+
}
770+
767771
HintFunction.from(getQuery().getHint()).ifPresent(codecRegistryProvider, options::hintString, options::hint);
768772
applyCollation(domainType, options::collation);
769773

770-
if (callback != null) {
771-
callback.accept(options);
772-
}
773-
774774
return options;
775775
}
776776

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java

+2-7
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@
183183
* @author Roman Puchkovskiy
184184
* @author Mathieu Ouellet
185185
* @author Yadhukrishna S Pai
186+
* @author Florian Lüdiger
186187
* @since 2.0
187188
*/
188189
public class ReactiveMongoTemplate implements ReactiveMongoOperations, ApplicationContextAware {
@@ -1730,20 +1731,14 @@ public Mono<UpdateResult> updateMulti(Query query, UpdateDefinition update, Clas
17301731
protected Mono<UpdateResult> doUpdate(String collectionName, Query query, @Nullable UpdateDefinition update,
17311732
@Nullable Class<?> entityClass, boolean upsert, boolean multi) {
17321733

1733-
if (query.isSorted() && LOGGER.isWarnEnabled()) {
1734-
1735-
LOGGER.warn(String.format("%s does not support sort ('%s'); Please use findAndModify() instead",
1736-
upsert ? "Upsert" : "UpdateFirst", serializeToJsonSafely(query.getSortObject())));
1737-
}
1738-
17391734
MongoPersistentEntity<?> entity = entityClass == null ? null : getPersistentEntity(entityClass);
17401735

17411736
UpdateContext updateContext = multi ? queryOperations.updateContext(update, query, upsert)
17421737
: queryOperations.updateSingleContext(update, query, upsert);
17431738
updateContext.increaseVersionForUpdateIfNecessary(entity);
17441739

17451740
Document queryObj = updateContext.getMappedQuery(entity);
1746-
UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass);
1741+
UpdateOptions updateOptions = updateContext.getUpdateOptions(entityClass, query);
17471742

17481743
Flux<UpdateResult> result;
17491744

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java

+59
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
* @author Laszlo Csontos
127127
* @author duozhilin
128128
* @author Jakub Zurawa
129+
* @author Florian Lüdiger
129130
*/
130131
@ExtendWith(MongoClientExtension.class)
131132
public class MongoTemplateTests {
@@ -4065,6 +4066,64 @@ void readsMapWithDotInKey() {
40654066
assertThat(loaded.mapValue).isEqualTo(sourceMap);
40664067
}
40674068

4069+
@Test // GH-4797
4070+
public void updateFirstWithSortingAscendingShouldUpdateCorrectEntities() {
4071+
4072+
PersonWithIdPropertyOfTypeObjectId youngPerson = new PersonWithIdPropertyOfTypeObjectId();
4073+
youngPerson.setId(new ObjectId());
4074+
youngPerson.setAge(27);
4075+
youngPerson.setFirstName("Dave");
4076+
template.save(youngPerson);
4077+
4078+
PersonWithIdPropertyOfTypeObjectId oldPerson = new PersonWithIdPropertyOfTypeObjectId();
4079+
oldPerson.setId(new ObjectId());
4080+
oldPerson.setAge(34);
4081+
oldPerson.setFirstName("Dave");
4082+
template.save(oldPerson);
4083+
4084+
template.updateFirst(query(where("firstName").is("Dave")).with(Sort.by(Direction.ASC, "age")),
4085+
update("firstName", "Mike"), PersonWithIdPropertyOfTypeObjectId.class);
4086+
4087+
PersonWithIdPropertyOfTypeObjectId oldPersonResult = template.findById(oldPerson.getId(),
4088+
PersonWithIdPropertyOfTypeObjectId.class);
4089+
assertThat(oldPersonResult).isNotNull();
4090+
assertThat(oldPersonResult.getFirstName()).isEqualTo("Dave");
4091+
4092+
PersonWithIdPropertyOfTypeObjectId youngPersonResult = template.findById(youngPerson.getId(),
4093+
PersonWithIdPropertyOfTypeObjectId.class);
4094+
assertThat(youngPersonResult).isNotNull();
4095+
assertThat(youngPersonResult.getFirstName()).isEqualTo("Mike");
4096+
}
4097+
4098+
@Test // GH-4797
4099+
public void updateFirstWithSortingDescendingShouldUpdateCorrectEntities() {
4100+
4101+
PersonWithIdPropertyOfTypeObjectId youngPerson = new PersonWithIdPropertyOfTypeObjectId();
4102+
youngPerson.setId(new ObjectId());
4103+
youngPerson.setAge(27);
4104+
youngPerson.setFirstName("Dave");
4105+
template.save(youngPerson);
4106+
4107+
PersonWithIdPropertyOfTypeObjectId oldPerson = new PersonWithIdPropertyOfTypeObjectId();
4108+
oldPerson.setId(new ObjectId());
4109+
oldPerson.setAge(34);
4110+
oldPerson.setFirstName("Dave");
4111+
template.save(oldPerson);
4112+
4113+
template.updateFirst(query(where("firstName").is("Dave")).with(Sort.by(Direction.DESC, "age")),
4114+
update("firstName", "Mike"), PersonWithIdPropertyOfTypeObjectId.class);
4115+
4116+
PersonWithIdPropertyOfTypeObjectId oldPersonResult = template.findById(oldPerson.getId(),
4117+
PersonWithIdPropertyOfTypeObjectId.class);
4118+
assertThat(oldPersonResult).isNotNull();
4119+
assertThat(oldPersonResult.getFirstName()).isEqualTo("Mike");
4120+
4121+
PersonWithIdPropertyOfTypeObjectId youngPersonResult = template.findById(youngPerson.getId(),
4122+
PersonWithIdPropertyOfTypeObjectId.class);
4123+
assertThat(youngPersonResult).isNotNull();
4124+
assertThat(youngPersonResult.getFirstName()).isEqualTo("Dave");
4125+
}
4126+
40684127
private AtomicReference<ImmutableVersioned> createAfterSaveReference() {
40694128

40704129
AtomicReference<ImmutableVersioned> saved = new AtomicReference<>();

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java

+36
Original file line numberDiff line numberDiff line change
@@ -1846,6 +1846,42 @@ void resumesAtBsonTimestampCorrectly() throws InterruptedException {
18461846
.verify();
18471847
}
18481848

1849+
@Test // GH-4797
1850+
public void updateFirstWithSortingAscendingShouldUpdateCorrectEntities() {
1851+
1852+
Person youngPerson = new Person("Dave", 27);
1853+
Person oldPerson = new Person("Dave", 34);
1854+
1855+
template.insertAll(List.of(youngPerson, oldPerson))
1856+
.then(template.updateFirst(new Query(where("firstName").is("Dave")).with(Sort.by(Direction.ASC, "age")),
1857+
new Update().set("firstName", "Carter"), Person.class))
1858+
.flatMapMany(p -> template.find(new Query().with(Sort.by(Direction.ASC, "age")), Person.class))
1859+
.as(StepVerifier::create) //
1860+
.consumeNextWith(actual -> {
1861+
assertThat(actual.getFirstName()).isEqualTo("Carter");
1862+
}).consumeNextWith(actual -> {
1863+
assertThat(actual.getFirstName()).isEqualTo("Dave");
1864+
}).verifyComplete();
1865+
}
1866+
1867+
@Test // GH-4797
1868+
public void updateFirstWithSortingDescendingShouldUpdateCorrectEntities() {
1869+
1870+
Person youngPerson = new Person("Dave", 27);
1871+
Person oldPerson = new Person("Dave", 34);
1872+
1873+
template.insertAll(List.of(youngPerson, oldPerson))
1874+
.then(template.updateFirst(new Query(where("firstName").is("Dave")).with(Sort.by(Direction.DESC, "age")),
1875+
new Update().set("firstName", "Carter"), Person.class))
1876+
.flatMapMany(p -> template.find(new Query().with(Sort.by(Direction.ASC, "age")), Person.class))
1877+
.as(StepVerifier::create) //
1878+
.consumeNextWith(actual -> {
1879+
assertThat(actual.getFirstName()).isEqualTo("Dave");
1880+
}).consumeNextWith(actual -> {
1881+
assertThat(actual.getFirstName()).isEqualTo("Carter");
1882+
}).verifyComplete();
1883+
}
1884+
18491885
private PersonWithAList createPersonWithAList(String firstname, int age) {
18501886

18511887
PersonWithAList p = new PersonWithAList();

0 commit comments

Comments
 (0)