From 22b96c760164851b9d6b8b17ef3b7f64dda354b8 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 10 Feb 2023 13:56:37 -0800 Subject: [PATCH 1/4] Initial implementation of $push + $each --- .../command/clause/update/PushOperation.java | 94 +++++++++++++++---- .../clause/update/UpdateOperation.java | 6 +- .../clause/update/PushOperationTest.java | 59 +++++++++--- 3 files changed, 126 insertions(+), 33 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java index 7a18eb8221..70908e6ad1 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java @@ -30,33 +30,77 @@ public static PushOperation construct(ObjectNode args) { List updates = new ArrayList<>(); while (fieldIter.hasNext()) { Map.Entry entry = fieldIter.next(); - // 06-Feb-2023, tatu: Until "$each" supported, verify that no modifiers included + // At main level we must have field name (no modifiers) + final String name = entry.getKey(); + if (looksLikeModifier(name)) { + throw new JsonApiException( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM, + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage() + + ": $push requires field names at main level, found modifier: " + + name); + } + // But within field value modifiers are allowed: if there's one, all must be modifiers JsonNode value = entry.getValue(); - String firstModifier = findModifier(value); - if (firstModifier != null) { + PushAction action; + if (value.isObject() && hasModifier((ObjectNode) value)) { + action = buildActionWithModifiers(name, (ObjectNode) value); + } else { + action = new PushAction(name, entry.getValue(), false); + } + updates.add(action); + } + return new PushOperation(updates); + } + + private static PushAction buildActionWithModifiers(String propName, ObjectNode actionDef) { + // We know there is at least one modifier; and if so, all must be modifiers. + // For now we only support "$each" + JsonNode eachArg = null; + + Iterator> it = actionDef.fields(); + while (it.hasNext()) { + Map.Entry entry = it.next(); + final String modifier = entry.getKey(); + + // Since we only support "$each" for now + if (!"$each".equals(modifier)) { throw new JsonApiException( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER, ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() - + ": $push does not yet support modifiers; trying to use " - + firstModifier); + + ": $push only supports $each currently; trying to use " + + modifier); + } + // And argument for $each must be an Array + eachArg = entry.getValue(); + if (!eachArg.isArray()) { + throw new JsonApiException( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM, + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage() + + ": $push modifier $each requires ARRAY argument, found: " + + eachArg.getNodeType()); } - updates.add(new PushAction(entry.getKey(), entry.getValue())); } - return new PushOperation(updates); + + // For now should not be possible to occur but once we add other modifiers could: + if (eachArg == null) { + throw new JsonApiException( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM, + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage() + + ": $push modifiers can only be used with $each modifier; none included"); + } + + return new PushAction(propName, eachArg, true); } - private static String findModifier(JsonNode node) { - if (node.isObject()) { - // Sigh. Wish things were not returned as iterators by JsonNode... - Iterator it = node.fieldNames(); - while (it.hasNext()) { - String name = it.next(); - if (name.startsWith("$")) { - return name; - } + private static boolean hasModifier(ObjectNode node) { + // Sigh. Wish things were not returned as iterators by JsonNode... + Iterator it = node.fieldNames(); + while (it.hasNext()) { + if (looksLikeModifier(it.next())) { + return true; } } - return null; + return false; } @Override @@ -65,10 +109,12 @@ public boolean updateDocument(ObjectNode doc) { final String path = update.path; final JsonNode toAdd = update.value; JsonNode node = doc.get(path); + ArrayNode array; + if (node == null) { // No such property? Add new 1-element array - doc.putArray(path).add(toAdd); + array = doc.putArray(path); } else if (node.isArray()) { // Already array? Append - ((ArrayNode) node).add(toAdd); + array = (ArrayNode) node; } else { // Something else? fail throw new JsonApiException( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET, @@ -78,6 +124,14 @@ public boolean updateDocument(ObjectNode doc) { + " of type " + node.getNodeType()); } + // Regular add or $each? + if (update.each) { + for (JsonNode element : toAdd) { + array.add(element); + } + } else { + array.add(toAdd); + } } // Every valid update operation modifies document so need just one: @@ -95,5 +149,5 @@ public boolean equals(Object o) { * Value class for per-field update operations: initially simple replacement but will need * different value type soon to allow {@code $each modifier}. */ - private record PushAction(String path, JsonNode value) {} + private record PushAction(String path, JsonNode value, boolean each) {} } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java index fc4672b202..2adccda09a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java @@ -42,7 +42,7 @@ protected static String validateSetPath(UpdateOperator oper, String path) { * @return Path passed in if valid */ protected static String validateNonModifierPath(UpdateOperator oper, String path) { - if (path.startsWith("$")) { + if (looksLikeModifier(path)) { throw new JsonApiException( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER, ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() @@ -52,4 +52,8 @@ protected static String validateNonModifierPath(UpdateOperator oper, String path } return path; } + + protected static boolean looksLikeModifier(String path) { + return path.startsWith("$"); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java index 6a77f30563..0cdd9743b2 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java @@ -31,7 +31,7 @@ public void testSimplePushToExisting() { { "array" : 32 } """)); assertThat(oper).isInstanceOf(PushOperation.class); - ObjectNode doc = defaultArrayTestDoc(); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson(""" @@ -49,7 +49,7 @@ public void testSimplePushToNonExisting() { { "newArray" : "value" } """)); assertThat(oper).isInstanceOf(PushOperation.class); - ObjectNode doc = defaultArrayTestDoc(); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson( @@ -60,12 +60,54 @@ public void testSimplePushToNonExisting() { } } + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class pushWithEachCases { + @Test + public void withEachToExisting() { + UpdateOperation oper = + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "array" : { "$each" : [ 17, false ] } } + """)); + assertThat(oper).isInstanceOf(PushOperation.class); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); + assertThat(oper.updateDocument(doc)).isTrue(); + ObjectNode expected = + objectFromJson( + """ + { "a" : 1, "array" : [ true, 17, false ] } + """); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void withEachToNonExisting() { + UpdateOperation oper = + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "newArray" : { "$each" : [ -50, "abc" ] } } + """)); + assertThat(oper).isInstanceOf(PushOperation.class); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); + assertThat(oper.updateDocument(doc)).isTrue(); + ObjectNode expected = + objectFromJson( + """ + { "a" : 1, "array" : [ true ], "newArray" : [ -50, "abc" ] } + """); + assertThat(doc).isEqualTo(expected); + } + } + @Nested @TestMethodOrder(MethodOrderer.OrderAnnotation.class) class invalidCases { @Test public void testPushOnNonArrayProperty() { - ObjectNode doc = defaultArrayTestDoc(); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); UpdateOperation oper = UpdateOperator.PUSH.resolveOperation( objectFromJson(""" @@ -93,7 +135,7 @@ public void testPushWithUnknownModifier() { UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "a" : { "$each" : [ 1, 2, 3 ] } } + { "a" : { "$sort" : { "field": 1 } } } """)); }); assertThat(e) @@ -101,14 +143,7 @@ public void testPushWithUnknownModifier() { .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER) .hasMessage( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() - + ": $push does not yet support modifiers; trying to use $each"); + + ": $push only supports $each currently; trying to use $sort"); } } - - ObjectNode defaultArrayTestDoc() { - return objectFromJson( - """ - { "a" : 1, "array" : [ true ] } - """); - } } From 3b542b97179b21350a9628f7a08314adcd063581 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 10 Feb 2023 14:15:55 -0800 Subject: [PATCH 2/4] Add unit tests to validate $push + $each --- .../command/clause/update/PushOperation.java | 5 +- .../clause/update/PushOperationTest.java | 145 +++++++++++++----- 2 files changed, 110 insertions(+), 40 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java index 70908e6ad1..6fa13e029d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/PushOperation.java @@ -67,8 +67,9 @@ private static PushAction buildActionWithModifiers(String propName, ObjectNode a throw new JsonApiException( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER, ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() - + ": $push only supports $each currently; trying to use " - + modifier); + + ": $push only supports $each currently; trying to use '" + + modifier + + "'"); } // And argument for $each must be an Array eachArg = entry.getValue(); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java index 0cdd9743b2..7c60e8432c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java @@ -22,21 +22,23 @@ public class PushOperationTest extends UpdateOperationTestBase { @Nested @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - class happyPath { + class BasicPushHappyPath { @Test public void testSimplePushToExisting() { UpdateOperation oper = UpdateOperator.PUSH.resolveOperation( - objectFromJson(""" - { "array" : 32 } - """)); + objectFromJson( + """ + { "array" : 32 } + """)); assertThat(oper).isInstanceOf(PushOperation.class); ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = - objectFromJson(""" - { "a" : 1, "array" : [ true, 32 ] } - """); + objectFromJson( + """ + { "a" : 1, "array" : [ true, 32 ] } + """); assertThat(doc).isEqualTo(expected); } @@ -46,39 +48,106 @@ public void testSimplePushToNonExisting() { UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "newArray" : "value" } - """)); + { "newArray" : "value" } + """)); assertThat(oper).isInstanceOf(PushOperation.class); ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson( """ - { "a" : 1, "array" : [ true ], "newArray" : [ "value" ] } - """); + { "a" : 1, "array" : [ true ], "newArray" : [ "value" ] } + """); assertThat(doc).isEqualTo(expected); } } @Nested @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - class pushWithEachCases { + class BasicPushInvalidCases { + @Test + public void testPushOnNonArrayProperty() { + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); + UpdateOperation oper = + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "a" : 57 } + """)); + Exception e = + catchException( + () -> { + oper.updateDocument(doc); + }); + assertThat(e) + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET) + .hasMessageStartingWith( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET.getMessage() + + ": $push requires target to be Array"); + } + + // Test to make sure we know to look for "$"-qualifiers even if not yet supporting them? + @Test + public void testPushWithUnknownModifier() { + Exception e = + catchException( + () -> { + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "a" : { "$sort" : { "field": 1 } } } + """)); + }); + assertThat(e) + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER) + .hasMessage( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() + + ": $push only supports $each currently; trying to use '$sort'"); + } + + // Modifiers, if any, must be "under" field name, not at main level (and properties + // cannot have names starting with "$") + @Test + public void testPushWithMisplacedModifier() { + Exception e = + catchException( + () -> { + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "$each" : [ 1, 2 ] } + """)); + }); + assertThat(e) + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM) + .hasMessage( + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage() + + ": $push requires field names at main level, found modifier: $each"); + } + } + + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class PushWithEachHappyCases { @Test public void withEachToExisting() { UpdateOperation oper = UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "array" : { "$each" : [ 17, false ] } } - """)); + { "array" : { "$each" : [ 17, false ] } } + """)); assertThat(oper).isInstanceOf(PushOperation.class); ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson( """ - { "a" : 1, "array" : [ true, 17, false ] } - """); + { "a" : 1, "array" : [ true, 17, false ] } + """); assertThat(doc).isEqualTo(expected); } @@ -88,62 +157,62 @@ public void withEachToNonExisting() { UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "newArray" : { "$each" : [ -50, "abc" ] } } - """)); + { "newArray" : { "$each" : [ -50, "abc" ] } } + """)); assertThat(oper).isInstanceOf(PushOperation.class); ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson( """ - { "a" : 1, "array" : [ true ], "newArray" : [ -50, "abc" ] } - """); + { "a" : 1, "array" : [ true ], "newArray" : [ -50, "abc" ] } + """); assertThat(doc).isEqualTo(expected); } } @Nested @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - class invalidCases { + class PushWithEachInvalidCases { + // Argument for "$each" must be an Array @Test - public void testPushOnNonArrayProperty() { - ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); - UpdateOperation oper = - UpdateOperator.PUSH.resolveOperation( - objectFromJson(""" - { "a" : 57 } - """)); + public void nonArrayParamForEach() { Exception e = catchException( () -> { - oper.updateDocument(doc); + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "array" : { "$each" : 365 } } + """)); }); assertThat(e) .isInstanceOf(JsonApiException.class) - .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM) .hasMessageStartingWith( - ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET.getMessage() - + ": $push requires target to be Array"); + ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage() + + ": $push modifier $each requires ARRAY argument, found: NUMBER"); } - // Test to make sure we know to look for "$"-qualifiers even if not yet supporting them? + // If there is one modifier for given field, all Object properties must be (supported) + // modifiers: @Test - public void testPushWithUnknownModifier() { + public void nonModifierParamForEach() { Exception e = catchException( () -> { UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "a" : { "$sort" : { "field": 1 } } } - """)); + { "array" : { "$each" : [ 1, 2, 3 ], "value" : 3 } } + """)); }); assertThat(e) .isInstanceOf(JsonApiException.class) .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER) - .hasMessage( + .hasMessageStartingWith( ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage() - + ": $push only supports $each currently; trying to use $sort"); + + ": $push only supports $each currently; trying to use 'value'"); } } } From cafffe6b5c34c7943d67561973496539a210275d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 10 Feb 2023 14:43:55 -0800 Subject: [PATCH 3/4] Add basic IT as well --- .../api/v1/FindAndUpdateIntegrationTest.java | 144 +++++++++++++----- 1 file changed, 103 insertions(+), 41 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java index 6452c766df..ddf9ad7a1e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java @@ -540,22 +540,30 @@ public void findByColumnAndSetSubDoc() { .statusCode(200) .body("data.docs[0]", jsonEquals(expected)); } + } + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class UpdateOneWithPush { @Test @Order(2) - public void findByColumnAndPushToArray() { + public void findByColumnAndPush() { + insertDoc( + """ + { + "_id": "update_doc_push", + "array": [ 2 ] + } + """); String json = """ { - "insertOne": { - "document": { - "_id": "update_doc_push", - "array": [ 2 ] - } + "updateOne": { + "filter" : {"_id" : "update_doc_push"}, + "update" : {"$push" : {"array": 13 }} } } """; - given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) @@ -563,13 +571,50 @@ public void findByColumnAndPushToArray() { .when() .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() - .statusCode(200); + .statusCode(200) + .body("status.updatedIds[0]", is("update_doc_push")); + + String expected = "{\"_id\":\"update_doc_push\", \"array\": [2, 13]}"; json = + """ + { + "find": { + "filter" : {"_id" : "update_doc_push"} + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.docs[0]", jsonEquals(expected)); + } + + @Test + @Order(2) + public void findByColumnAndPushWithEach() { + insertDoc( + """ + { + "_id": "update_doc_push_each", + "array": [ 1 ] + } + """); + String json = """ { "updateOne": { - "filter" : {"_id" : "update_doc_push"}, - "update" : {"$push" : {"array": 13 }} + "filter" : {"_id" : "update_doc_push_each"}, + "update" : { + "$push" : { + "array": { "$each" : [ 2, 3 ] }, + "newArray": { "$each" : [ true ] } + } + } } } """; @@ -581,17 +626,22 @@ public void findByColumnAndPushToArray() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) - .body("status.updatedIds[0]", is("update_doc_push")); + .body("status.updatedIds[0]", is("update_doc_push_each")); - String expected = "{\"_id\":\"update_doc_push\", \"array\": [2, 13]}"; + String expected = + """ + { "_id":"update_doc_push_each", + "array": [1, 2, 3], + "newArray": [true] } + """; json = """ - { - "find": { - "filter" : {"_id" : "update_doc_push"} - } - } - """; + { + "find": { + "filter" : {"_id" : "update_doc_push_each"} + } + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) @@ -602,30 +652,21 @@ public void findByColumnAndPushToArray() { .statusCode(200) .body("data.docs[0]", jsonEquals(expected)); } + } + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class UpdateOneWithInc { @Test @Order(2) public void findByColumnAndInc() { - String doc = + insertDoc( """ - { - "insertOne": { - "document": { - "_id": "update_doc_inc", - "number": 123 - } - } + { + "_id": "update_doc_inc", + "number": 123 } - """; - - given() - .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) - .contentType(ContentType.JSON) - .body(doc) - .when() - .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) - .then() - .statusCode(200); + """); String updateJson = """ { @@ -648,12 +689,12 @@ public void findByColumnAndInc() { String expectedDoc = "{\"_id\":\"update_doc_inc\", \"number\": 119, \"newProp\": 0.25 }"; String findJson = """ - { - "find": { - "filter" : {"_id" : "update_doc_inc"} + { + "find": { + "filter" : {"_id" : "update_doc_inc"} + } } - } - """; + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) @@ -665,4 +706,25 @@ public void findByColumnAndInc() { .body("data.docs[0]", jsonEquals(expectedDoc)); } } + + private void insertDoc(String docJson) { + String doc = + """ + { + "insertOne": { + "document": %s + } + } + """ + .formatted(docJson); + + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(doc) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200); + } } From b5f6497baca588d855b2f29091d57b8750d6d29b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 10 Feb 2023 14:53:19 -0800 Subject: [PATCH 4/4] Add 2 more unit tests based on code review suggestions --- .../clause/update/PushOperationTest.java | 44 +++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java index 7c60e8432c..ea387d9848 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/PushOperationTest.java @@ -157,15 +157,53 @@ public void withEachToNonExisting() { UpdateOperator.PUSH.resolveOperation( objectFromJson( """ - { "newArray" : { "$each" : [ -50, "abc" ] } } - """)); + { "newArray" : { "$each" : [ -50, "abc" ] } } + """)); assertThat(oper).isInstanceOf(PushOperation.class); ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ true ] }"); assertThat(oper.updateDocument(doc)).isTrue(); ObjectNode expected = objectFromJson( """ - { "a" : 1, "array" : [ true ], "newArray" : [ -50, "abc" ] } + { "a" : 1, "array" : [ true ], "newArray" : [ -50, "abc" ] } + """); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void withEachNestedArray() { + UpdateOperation oper = + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "array" : { "$each" : [ [ 1, 2], [ 3 ] ] } } + """)); + assertThat(oper).isInstanceOf(PushOperation.class); + ObjectNode doc = objectFromJson("{ \"a\" : 1, \"array\" : [ null ] }"); + assertThat(oper.updateDocument(doc)).isTrue(); + ObjectNode expected = + objectFromJson( + """ + { "a" : 1, "array" : [ null, [ 1, 2 ], [ 3 ] ] } + """); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void withEachNestedArrayNonExisting() { + UpdateOperation oper = + UpdateOperator.PUSH.resolveOperation( + objectFromJson( + """ + { "array" : { "$each" : [ [ 1, 2], [ 3 ] ] } } + """)); + assertThat(oper).isInstanceOf(PushOperation.class); + ObjectNode doc = objectFromJson("{ \"x\" : 1 }"); + assertThat(oper.updateDocument(doc)).isTrue(); + ObjectNode expected = + objectFromJson( + """ + { "x" : 1, "array" : [ [ 1, 2 ], [ 3 ] ] } """); assertThat(doc).isEqualTo(expected); }