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.

Sign up for GitHub

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 #521: allow hyphen in names, prevent empty String #522

Merged
merged 4 commits into from
Sep 11, 2023
Merged

Fix #521: allow hyphen in names, prevent empty String #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f85b557
Fix #521: allow hyphen in names, prevent empty String
tatu-at-datastax Sep 5, 2023
27f5002
Merge branch 'main' into tatu/521-allow-hyphen-in-names
tatu-at-datastax Sep 11, 2023
2515eba
Add Insert IT with hyphen field
tatu-at-datastax Sep 11, 2023
3b02877
Add find()-test wrt hyphenated column name as well
tatu-at-datastax Sep 11, 2023
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
2 changes: 1 addition & 1 deletion docs/jsonapi-spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ the rules for user defined field names.
<user-field-name>

<id-field-name> = _id
<user-field-name> ::= ["a-zA-Z0-9_"]+
<user-field-name> ::= ["a-zA-Z0-9_-"]+
```

The `_id` field is a "reserved name" and is always interpreted as the
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 34 additions & 2 deletions src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void setUp() {
"insertOne": {
"document": {
"_id": {"$date": 6},
"username": "user6"
"user-name": "user6"
}
}
}
Expand Down Expand Up @@ -227,7 +227,7 @@ public void byDateId() {
"""
{
"_id": {"$date": 6},
"username": "user6"
"user-name": "user6"
}
""";
given()
Expand Down Expand Up @@ -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 =
Expand Down
81 changes: 66 additions & 15 deletions src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,47 @@ public void insertDocument() {
.body("status.insertedIds[0]", is("doc3"))
.body("data", is(nullValue()))
.body("errors", is(nullValue()));
given()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing not change, just inlined JSON payloads for bit more compact code.

.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 insertDocumentWithHyphenatedColumn() {
String json =
"""
{
"_id":"doc3",
"username":"user3"
}
""";
{
"insertOne": {
"document": {
"_id": "doc-hyphen",
"user-name": "user #1"
}
}
}
""";

given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
Expand All @@ -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()));
}

Expand Down
67 changes: 48 additions & 19 deletions src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down