Skip to content
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

Fix #433: allow _id with $setOnInsert #434

Merged
merged 3 commits into from
May 22, 2023
Merged
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 @@ -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;
Expand Down Expand Up @@ -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<Action> additions = new ArrayList<>();
String path = validateUpdatePath(UpdateOperator.SET, filterPath);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ public void testChangeForObjectWithDifferentFieldOrder() {
@Nested
class InvalidCasesRoot {
@Test
public void testNoReplacingDocId() {
public void testNoReplacingDocIdWithSet() {
Exception e =
catchException(
() -> {
Expand All @@ -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
Expand Down