findOneAndUpdate() and findOneAndReplace() to return document whe…

…n no data change (#392)
stargate · Apr 24, 2023 · f6d32f1 · f6d32f1
1 parent 10495a8
commit f6d32f1
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 3 deletions.
diff --git a/...in/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java b/...in/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java
@@ -162,8 +162,17 @@ private Uni<UpdatedDocument> processUpdate(
               // if no changes return null item
               DocumentUpdater.DocumentUpdaterResponse documentUpdaterResponse =
                   documentUpdater().apply(readDocument.document().deepCopy(), upsert);
-              if (!documentUpdaterResponse.modified()) {
-                return Uni.createFrom().nullItem();
+
+              // In case no change to document and not an upsert document, short circuit and return
+              if (!documentUpdaterResponse.modified() && !upsert) {
+                // If no change return the original document Issue #390
+                if (returnDocumentInResponse) {
+                  resultProjection.applyProjection(originalDocument);
+                  return Uni.createFrom()
+                      .item(new UpdatedDocument(readDocument.id(), upsert, originalDocument, null));
+                } else {
+                  return Uni.createFrom().nullItem();
+                }
               }
 
               // otherwise shred

diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndReplaceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndReplaceIntegrationTest.java
@@ -176,7 +176,8 @@ public void byIdWithIdNoChange() {
           .post(CollectionResource.BASE_PATH, namespaceName, collectionName)
           .then()
           .statusCode(200)
-          .body("data.docs", hasSize(0))
+          .body("data.docs", hasSize(1))
+          .body("data.docs[0]", jsonEquals(document))
           .body("status.matchedCount", is(1))
           .body("status.modifiedCount", is(0))
           .body("errors", is(nullValue()));

diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndUpdateIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndUpdateIntegrationTest.java
@@ -4,6 +4,7 @@
 import static io.stargate.sgv2.common.IntegrationTestUtils.getAuthToken;
 import static net.javacrumbs.jsonunit.JsonMatchers.jsonEquals;
 import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
@@ -87,6 +88,44 @@ public void byIdAndSet() {
           .body("data.docs[0]", jsonEquals(expected));
     }
 
+    @Test
+    public void byIdAndSetNoChange() {
+      String document =
+          """
+        {
+          "_id": "doc3",
+          "username": "admin",
+          "active_user" : true
+        }
+        """;
+      insertDoc(document);
+
+      String json =
+          """
+        {
+          "findOneAndUpdate": {
+            "filter" : {"_id" : "doc3"},
+            "sort": { "username": 1 },
+            "update" : {"$set" : {"username": "admin"}},
+            "options": {"returnDocument": "before"}
+          }
+        }
+        """;
+      given()
+          .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
+          .contentType(ContentType.JSON)
+          .body(json)
+          .when()
+          .post(CollectionResource.BASE_PATH, namespaceName, collectionName)
+          .then()
+          .statusCode(200)
+          .body("data.docs", hasSize(1))
+          .body("data.docs[0]", jsonEquals(document))
+          .body("status.matchedCount", is(1))
+          .body("status.modifiedCount", is(0))
+          .body("errors", is(nullValue()));
+    }
+
     @Test
     public void byIdAndSetNotFound() {
       String json =

diff --git a/...ava/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java b/...ava/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java
@@ -218,6 +218,109 @@ public void happyPath() throws Exception {
       assertThat(result.errors()).isNull();
     }
 
+    @Test
+    public void noChange() throws Exception {
+      String collectionReadCql =
+          "SELECT key, tx_id, doc_json FROM \"%s\".\"%s\" WHERE key = ? LIMIT 1"
+              .formatted(KEYSPACE_NAME, COLLECTION_NAME);
+
+      UUID tx_id = UUID.randomUUID();
+      String doc1 =
+          """
+                {
+                  "_id": "doc1",
+                  "username": "user1"
+                }
+                """;
+
+      ValidatingStargateBridge.QueryAssert selectQueryAssert =
+          withQuery(
+                  collectionReadCql,
+                  Values.of(
+                      CustomValueSerializers.getDocumentIdValue(DocumentId.fromString("doc1"))))
+              .withPageSize(1)
+              .withColumnSpec(
+                  List.of(
+                      QueryOuterClass.ColumnSpec.newBuilder()
+                          .setName("key")
+                          .setType(TypeSpecs.tuple(TypeSpecs.TINYINT, TypeSpecs.VARCHAR))
+                          .build(),
+                      QueryOuterClass.ColumnSpec.newBuilder()
+                          .setName("tx_id")
+                          .setType(TypeSpecs.UUID)
+                          .build(),
+                      QueryOuterClass.ColumnSpec.newBuilder()
+                          .setName("doc_json")
+                          .setType(TypeSpecs.VARCHAR)
+                          .build()))
+              .returning(
+                  List.of(
+                      List.of(
+                          Values.of(
+                              CustomValueSerializers.getDocumentIdValue(
+                                  DocumentId.fromString("doc1"))),
+                          Values.of(tx_id),
+                          Values.of(doc1))));
+
+      String collectionUpdateCql = UPDATE.formatted(KEYSPACE_NAME, COLLECTION_NAME);
+      JsonNode jsonNode = objectMapper.readTree(doc1);
+      WritableShreddedDocument shredDocument = shredder.shred(jsonNode);
+      DBFilterBase.IDFilter filter =
+          new DBFilterBase.IDFilter(
+              DBFilterBase.IDFilter.Operator.EQ, DocumentId.fromString("doc1"));
+      FindOperation findOperation =
+          new FindOperation(
+              COMMAND_CONTEXT,
+              List.of(filter),
+              DocumentProjector.identityProjector(),
+              null,
+              1,
+              1,
+              ReadType.DOCUMENT,
+              objectMapper,
+              null,
+              0,
+              0);
+      String updateClause =
+          """
+                       { "$set" : { "username" : "user1" }}
+                  """;
+      DocumentUpdater documentUpdater =
+          DocumentUpdater.construct(objectMapper.readValue(updateClause, UpdateClause.class));
+      ReadAndUpdateOperation operation =
+          new ReadAndUpdateOperation(
+              COMMAND_CONTEXT,
+              findOperation,
+              documentUpdater,
+              true,
+              false,
+              false,
+              shredder,
+              DocumentProjector.identityProjector(),
+              1,
+              3);
+
+      Supplier<CommandResult> execute =
+          operation
+              .execute(queryExecutor)
+              .subscribe()
+              .withSubscriber(UniAssertSubscriber.create())
+              .awaitItem()
+              .getItem();
+
+      // assert query execution
+      selectQueryAssert.assertExecuteCount().isOne();
+
+      // then result
+      CommandResult result = execute.get();
+      assertThat(result.status())
+          .hasSize(2)
+          .containsEntry(CommandStatus.MATCHED_COUNT, 1)
+          .containsEntry(CommandStatus.MODIFIED_COUNT, 0);
+      assertThat(result.errors()).isNull();
+      assertThat(result.data().docs()).hasSize(1);
+    }
+
     @Test
     public void happyPathWithSort() throws Exception {
       String collectionReadCql =