From 237b8a9c40914bb5a7aa21918367ef64fbd24bda Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 16 Mar 2022 15:05:31 +0100 Subject: [PATCH] DotExpandingXContentParser to expose the original token location (#84970) With #79922 we have introduced a parser that expands dots in fields names on the fly, so that the expansion no longer needs to be handled by consumers. The token location exposed by such parser can be confusing to interpret: consumers are parsing the expanded version which requires jumping ahead reading tokens and exposing additional field names and start objects, while users have sent the unexpanded version and would like errors to refer to the original content. This commit adds a test for this scenario and tweaks the DotExpandingXContentParser to cache the token location before jumping ahead to expand dots in field names. --- docs/changelog/84970.yaml | 5 ++ .../xcontent/DotExpandingXContentParser.java | 21 +++++-- .../DotExpandingXContentParserTests.java | 60 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 docs/changelog/84970.yaml diff --git a/docs/changelog/84970.yaml b/docs/changelog/84970.yaml new file mode 100644 index 0000000000000..ac660f0a2e294 --- /dev/null +++ b/docs/changelog/84970.yaml @@ -0,0 +1,5 @@ +pr: 84970 +summary: '`DotExpandingXContentParser` to expose the original token location' +area: Search +type: bug +issues: [] diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/DotExpandingXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/DotExpandingXContentParser.java index f352143979806..e9a4cd45eaa68 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/xcontent/DotExpandingXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/xcontent/DotExpandingXContentParser.java @@ -15,7 +15,8 @@ /** * An XContentParser that reinterprets field names containing dots as an object structure. * - * A fieldname named {@code "foo.bar.baz":...} will be parsed instead as {@code 'foo':{'bar':{'baz':...}}} + * A field name named {@code "foo.bar.baz":...} will be parsed instead as {@code 'foo':{'bar':{'baz':...}}}. + * The token location is preserved so that error messages refer to the original content being parsed. */ public class DotExpandingXContentParser extends FilterXContentParser { @@ -59,13 +60,14 @@ private void expandDots() throws IOException { if (subpaths.length == 1 && field.endsWith(".") == false) { return; } + XContentLocation location = delegate().getTokenLocation(); Token token = delegate().nextToken(); if (token == Token.START_OBJECT || token == Token.START_ARRAY) { - parsers.push(new DotExpandingXContentParser(new XContentSubParser(delegate()), delegate(), subpaths)); + parsers.push(new DotExpandingXContentParser(new XContentSubParser(delegate()), delegate(), subpaths, location)); } else if (token == Token.END_OBJECT || token == Token.END_ARRAY) { throw new IllegalStateException("Expecting START_OBJECT or START_ARRAY or VALUE but got [" + token + "]"); } else { - parsers.push(new DotExpandingXContentParser(new SingletonValueXContentParser(delegate()), delegate(), subpaths)); + parsers.push(new DotExpandingXContentParser(new SingletonValueXContentParser(delegate()), delegate(), subpaths, location)); } } @@ -118,14 +120,16 @@ private enum State { final String[] subPaths; final XContentParser subparser; + private XContentLocation currentLocation; private int expandedTokens = 0; private int innerLevel = -1; private State state = State.EXPANDING_START_OBJECT; - private DotExpandingXContentParser(XContentParser subparser, XContentParser root, String[] subPaths) { + private DotExpandingXContentParser(XContentParser subparser, XContentParser root, String[] subPaths, XContentLocation startLocation) { super(root); this.subPaths = subPaths; this.subparser = subparser; + this.currentLocation = startLocation; } @Override @@ -158,6 +162,7 @@ public Token nextToken() throws IOException { if (token != null) { return token; } + currentLocation = getTokenLocation(); state = State.ENDING_EXPANDED_OBJECT; } assert expandedTokens % 2 == 1; @@ -165,6 +170,14 @@ public Token nextToken() throws IOException { return expandedTokens < 0 ? null : Token.END_OBJECT; } + @Override + public XContentLocation getTokenLocation() { + if (state == State.PARSING_ORIGINAL_CONTENT) { + return super.getTokenLocation(); + } + return currentLocation; + } + @Override public Token currentToken() { return switch (state) { diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/DotExpandingXContentParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/DotExpandingXContentParserTests.java index 2643d5c8c86c7..8134d1e922651 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/DotExpandingXContentParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/DotExpandingXContentParserTests.java @@ -166,4 +166,64 @@ public void testNestedExpansions() throws IOException { {"first.dot":{"second.dot":"value","third":"value"},"nodots":"value"}\ """); } + + public void testGetTokenLocation() throws IOException { + String jsonInput = """ + {"first.dot":{"second.dot":"value", + "value":null}}\ + """; + XContentParser expectedParser = createParser(JsonXContent.jsonXContent, jsonInput); + XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, jsonInput)); + + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); + assertEquals(XContentParser.Token.START_OBJECT, expectedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.FIELD_NAME, expectedParser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); + assertEquals("first", dotExpandedParser.currentName()); + assertEquals("first.dot", expectedParser.currentName()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals("dot", dotExpandedParser.currentName()); + assertEquals(XContentParser.Token.START_OBJECT, expectedParser.nextToken()); + assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.FIELD_NAME, expectedParser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); + assertEquals("second", dotExpandedParser.currentName()); + assertEquals("second.dot", expectedParser.currentName()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals("dot", dotExpandedParser.currentName()); + assertEquals(XContentParser.Token.VALUE_STRING, expectedParser.nextToken()); + assertEquals(XContentParser.Token.VALUE_STRING, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.END_OBJECT, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.FIELD_NAME, expectedParser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); + assertEquals("value", dotExpandedParser.currentName()); + assertEquals("value", expectedParser.currentName()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.VALUE_NULL, expectedParser.nextToken()); + assertEquals(XContentParser.Token.VALUE_NULL, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.END_OBJECT, dotExpandedParser.nextToken()); + assertEquals(XContentParser.Token.END_OBJECT, expectedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.END_OBJECT, dotExpandedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertEquals(XContentParser.Token.END_OBJECT, dotExpandedParser.nextToken()); + assertEquals(XContentParser.Token.END_OBJECT, expectedParser.nextToken()); + assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); + assertNull(dotExpandedParser.nextToken()); + assertNull(expectedParser.nextToken()); + } }