diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java index 048f65c3c1..d8aa880760 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java @@ -175,10 +175,11 @@ private Uni processUpdate( } } - // otherwise shred - JsonNode updatedDocument = documentUpdaterResponse.document(); final WritableShreddedDocument writableShreddedDocument = - shredder().shred(updatedDocument, readDocument.txnId()); + shredder().shred(documentUpdaterResponse.document(), readDocument.txnId()); + + // Have to do this because shredder adds _id field to the document if it doesn't exist + JsonNode updatedDocument = writableShreddedDocument.docJsonNode(); // update the document return updatedDocument(queryExecutor, writableShreddedDocument) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java index 8316e62f66..026cd563d2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java @@ -92,7 +92,7 @@ public WritableShreddedDocument shred(JsonNode doc, UUID txId) { validateDocument(documentLimits, docWithId, docJson); final WritableShreddedDocument.Builder b = - WritableShreddedDocument.builder(new DocValueHasher(), docId, txId, docJson); + WritableShreddedDocument.builder(new DocValueHasher(), docId, txId, docJson, docWithId); // And now let's traverse the document, _including DocumentId so it will also // be indexed along with other fields. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/WritableShreddedDocument.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/WritableShreddedDocument.java index de059484ab..7a0a5e4661 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/WritableShreddedDocument.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/WritableShreddedDocument.java @@ -29,6 +29,7 @@ public record WritableShreddedDocument( /** Optional transaction id used for optimistic locking */ UUID txID, String docJson, + JsonNode docJsonNode, Set existKeys, Map subDocEquals, Map arraySize, @@ -39,8 +40,9 @@ public record WritableShreddedDocument( Map queryTextValues, Map queryTimestampValues, Set queryNullValues) { - public static Builder builder(DocValueHasher hasher, DocumentId id, UUID txID, String docJson) { - return new Builder(hasher, id, txID, docJson); + public static Builder builder( + DocValueHasher hasher, DocumentId id, UUID txID, String docJson, JsonNode docJsonNode) { + return new Builder(hasher, id, txID, docJson, docJsonNode); } /** @@ -58,6 +60,7 @@ public static class Builder implements ShredListener { private final UUID txID; private final String docJson; + private final JsonNode docJsonNode; private final Set existKeys; @@ -73,12 +76,13 @@ public static class Builder implements ShredListener { private Map queryTimestampValues; private Set queryNullValues; - public Builder(DocValueHasher hasher, DocumentId id, UUID txID, String docJson) { + public Builder( + DocValueHasher hasher, DocumentId id, UUID txID, String docJson, JsonNode docJsonNode) { this.hasher = hasher; this.id = id; this.txID = txID; this.docJson = Objects.requireNonNull(docJson); - + this.docJsonNode = docJsonNode; existKeys = new LinkedHashSet<>(); // retain document order } @@ -92,6 +96,7 @@ public WritableShreddedDocument build() { id, txID, docJson, + docJsonNode, existKeys, _nonNull(subDocEquals), _nonNull(arraySize), diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java index 07f4a45a03..d798eb3d31 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java @@ -95,7 +95,7 @@ private boolean replace(ObjectNode docToUpdate, boolean docInserted) { } // remove all data and add _id as first field docToUpdate.removeAll(); - docToUpdate.set(DocumentConstants.Fields.DOC_ID, idNode); + if (idNode != null) docToUpdate.set(DocumentConstants.Fields.DOC_ID, idNode); docToUpdate.setAll(replaceDocument()); // return modified flag as true return true; 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 index 7ce343fe30..a218d57dea 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndReplaceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndReplaceIntegrationTest.java @@ -3,7 +3,9 @@ import static io.restassured.RestAssured.given; import static io.stargate.sgv2.common.IntegrationTestUtils.getAuthToken; import static net.javacrumbs.jsonunit.JsonMatchers.jsonEquals; +import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import io.quarkus.test.common.QuarkusTestResource; @@ -329,6 +331,55 @@ public void withUpsert() { .body("data.documents[0]", jsonEquals(expected)); } + @Test + public void withUpsertNoId() { + String json = + """ + { + "findOneAndReplace": { + "filter" : {"username" : "username2"}, + "replacement" : {"username": "username2", "status" : true }, + "options" : {"returnDocument" : "after", "upsert" : true} + } + } + """; + 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.document._id", is(notNullValue())) + .body("data.document._id", any(String.class)) + .body("status.matchedCount", is(0)) + .body("status.modifiedCount", is(0)) + .body("status.upsertedId", is(notNullValue())) + .body("status.upsertedId", any(String.class)) + .body("errors", is(nullValue())); + + // assert state after update + json = + """ + { + "find": { + "filter" : {"username" : "username2"} + } + } + """; + 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.documents[0]._id", is(notNullValue())) + .body("data.documents[0]._id", any(String.class)); + } + @Test public void byIdWithDifferentId() { String document = 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 index b2412adc6f..334983cead 100644 --- 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.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -326,7 +327,9 @@ public void byColumnUpsert() { .then() .statusCode(200) .body("data.document", is(notNullValue())) + .body("data.document._id", any(String.class)) .body("status.upsertedId", is(notNullValue())) + .body("status.upsertedId", any(String.class)) .body("status.matchedCount", is(0)) .body("status.modifiedCount", is(0)) .body("errors", is(nullValue())); @@ -348,7 +351,8 @@ public void byColumnUpsert() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("data.documents[0]", is(notNullValue())); + .body("data.documents[0]", is(notNullValue())) + .body("data.documents[0]._id", any(String.class)); } @Test