From f85b557e734ac315946c59dce30fb70609390393 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 5 Sep 2023 16:44:43 -0700 Subject: [PATCH 1/3] Fix #521: allow hyphen in names, prevent empty String --- docs/jsonapi-spec.md | 2 +- .../config/constants/DocumentConstants.java | 2 +- .../jsonapi/service/shredding/Shredder.java | 7 ++ .../shredding/ShredderDocLimitsTest.java | 67 +++++++++++++------ 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/docs/jsonapi-spec.md b/docs/jsonapi-spec.md index 5acbe15b8d..1d09693805 100644 --- a/docs/jsonapi-spec.md +++ b/docs/jsonapi-spec.md @@ -256,7 +256,7 @@ the rules for user defined field names. = _id - ::= ["a-zA-Z0-9_"]+ + ::= ["a-zA-Z0-9_-"]+ ``` The `_id` field is a "reserved name" and is always interpreted as the diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java b/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java index bc26e835e4..d1fbb6d574 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java @@ -24,7 +24,7 @@ interface Fields { // Current definition of valid JSON API names: note that this only validates // characters, not length limits (nor empty nor "too long" allowed but validated // separately) - Pattern VALID_NAME_PATTERN = Pattern.compile("[a-zA-Z0-9_]*"); + Pattern VALID_NAME_PATTERN = Pattern.compile("[a-zA-Z0-9_\\-]*"); } interface KeyTypeId { 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 ed538c5d21..fab9e9bb4f 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 @@ -294,6 +294,13 @@ private void validateObjectKey( key.length(), limits.maxPropertyNameLength())); } + if (key.length() == 0) { + // NOTE: validity failure, not size limit + throw new JsonApiException( + ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION, + String.format( + "%s: empty names not allowed", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage())); + } if (!DocumentConstants.Fields.VALID_NAME_PATTERN.matcher(key).matches()) { // Special names are accepted in some cases: for now only one such case for // Date values -- if more needed, will refactor. But for now inline: diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java index 1f626943de..775d146b0c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java @@ -208,25 +208,6 @@ public void catchTooLongNames() { + ")"); } - @ParameterizedTest - @ValueSource(strings = {"$function", "dot.ted", "index[1]"}) - public void catchInvalidFieldName(String invalidName) { - final ObjectNode doc = objectMapper.createObjectNode(); - doc.put("_id", 123); - doc.put(invalidName, 123456); - - Exception e = catchException(() -> shredder.shred(doc)); - assertThat(e) - .isNotNull() - .isInstanceOf(JsonApiException.class) - .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION) - .hasMessageStartingWith(ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage()) - .hasMessageEndingWith( - "Document key name constraints violated: Property name ('" - + invalidName - + "') contains character(s) not allowed"); - } - @Test public void allowNotTooLongStringValues() { final ObjectNode doc = objectMapper.createObjectNode(); @@ -280,6 +261,54 @@ public void catchTooLongNumberValues() throws Exception { } } + // Tests for size of atomic value / name length violations + @Nested + class ValidationDocNameViolations { + @ParameterizedTest + @ValueSource(strings = {"name", "a123", "snake_case", "camelCase", "ab-cd-ef"}) + public void allowRegularFieldNames(String validName) { + final ObjectNode doc = objectMapper.createObjectNode(); + doc.put("_id", 123); + doc.put(validName, 123456); + + // Enough to verify that shredder does not throw exception + assertThat(shredder.shred(doc)).isNotNull(); + } + + @Test + public void catchEmptyFieldName() { + final ObjectNode doc = objectMapper.createObjectNode(); + doc.put("", 123456); + + Exception e = catchException(() -> shredder.shred(doc)); + assertThat(e) + .isNotNull() + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION) + .hasMessageStartingWith(ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage()) + .hasMessageEndingWith("Document key name constraints violated: empty names not allowed"); + } + + @ParameterizedTest + @ValueSource(strings = {"$function", "dot.ted", "index[1]", "a/b", "a\\b"}) + public void catchInvalidFieldName(String invalidName) { + final ObjectNode doc = objectMapper.createObjectNode(); + doc.put("_id", 123); + doc.put(invalidName, 123456); + + Exception e = catchException(() -> shredder.shred(doc)); + assertThat(e) + .isNotNull() + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION) + .hasMessageStartingWith(ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage()) + .hasMessageEndingWith( + "Document key name constraints violated: Property name ('" + + invalidName + + "') contains character(s) not allowed"); + } + } + private ObjectNode docWithNArrayElems(String propName, int count) { final ObjectNode doc = objectMapper.createObjectNode(); doc.put("_id", 123); From 2515eba66ab085de039c8f4b94a48f36e26b86bb Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 11 Sep 2023 10:29:30 -0700 Subject: [PATCH 2/3] Add Insert IT with hyphen field --- .../jsonapi/api/v1/InsertIntegrationTest.java | 81 +++++++++++++++---- 1 file changed, 66 insertions(+), 15 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java index 55dfe7cd15..b5ae557d0d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java @@ -67,22 +67,47 @@ public void insertDocument() { .body("status.insertedIds[0]", is("doc3")) .body("data", is(nullValue())) .body("errors", is(nullValue())); + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "doc3"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body( + "data.documents[0]", + jsonEquals( + """ + { + "_id":"doc3", + "username":"user3" + } + """)) + .body("errors", is(nullValue())); + } - json = - """ - { - "find": { - "filter" : {"_id" : "doc3"} - } - } - """; - String expected = + // [https://github.com/stargate/jsonapi/issues/521]: allow hyphens in property names + @Test + public void insertDocumentWithHyphen() { + String json = """ - { - "_id":"doc3", - "username":"user3" - } - """; + { + "insertOne": { + "document": { + "_id": "doc-hyphen", + "user-name": "user #1" + } + } + } + """; given() .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) @@ -92,7 +117,33 @@ public void insertDocument() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("data.documents[0]", jsonEquals(expected)) + .body("status.insertedIds[0]", is("doc-hyphen")) + .body("data", is(nullValue())) + .body("errors", is(nullValue())); + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"_id" : "doc-hyphen"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body( + "data.documents[0]", + jsonEquals( + """ + { + "_id": "doc-hyphen", + "user-name": "user #1" + } + """)) .body("errors", is(nullValue())); } From 3b028770ee0ada87d8554928decdcb2ba0237a86 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 11 Sep 2023 10:42:28 -0700 Subject: [PATCH 3/3] Add find()-test wrt hyphenated column name as well --- .../jsonapi/api/v1/FindIntegrationTest.java | 36 +++++++++++++++++-- .../jsonapi/api/v1/InsertIntegrationTest.java | 2 +- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index 65622a53bb..d8312b57c4 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -112,7 +112,7 @@ public void setUp() { "insertOne": { "document": { "_id": {"$date": 6}, - "username": "user6" + "user-name": "user6" } } } @@ -227,7 +227,7 @@ public void byDateId() { """ { "_id": {"$date": 6}, - "username": "user6" + "user-name": "user6" } """; given() @@ -466,6 +466,38 @@ public void byColumn() { .body("data.documents", hasSize(1)); } + // [https://github.com/stargate/jsonapi/issues/521]: allow hyphens in property names + @Test + public void byColumnWithHyphen() { + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body( + """ + { + "find": { + "filter" : {"user-name" : "user6"} + } + } + """) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", hasSize(1)) + .body( + "data.documents[0]", + jsonEquals( + """ + { + "_id": {"$date": 6}, + "user-name": "user6" + } + """)); + } + @Test public void withEqComparisonOperator() { String json = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java index b5ae557d0d..df55a4ffd5 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java @@ -96,7 +96,7 @@ public void insertDocument() { // [https://github.com/stargate/jsonapi/issues/521]: allow hyphens in property names @Test - public void insertDocumentWithHyphen() { + public void insertDocumentWithHyphenatedColumn() { String json = """ {