diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/SetOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/SetOperation.java index 93bf7ea684..174b17b141 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/SetOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/SetOperation.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.util.JsonUtil; import io.stargate.sgv2.jsonapi.util.PathMatch; import io.stargate.sgv2.jsonapi.util.PathMatchLocator; @@ -30,7 +31,10 @@ public static SetOperation constructSet(ObjectNode args) { return construct(args, false, UpdateOperator.SET); } - /** Override method used to create an operation that $sets a single property */ + /** + * Override method used to create an operation that $sets a single property (never used for + * $setOnInsert) + */ public static SetOperation constructSet(String filterPath, JsonNode value) { List additions = new ArrayList<>(); String path = validateUpdatePath(UpdateOperator.SET, filterPath); @@ -51,7 +55,11 @@ private static SetOperation construct( var it = args.fields(); while (it.hasNext()) { var entry = it.next(); - String path = validateUpdatePath(operator, entry.getKey()); + // 19-May-2023, tatu: As per [json-api#433] need to allow _id override on $setOnInsert + String path = entry.getKey(); + if (!onlyOnInsert || !DocumentConstants.Fields.DOC_ID.equals(path)) { + path = validateUpdatePath(operator, path); + } additions.add(new Action(PathMatchLocator.forPath(path), entry.getValue())); } return new SetOperation(additions, onlyOnInsert); 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 334983cead..f0b2b75b44 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 @@ -1261,6 +1261,71 @@ public void byIdUpsertAndAddOnInsert() { .statusCode(200) .body("data.documents[0]", jsonEquals(expected)); } + + @Test + public void useGivenDocIdOnInsert() { + String json = + """ + { + "findOneAndUpdate": { + "filter" : {"_id" : "noSuchItem"}, + "update" : { + "$set" : { + "active_user": true, + "extra": 13 + }, + "$setOnInsert" : { + "_id": "setOnInsertDoc2", + "new_user": true + } + }, + "options" : {"returnDocument" : "after", "upsert" : true} + } + } + """; + + // On Insert (for Upsert) should apply both $set and $setOnInsert + String expected = + """ + { + "_id": "setOnInsertDoc2", + "active_user": true, + "new_user": true, + "extra": 13 + } + """; + 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", jsonEquals(expected)) + .body("status.upsertedId", is("setOnInsertDoc2")) + .body("status.matchedCount", is(0)) + .body("status.modifiedCount", is(0)) + .body("errors", is(nullValue())); + + // And verify that the document was inserted as expected: + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "setOnInsertDoc2"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("data.documents[0]", jsonEquals(expected)); + } } @Nested diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java index 9ce40e446e..aabc1ad725 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java @@ -333,6 +333,63 @@ public void upsert() { .body("data.documents[0]", jsonEquals(expected)); } + @Test + public void upsertWithSetOnInsert() { + insert(2); + String json = + """ + { + "updateMany": { + "filter" : {"_id": "no-such-doc"}, + "update" : { + "$set" : {"active_user": true}, + "$setOnInsert" : {"_id": "docX"} + }, + "options" : {"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("status.upsertedId", is("docX")) + .body("status.matchedCount", is(0)) + .body("status.modifiedCount", is(0)) + .body("status.moreData", nullValue()) + .body("errors", is(nullValue())); + + // assert upsert + String expected = + """ + { + "_id": "docX", + "active_user": true + } + """; + json = + """ + { + "find": { + "filter" : {"_id" : "docX"} + } + } + """; + 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]", jsonEquals(expected)); + } + @Test public void byIdNoChange() { insert(2); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java index ec72d6f236..f1a4f55af0 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java @@ -179,6 +179,62 @@ public void byIdUpsert() { .body("data.documents[0]", jsonEquals(expected)); } + @Test + public void byIdUpsertSetOnInsert() { + String json = + """ + { + "updateOne": { + "filter" : {"_id" : "no-such-doc"}, + "update" : { + "$set" : {"active_user": true}, + "$setOnInsert" : {"_id": "upsertSetOnInsert1"} + }, + "options" : {"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("status.upsertedId", is("upsertSetOnInsert1")) + .body("status.matchedCount", is(0)) + .body("status.modifiedCount", is(0)) + .body("errors", is(nullValue())); + + // assert state after update + json = + """ + { + "find": { + "filter" : {"_id" : "upsertSetOnInsert1"} + } + } + """; + String expected = + """ + { + "_id": "upsertSetOnInsert1", + "active_user": 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.documents[0]", jsonEquals(expected)); + } + @Test public void byColumnUpsert() { String json = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/SetOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/SetOperationTest.java index ea5e1da03e..491b399d53 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/SetOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/SetOperationTest.java @@ -346,7 +346,7 @@ public void testChangeForObjectWithDifferentFieldOrder() { @Nested class InvalidCasesRoot { @Test - public void testNoReplacingDocId() { + public void testNoReplacingDocIdWithSet() { Exception e = catchException( () -> { @@ -363,6 +363,27 @@ public void testNoReplacingDocId() { .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_FOR_DOC_ID) .hasMessage(ErrorCode.UNSUPPORTED_UPDATE_FOR_DOC_ID.getMessage() + ": $set"); } + + // As per [json-api#433]: Ok to override _id for $setOnInsert + @Test + public void testReplaceDocIdWithSetOnInsert() { + UpdateOperation oper = + UpdateOperator.SET_ON_INSERT.resolveOperation( + objectFromJson(""" + { "_id": 1 } + """)); + assertThat(oper).isInstanceOf(SetOperation.class); + // Should indicate document being modified + ObjectNode doc = + objectFromJson(""" + { "_id": 0, "a": 1 } + """); + assertThat(oper.updateDocument(doc)).isTrue(); + assertThat(doc) + .isEqualTo(fromJson(""" + { "_id": 1, "a": 1 } + """)); + } } @Nested