Skip to content

Commit

Permalink
Fix #91: remove escaping/decorating of dotted path Shredder produces
Browse files Browse the repository at this point in the history
  • Loading branch information
@tatu-at-datastax
tatu-at-datastax committed Feb 9, 2023
1 parent 6177870 commit 1b30a3f
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 86 deletions.
58 changes: 15 additions & 43 deletions src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPath.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,28 @@

/**
* Immutable value class used as key for entries in Shredded document representations: constructed
* from either nested path in input documents (during "shredding"), or when "un-shredding" back to
* Document from Shredded representations.
* from nested path in input documents (during "shredding") to match "dot notation" path used
* for query filtering (and possibly projection)
*
* <p>Internally path is simply expressed as a {@link String} where segments are separated by comma
* ({@code "."}) and segments themselves are either:
*
* <ul>
* <li>Escaped Object property name (see below about escaping)
* <li>Decorated Array element index (see below for details)
* <li>Object property name (see below about escaping)
* <li>Array element index (see below for details)
* </ul>
*
* <p>Array element indexes are enclosed in brackets, so the first element's segment would be
* expressed as String {@code "[0]"}. Property names are included as is -- so property {@code
* "name"} has segment {@code name} -- with the exception that due to need to distinguish
* encoding-characters comma and opening bracket, escaping is needed. Backslash character {@code
* '\\'} is used for escaping, preceding character to escape. Backslash itself also needs escaping.
* This means that property name like {@code "a.b"} is encoded as {@code "a\\.b"}.
* <p>No escaping is used so path itself is ambiguous (segment "1" can mean
* either Array index #1 or Object property "1") without context document, but
* will NOT be ambiguous when applied to specific target document where context
* node's type (Array or Object) is known.
*
* <p>As a simple example consider following JSON document:
*
* <pre>
* { "name" : "Bob",
* "values" : [ 1, 2 ],
* "[extra.stuff]" : true
* "extra.stuff" : true
* }
* </pre>
*
Expand All @@ -38,9 +36,9 @@
* <ul>
* <li>{@code "name"}
* <li>{@code "values"}
* <li>{@code "values.[0]"}
* <li>{@code "values.[1]"}
* <li>{@code "\\[extra\\.stuff]"}
* <li>{@code "values.0"}
* <li>{@code "values.1"}
* <li>{@code "extra.stuff"}
* </ul>
*
* <p>Instances can be sorted; sorting order uses underlying encoded path value.
Expand Down Expand Up @@ -121,37 +119,11 @@ public Builder nestedValueBuilder() {
}

public Builder property(String propName) {
// Properties trickier since need to escape '.', '[' and `\`
final String encodedProp = encodePropertyName(propName);
childPath = (basePath == null) ? encodedProp : (basePath + '.' + encodedProp);
// Simple as we no longer apply escaping:
childPath = (basePath == null) ? propName : (basePath + '.' + propName);
return this;
}

static String encodePropertyName(String propName) {
// First: loop through to see if anything to escape; if not, can return as-is
final int len = propName.length();
int i = 0;
for (; i < len; ++i) {
char c = propName.charAt(i);
if (c == '.' || c == '[' || c == '\\') {
break;
}
}
// common case: nothing to escape
if (i == len) {
return propName;
}
StringBuilder sb = new StringBuilder(len + 3);
for (i = 0; i < len; ++i) {
char c = propName.charAt(i);
if (c == '.' || c == '[' || c == '\\') {
sb.append('\\');
}
sb.append(c);
}
return sb.toString();
}

public Builder index(int index) {
// Indexes are easy as no escaping needed
StringBuilder sb;
Expand All @@ -161,7 +133,7 @@ public Builder index(int index) {
} else {
sb = new StringBuilder(basePath).append('.');
}
childPath = sb.append('[').append(index).append(']').toString();
childPath = sb.append(index).toString();
return this;
}

Expand Down
48 changes: 11 additions & 37 deletions src/test/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPathTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,6 @@

// Simple unit test with no injection needed:
public class JsonPathTest {
@Nested
class EncodePropertyName {
@Test
public void encodePlain() {
assertThat(JsonPath.Builder.encodePropertyName("simple")).isEqualTo("simple");
}

@Test
public void encodeDots() {
assertThat(JsonPath.Builder.encodePropertyName("with.dot")).isEqualTo("with\\.dot");
assertThat(JsonPath.Builder.encodePropertyName(".x.y")).isEqualTo("\\.x\\.y");
}

@Test
public void encodeIndex() {
assertThat(JsonPath.Builder.encodePropertyName("[123]")).isEqualTo("\\[123]");
}

@Test
public void encodeOther() {
assertThat(JsonPath.Builder.encodePropertyName("a\\b")).isEqualTo("a\\\\b");
}
}

@Nested
class Builder {

Expand All @@ -48,22 +24,20 @@ public void rootPropertyPathViaBuilder() {
b.property("b2");
assertThat(b.build().toString()).isEqualTo("b2");
b.property("with.dot");
assertThat(b.build().toString()).isEqualTo("with\\.dot");
assertThat(b.build().toString()).isEqualTo("with.dot");
b.property("[[bracketed]]");
assertThat(b.build().toString()).isEqualTo("\\[\\[bracketed]]");
b.property("\\x");
assertThat(b.build().toString()).isEqualTo("\\\\x");
assertThat(b.build().toString()).isEqualTo("[[bracketed]]");
}

@Test
public void rootArrayPathViaBuilder() {
JsonPath.Builder b = JsonPath.rootBuilder();
b.index(0);
assertThat(b.build().toString()).isEqualTo("[0]");
assertThat(b.build().toString()).isEqualTo("0");
b.index(1);
assertThat(b.build().toString()).isEqualTo("[1]");
assertThat(b.build().toString()).isEqualTo("1");
b.index(999);
assertThat(b.build().toString()).isEqualTo("[999]");
assertThat(b.build().toString()).isEqualTo("999");
}

@Test
Expand All @@ -73,25 +47,25 @@ public void nestedPropertyPathViaBuilder() {
JsonPath.Builder b2 = b.nestedValueBuilder();

assertThat(b2.property("propX").build().toString()).isEqualTo("props.propX");
assertThat(b2.index(12).build().toString()).isEqualTo("props.[12]");
assertThat(b2.property("with.dot").build().toString()).isEqualTo("props.with\\.dot");
assertThat(b2.index(12).build().toString()).isEqualTo("props.12");
assertThat(b2.property("with.dot").build().toString()).isEqualTo("props.with.dot");
}

@Test
public void nestedIndexPathViaBuilder() {
JsonPath.Builder b = JsonPath.rootBuilder();
b.property("arrays");
JsonPath.Builder b2 = b.nestedValueBuilder().index(5).nestedValueBuilder();
assertThat(b2.build().toString()).isEqualTo("arrays.[5]");
assertThat(b2.index(9).build().toString()).isEqualTo("arrays.[5].[9]");
assertThat(b2.build().toString()).isEqualTo("arrays.5");
assertThat(b2.index(9).build().toString()).isEqualTo("arrays.5.9");

// Builders are stateful so 'b3' has its last configuration that we can use:
JsonPath.Builder b3 = b2.nestedValueBuilder().property("leaf").nestedValueBuilder();
assertThat(b3.build().toString()).isEqualTo("arrays.[5].[9].leaf");
assertThat(b3.build().toString()).isEqualTo("arrays.5.9.leaf");

b.property("arr[0]");
b2 = b.nestedValueBuilder().index(3).nestedValueBuilder();
assertThat(b2.build().toString()).isEqualTo("arr\\[0].[3]");
assertThat(b2.build().toString()).isEqualTo("arr[0].3");
}
}
}
12 changes: 6 additions & 6 deletions src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ public void shredSimpleWithId() throws Exception {
JsonPath.from("_id"),
JsonPath.from("name"),
JsonPath.from("values"),
JsonPath.from("values.[0]"),
JsonPath.from("values.[1]"),
JsonPath.from("\\[extra\\.stuff]"),
JsonPath.from("values.0"),
JsonPath.from("values.1"),
JsonPath.from("[extra.stuff]"),
JsonPath.from("nullable"));

// First verify paths
Expand All @@ -76,10 +76,10 @@ public void shredSimpleWithId() throws Exception {

// Then atomic value containers
assertThat(doc.queryBoolValues())
.isEqualTo(Collections.singletonMap(JsonPath.from("\\[extra\\.stuff]"), Boolean.TRUE));
.isEqualTo(Collections.singletonMap(JsonPath.from("[extra.stuff]"), Boolean.TRUE));
Map<JsonPath, BigDecimal> expNums = new LinkedHashMap<>();
expNums.put(JsonPath.from("values.[0]"), BigDecimal.valueOf(1));
expNums.put(JsonPath.from("values.[1]"), BigDecimal.valueOf(2));
expNums.put(JsonPath.from("values.0"), BigDecimal.valueOf(1));
expNums.put(JsonPath.from("values.1"), BigDecimal.valueOf(2));
assertThat(doc.queryNumberValues()).isEqualTo(expNums);
assertThat(doc.queryTextValues())
.isEqualTo(Map.of(JsonPath.from("_id"), "abc", JsonPath.from("name"), "Bob"));
Expand Down

0 comments on commit 1b30a3f

Please sign in to comment.