Allow updateOne and replaceOne to supply sort option #1585

joykim1005 · 2024-12-18T18:26:56Z

Ticket

Description

This PR adds a sort option for updateOne and replaceOne commands so if it matches more than one candidate document, the first one matched by sort order will be updated. It also syncs CRUD unified spec tests to this.

Testing

ran ./gradlew check
local testing - ran UnifiedCRUDTest.java

vbabanin

Looking good overall! I noticed in the spec changes here: https://github.com/mongodb/specifications/pull/1644/files#diff-d6a7b61abed64b14a2e1eb4827556e8ed74c84bc8ae1f635ff8de67a4cfbcfba that ReplaceOneOptions might also need to be updated. Should we include that in this PR?

vbabanin · 2024-12-19T02:33:43Z

driver-core/src/main/com/mongodb/client/model/UpdateOptions.java

@@ -40,6 +41,7 @@ public class UpdateOptions {
    private String hintString;
    private BsonValue comment;
    private Bson variables;
+    private BsonDocument sort;


Let’s use the Bson type here to allow users to choose between Document and BsonDocument, similar to how it’s done in FindOneAndDeleteOptions and other cases.

vbabanin · 2024-12-19T03:20:15Z

driver-core/src/main/com/mongodb/client/model/UpdateOptions.java

+     *
+     * @return a document describing the sort criteria
+     * @since 5.3
+     * @mongodb.driver.manual reference/method/cursor.sort/ Sort


Should we remove the manual reference here and from the getSort method, given that it seems more relevant to the cursor?

I referenced the findOptions and the other findOneAnd..Options and thought it might be helpful to have a reference! I don't have a strong opinion here though.

vbabanin · 2024-12-19T03:31:16Z

driver-core/src/main/com/mongodb/client/model/UpdateOptions.java

+     * @since 5.3
+     * @mongodb.driver.manual reference/method/cursor.sort/ Sort
+     */
+    public UpdateOptions sort(@Nullable final BsonDocument sort) {


Should we consider adding a note in the Javadoc to clarify that sort is not applicable to updateMany? This could help avoid confusion for users.

This is a good point, will go ahead and add it!

vbabanin · 2024-12-19T03:43:03Z

driver-core/src/main/com/mongodb/client/model/UpdateOptions.java

+
+    /**
+     * Sets the sort criteria to apply to the query.
+     *


Could we clarify the purpose of this sort option in the Javadoc? It would be helpful to explain how it determines which document is updated when multiple matches are found.

Suggested change

*

*

* When multiple documents match the query, the sort order specifies the document

* to be updated first.

*

vbabanin · 2024-12-19T03:48:26Z

driver-core/src/main/com/mongodb/client/model/BulkWriteOptions.java

@@ -31,6 +32,7 @@ public final class BulkWriteOptions {
    private Boolean bypassDocumentValidation;
    private BsonValue comment;
    private Bson variables;
+    private BsonDocument sort;


Judging by the spec, BulkWriteOptions does not seem to require a sort option.

I overlooked that part, will remove it!

joykim1005 · 2024-12-19T15:01:46Z

Looking good overall! I noticed in the spec changes here: https://github.com/mongodb/specifications/pull/1644/files#diff-d6a7b61abed64b14a2e1eb4827556e8ed74c84bc8ae1f635ff8de67a4cfbcfba that ReplaceOneOptions might also need to be updated. Should we include that in this PR?

I originally didn't include it because the ticket specifically dealt with updateOne, but I can go ahead and include that in this PR and update the ticket as it is adding the same option!

…java-driver into JAVA-5543-test

vbabanin · 2024-12-21T02:04:59Z

driver-core/src/main/com/mongodb/client/model/ReplaceOptions.java

+    /**
+     * Gets the sort criteria to apply to the query. The default is null, which means that the documents will be returned in an undefined
+     * order.
+     *
+     * @return a document describing the sort criteria
+     * @since 5.3
+     * @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort
+     */


The sort criteria in the replace operation determines the order in which matching documents are processed. It does not return documents to the caller unless it’s used in a specific operation like findAndModify.

We should also specify the minimum server version @mongodb.server.release 8.0 for this option.

Should we clarify this in the Javadoc? Here’s a suggestion to make this more explicit:

Suggested change

/**

* Gets the sort criteria to apply to the query. The default is null, which means that the documents will be returned in an undefined

* order.

*

* @return a document describing the sort criteria

* @since 5.3

* @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort

*/

/**

* Gets the sort criteria to apply to the operation.

*

* <p>

* The sort criteria determines which document the operation replaces if the query matches multiple documents.

* The first document matched by the sort criteria will be replaced.

* The default is null, which means no specific sort criteria is applied.

*

* @return a document describing the sort criteria, or null if no sort is specified.

* @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort

* @mongodb.server.release 8.0

* @since 5.3

* @see #sort(Bson)

*/

If the ReplaceOptions.getSort() method Javadoc gets updated, we should make similar changes to the equivalent UpdateOptions method for consistency.

vbabanin · 2024-12-21T02:19:27Z

driver-core/src/main/com/mongodb/client/model/ReplaceOptions.java

+    /**
+     * Sets the sort criteria to apply to the query. When multiple documents match the query, the sort order specifies the document to be
+     * updated first.
+     *
+     * @param sort the sort criteria, which may be null.
+     * @return this
+     * @since 5.3
+     * @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort
+     */


The term replaces might be more appropriate than updates here, as this operation replaces the entire document.

We should also specify the minimum server version that supports this option.

What do you think about updating the Javadoc to something like this?

Suggested change

/**

* Sets the sort criteria to apply to the query. When multiple documents match the query, the sort order specifies the document to be

* updated first.

*

* @param sort the sort criteria, which may be null.

* @return this

* @since 5.3

* @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort

*/

/**

* Sets the sort criteria to apply to the operation. A null value means no sort criteria is set.

*

* <p>

* The sort criteria determines which document the operation replaces if the query matches multiple documents.

* The first document matched by the specified sort criteria will be replaced.

*

* @param sort the sort criteria, which may be null.

* @return this

* @mongodb.driver.manual reference/method/db.collection.replaceOne/ Sort

* @mongodb.server.release 8.0

* @since 5.3

*/

If the ReplaceOptions.sort() method Javadoc gets updated, we should make similar changes to the equivalent UpdateOptions method for consistency.

vbabanin · 2024-12-21T19:58:35Z

driver-core/src/main/com/mongodb/client/model/UpdateOptions.java

Let’s also add unit tests to UpdateOptionsSpecification and UpdateRequestSpecification.

Additionally, we should update the tests in MongoCollectionSpecification to cover the logic for the new option.

vbabanin · 2024-12-27T05:31:09Z

driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java

+    public BsonDocument getSort() {
+        return sort;
+    }
+
+    public MixedBulkWriteOperation sort(@Nullable final BsonDocument sort) {
+        this.sort = sort;
+        return this;
+    }
+


These methods don’t appear to be used. Should we consider removing them?

This also applies to ReplaceRequest.

Allow update to supply sort option

100910d

joykim1005 marked this pull request as ready for review December 18, 2024 19:24

joykim1005 requested review from a team and jyemin and removed request for a team December 18, 2024 19:25

jyemin requested review from vbabanin and removed request for vbabanin December 18, 2024 19:26

vbabanin self-requested a review December 18, 2024 20:48

Merge branch 'main' into JAVA-5543-test

d51fa83

vbabanin requested changes Dec 19, 2024

View reviewed changes

joykim1005 added 2 commits December 19, 2024 16:56

Update PR

19c77f3

Merge branch 'JAVA-5543-test' of https://github.com/joykim1005/mongo-…

fc7ec3e

…java-driver into JAVA-5543-test

joykim1005 requested a review from vbabanin December 19, 2024 21:59

vbabanin assigned joykim1005 Dec 21, 2024

joykim1005 changed the title ~~Allow update to supply sort option~~ Allow updateOne and replaceOne to supply sort option Dec 26, 2024

vbabanin reviewed Dec 27, 2024

View reviewed changes

update

445b15d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow updateOne and replaceOne to supply sort option #1585

Allow updateOne and replaceOne to supply sort option #1585

joykim1005 commented Dec 18, 2024 •

edited

Loading

vbabanin left a comment

vbabanin Dec 19, 2024

vbabanin Dec 19, 2024

joykim1005 Dec 19, 2024

vbabanin Dec 19, 2024 •

edited

Loading

joykim1005 Dec 19, 2024

vbabanin Dec 19, 2024

joykim1005 Dec 19, 2024

vbabanin Dec 19, 2024

joykim1005 Dec 19, 2024

joykim1005 commented Dec 19, 2024

vbabanin Dec 21, 2024

vbabanin Dec 21, 2024

vbabanin Dec 21, 2024

vbabanin Dec 27, 2024 •

edited

Loading

-     *
+     *
+     * When multiple documents match the query, the sort order specifies the document
+     * to be updated first.
+     *

Allow updateOne and replaceOne to supply sort option #1585

Are you sure you want to change the base?

Allow updateOne and replaceOne to supply sort option #1585

Conversation

joykim1005 commented Dec 18, 2024 • edited Loading

Ticket

Description

Testing

vbabanin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbabanin Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joykim1005 commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbabanin Dec 27, 2024 • edited Loading

Choose a reason for hiding this comment

joykim1005 commented Dec 18, 2024 •

edited

Loading

vbabanin Dec 19, 2024 •

edited

Loading

vbabanin Dec 27, 2024 •

edited

Loading