Skip to content

Add query sort parameter for updateFirst method. #4888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
*
* @author Christoph Strobl
* @author Mark Paluch
* @author Florian Lüdiger
* @since 3.0
*/
class QueryOperations {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
* @author Laszlo Csontos
* @author duozhilin
* @author Jakub Zurawa
* @author Florian Lüdiger
*/
@ExtendWith(MongoClientExtension.class)
public class MongoTemplateTests {
Expand Down Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down