From e4ceeca0a9ecd9f668f308f2b0d7375dce59b5d4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 30 Mar 2023 17:47:17 -0700 Subject: [PATCH 1/3] Start work on #325: add improved ITs (some failing initially) --- .../v1/FindOneAndUpdateIntegrationTest.java | 261 ++++++++++++++++-- 1 file changed, 239 insertions(+), 22 deletions(-) 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 13ad60d9ad..1074abbd03 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 @@ -140,27 +140,15 @@ public void emptyOptionsAllowed() { @Test public void byIdReturnDocumentAfter() { - String document = + insertDoc( """ { "_id": "afterDoc3", "username": "afterUser3", "active_user" : true } - """; - insertDoc(document); - - String json = - """ - { - "findOneAndUpdate": { - "filter" : {"_id" : "afterDoc3"}, - "update" : {"$set" : {"active_user": false}}, - "options" : {"returnDocument" : "after"} - } - } - """; - String expected = + """); + final String expected = """ { "_id":"afterDoc3", @@ -171,7 +159,16 @@ public void byIdReturnDocumentAfter() { given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) - .body(json) + .body( + """ + { + "findOneAndUpdate": { + "filter" : {"_id" : "afterDoc3"}, + "update" : {"$set" : {"active_user": false}}, + "options" : {"returnDocument" : "after"} + } + } + """) .when() .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() @@ -182,23 +179,88 @@ public void byIdReturnDocumentAfter() { .body("errors", is(nullValue())); // assert state after update - json = + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "afterDoc3"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("errors", is(nullValue())) + .body("data.docs[0]", jsonEquals(expected)); + } + + @Test + public void byIdReturnDocumentBefore() { + final String docBefore = """ { - "find": { - "filter" : {"_id" : "afterDoc3"} - } + "_id": "beforeDoc3", + "username": "beforeUser3", + "active_user": true } """; + insertDoc(docBefore); + final String docAfter = + """ + { + "_id":"beforeDoc3", + "username":"beforeUser3", + "active_user":false, + "hits": 1 + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) - .body(json) + .body( + """ + { + "findOneAndUpdate": { + "filter" : {"_id" : "beforeDoc3"}, + "update" : { + "$set" : {"active_user": false}, + "$inc" : {"hits": 1} + }, + "options" : {"returnDocument" : "before"} + } + } + """) .when() .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) - .body("data.docs[0]", jsonEquals(expected)); + .body("data.docs[0]", jsonEquals(docBefore)) + .body("status.matchedCount", is(1)) + .body("status.modifiedCount", is(1)) + .body("errors", is(nullValue())); + + // assert state after update + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "beforeDoc3"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("errors", is(nullValue())) + .body("data.docs[0]", jsonEquals(docAfter)); } @Test @@ -1156,6 +1218,161 @@ public void byIdUpsertAndAddOnInsert() { } } + @Nested + class FindOneAndUpdateWithProjection { + @Test + public void projectionAfterUpdate() { + String document = + """ + { + "_id": "update_doc_projection_after", + "x": 1, + "y": 2, + "z": 3, + "subdoc": { + "a": 4, + "b": 5, + "c": 6 + } + } + """; + insertDoc(document); + + String json = + """ + { + "findOneAndUpdate": { + "filter" : {"_id" : "update_doc_projection_after"}, + "options" : {"returnDocument" : "after"}, + "projection" : { "x":0, "subdoc.c":0 }, + "update" : { + "$unset" : { + "subdoc.a": 1, + "z": 1 + } + } + } + } + """; + 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("status.matchedCount", is(1)) + .body("status.modifiedCount", is(1)) + .body("errors", is(nullValue())); + + // assert state after update, with given projection (which further drops values) + String expected = + """ + { + "_id": "update_doc_projection_after", + "y": 2, + "subdoc": { + "b": 5 + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "update_doc_projection_after"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.docs[0]", jsonEquals(expected)); + } + + @Test + public void projectionBeforeUpdate() { + String document = + """ + { + "_id": "update_doc_projection_before", + "a": 1, + "b": 2, + "c": 3, + "subdoc": { + "x": 4, + "y": 5, + "z": 6 + } + } + """; + insertDoc(document); + + String json = + """ + { + "findOneAndUpdate": { + "filter" : {"_id" : "update_doc_projection_before"}, + "options" : {"returnDocument" : "before"}, + "projection" : { "a":0, "subdoc.z":0 }, + "update" : { + "$unset" : { + "subdoc.x": 1, + "c": 1 + } + } + } + } + """; + 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("status.matchedCount", is(1)) + .body("status.modifiedCount", is(1)) + .body("errors", is(nullValue())); + + // assert state before update, with given projection (so unsets not visible) + String expected = + """ + { + "_id": "update_doc_projection_before", + "b": 2, + "c": 3, + "subdoc": { + "x": 4, + "y": 5 + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "update_doc_projection_before"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.docs[0]", jsonEquals(expected)); + } + } + @AfterEach public void cleanUpData() { deleteAllDocuments(); From af97c130a7b77d4a995de7c968d3143c8ad15fbd Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 30 Mar 2023 18:18:27 -0700 Subject: [PATCH 2/3] Minor fix to DocumentProjector wrt classification of "empty" Projector; also, test so it works as expected. --- .../service/projection/DocumentProjector.java | 7 +++-- .../projection/DocumentProjectorTest.java | 28 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index c0c366e700..d9a0813d22 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -14,8 +14,11 @@ * various {@code find} commands. */ public class DocumentProjector { - /** Pseudo-projector that makes no modifications to documents */ - private static final DocumentProjector IDENTITY_PROJECTOR = new DocumentProjector(null, true); + /** + * No-op projector that does not modify documents. Considered "exclusion" projector since "no + * exclusions" is conceptually what happens ("no inclusions" would drop all content) + */ + private static final DocumentProjector IDENTITY_PROJECTOR = new DocumentProjector(null, false); private final ProjectionLayer rootLayer; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorTest.java index 7e4d9f10b2..b50aaaccd9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorTest.java @@ -265,7 +265,7 @@ public void testSimpleIncludeInArray() throws Exception { @Nested class ProjectorApplyExclusions { @Test - public void testExcludeWithIdIncluded() throws Exception { + public void excludeWithIdIncluded() throws Exception { final JsonNode doc = objectMapper.readTree( """ @@ -315,7 +315,7 @@ public void testExcludeWithIdIncluded() throws Exception { } @Test - public void testExcludeWithIdExcluded() throws Exception { + public void excludeWithIdExcluded() throws Exception { final JsonNode doc = objectMapper.readTree( """ @@ -361,7 +361,7 @@ public void testExcludeWithIdExcluded() throws Exception { } @Test - public void testSimpleExcludeInArray() throws Exception { + public void excludeInArray() throws Exception { JsonNode doc = objectMapper.readTree( """ @@ -395,7 +395,7 @@ public void testSimpleExcludeInArray() throws Exception { } @Test - public void testExcludeSubDoc() throws Exception { + public void excludeInSubDoc() throws Exception { JsonNode doc = objectMapper.readTree( """ @@ -428,5 +428,25 @@ public void testExcludeSubDoc() throws Exception { } """)); } + + // "Empty" Projection is not really inclusion or exclusion, but technically + // let's consider it exclusion for sake of consistency (empty list to exclude + // is same as no Projection applied; empty inclusion would produce no output) + @Test + public void emptyProjectionAsExclude() throws Exception { + final String docJson = + """ + { + "_id": "doc5", + "value": 4 + } + """; + JsonNode doc = objectMapper.readTree(docJson); + DocumentProjector projection = + DocumentProjector.createFromDefinition(objectMapper.readTree("{}")); + assertThat(projection.isInclusion()).isFalse(); + projection.applyProjection(doc); + assertThat(doc).isEqualTo(objectMapper.readTree(docJson)); + } } } From 663ada685cd0495bfe4ae84f62ed68cb4c1e62ba Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 31 Mar 2023 11:17:58 -0700 Subject: [PATCH 3/3] Fix ITs as per Mahesh's help --- .../v1/FindOneAndUpdateIntegrationTest.java | 81 +++++++++++++------ 1 file changed, 55 insertions(+), 26 deletions(-) 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 1074abbd03..70792dd47d 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 @@ -1238,7 +1238,7 @@ public void projectionAfterUpdate() { """; insertDoc(document); - String json = + String updateQuery = """ { "findOneAndUpdate": { @@ -1254,29 +1254,44 @@ public void projectionAfterUpdate() { } } """; + // assert that returned document shows doc AFTER update WITH given projection + String expectedFiltered = + """ + { + "_id": "update_doc_projection_after", + "y": 2, + "subdoc": { + "b": 5 + } + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) - .body(json) + .body(updateQuery) .when() .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) .body("status.matchedCount", is(1)) .body("status.modifiedCount", is(1)) - .body("errors", is(nullValue())); + .body("errors", is(nullValue())) + .body("data.docs[0]", jsonEquals(expectedFiltered)); - // assert state after update, with given projection (which further drops values) - String expected = + // But also that update itself worked ($unset "z" and "subdoc.a") + String expectedUpdated = """ { "_id": "update_doc_projection_after", + "x": 1, "y": 2, "subdoc": { - "b": 5 + "b": 5, + "c": 6 } } - """; + """; + given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) @@ -1292,7 +1307,7 @@ public void projectionAfterUpdate() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) - .body("data.docs[0]", jsonEquals(expected)); + .body("data.docs[0]", jsonEquals(expectedUpdated)); } @Test @@ -1313,7 +1328,7 @@ public void projectionBeforeUpdate() { """; insertDoc(document); - String json = + String updateQuery = """ { "findOneAndUpdate": { @@ -1329,31 +1344,45 @@ public void projectionBeforeUpdate() { } } """; + // assert state before update, with given projection (so unsets not visible) + String expectedFiltered = + """ + { + "_id": "update_doc_projection_before", + "b": 2, + "c": 3, + "subdoc": { + "x": 4, + "y": 5 + } + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) - .body(json) + .body(updateQuery) .when() .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) .body("status.matchedCount", is(1)) .body("status.modifiedCount", is(1)) - .body("errors", is(nullValue())); - - // assert state before update, with given projection (so unsets not visible) - String expected = - """ - { - "_id": "update_doc_projection_before", - "b": 2, - "c": 3, - "subdoc": { - "x": 4, - "y": 5 - } - } - """; + .body("errors", is(nullValue())) + .body("data.docs[0]", jsonEquals(expectedFiltered)); + + // And with updates $unset of c and subdoc.x, but no Projection + String expectedUpdated = + """ + { + "_id": "update_doc_projection_before", + "a": 1, + "b": 2, + "subdoc": { + "y": 5, + "z": 6 + } + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) .contentType(ContentType.JSON) @@ -1369,7 +1398,7 @@ public void projectionBeforeUpdate() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) - .body("data.docs[0]", jsonEquals(expected)); + .body("data.docs[0]", jsonEquals(expectedUpdated)); } }