Skip to content

ObjectParser: add method to parse inner objects with names matching a given predicate #24020

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

Closed
wants to merge 3 commits into from
Closed

ObjectParser: add method to parse inner objects with names matching a given predicate #24020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e078ec6
Add method to parse unknown field names to ObjectParser
cbuescher Apr 10, 2017
59118cd
Adding predicate to check matching of unknown field name
cbuescher May 9, 2017
a8d225a
Adressing review comments
cbuescher May 10, 2017
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
104 changes: 100 additions & 4 deletions core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.common.xcontent;

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
Expand All @@ -30,9 +31,11 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.function.Supplier;

import static org.elasticsearch.common.xcontent.XContentParser.Token.START_ARRAY;
Expand Down Expand Up @@ -88,6 +91,11 @@ public static <Value, ElementValue> BiConsumer<Value, List<ElementValue>> fromLi
*/
private final boolean ignoreUnknownFields;

/**
* A special purpose field parser that gets used when the provided matchFieldPredicate accepts it
*/
SetOnce<MatchField> matchField = new SetOnce<>();
Copy link
Member

@javanna javanna May 10, 2017

Choose a reason for hiding this comment

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

this could be final?


/**
* Creates a new ObjectParser instance with a name. This name is used to reference the parser in exceptions and messages.
*/
Expand Down Expand Up @@ -214,6 +222,89 @@ public <T> void declareField(BiConsumer<Value, T> consumer, ContextParser<Contex
declareField((p, v, c) -> consumer.accept(v, parser.parse(p, c)), parseField, type);
}

/**
* Declares a parser for fields where the exact field name is not known when
* creating the ObjectParser. In order for this field name to match, it has
* to be accepted by a provided predicate. As an example, in the aggregation
* output parsing for the high level java rest client we have things like:
*
* <pre>
* "aggregations" : {
* "terms#genres" : { // aggregation type and arbitrary name
* "doc_count_error_upper_bound": 0,
* "sum_other_doc_count": 0,
* "buckets" : [
* {
* "key" : "jazz",
* "doc_count" : 10
* "sum#total_number_of_ratings": // aggregation type and arbitrary name
* "value": 2691
* }
* },
* [...]
* ]
* }
* }
* </pre>
*
* Since field names are arbitrary in these cases, we cannot match them with
* a ParseField like we do in other places when using ObjectParser. This
* method can be used to register a special parser for a field name that
* matches the provided predicate.
*
* @param consumer
* handle the values once they have been parsed
* @param parser
* parses each nested object
* @param fieldNamePredicate
* a predicate that returns true if the provided parser should
* handle this field
* @param type
* the accepted values for this field
*/
public <T> void declareMatchFieldParser(BiConsumer<Value, T> consumer, ContextParser<Context, T> parser,
Predicate<String> fieldNamePredicate, ValueType type) {
Objects.requireNonNull(consumer, "[consumer] is required");
Objects.requireNonNull(parser, "[parser] is required");
Objects.requireNonNull(fieldNamePredicate, "[fieldNameMatcher] is required");
Objects.requireNonNull(type, "[type] is required");
this.matchField
.set(new MatchField(new MatchFieldParser((p, v, c) -> consumer.accept(v, parser.parse(p, c)), type), fieldNamePredicate));
}

private class MatchField {

private final FieldParser parser;
private final Predicate<String> predicate;

private MatchField(final FieldParser matchFieldParser, final Predicate<String> matchFieldPredicate) {
this.parser = matchFieldParser;
this.predicate = matchFieldPredicate;
}
}

private class MatchFieldParser extends FieldParser {
Copy link
Member

@javanna javanna May 10, 2017

Choose a reason for hiding this comment

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

I wonder why we need this class at this point. I guess it is because we don't provide a ParseField and the parseField.match part in assertSupports would not work. Would it be an idea to change parse field to always work with a predicate internally, and allow to pass the predicate in through a new constructor?


MatchFieldParser(Parser<Value, Context> parser, ValueType type) {
super(parser, type.supportedTokens(), null, type);
}

@Override
void assertSupports(String parserName, XContentParser.Token token, String currentFieldName) {
if (supportedTokens.contains(token) == false) {
throw new IllegalArgumentException(
"[" + parserName + "] " + currentFieldName + " doesn't support values of type: " + token);
}
}

@Override
public String toString() {
return "FieldParser{" + "MatchAllFieldParser, supportedTokens=" + supportedTokens
+ ", type=" + type.name() + '}';
}

}

public <T> void declareObjectOrDefault(BiConsumer<Value, T> consumer, BiFunction<XContentParser, Context, T> objectParser,
Supplier<T> defaultValue, ParseField field) {
declareField((p, v, c) -> {
Expand Down Expand Up @@ -342,17 +433,22 @@ private void parseSub(XContentParser parser, FieldParser fieldParser, String cur

private FieldParser getParser(String fieldName) {
FieldParser parser = fieldParserMap.get(fieldName);
if (parser == null && false == ignoreUnknownFields) {
throw new IllegalArgumentException("[" + name + "] unknown field [" + fieldName + "], parser not found");
if (parser == null) {
MatchField matchField = this.matchField.get();
if (matchField != null && matchField.predicate.test(fieldName)) {
parser = matchField.parser;
} else if (false == this.ignoreUnknownFields) {
throw new IllegalArgumentException("[" + name + "] unknown field [" + fieldName + "], parser not found");
}
}
return parser;
}

private class FieldParser {
private final Parser<Value, Context> parser;
private final EnumSet<XContentParser.Token> supportedTokens;
protected final EnumSet<XContentParser.Token> supportedTokens;
private final ParseField parseField;
private final ValueType type;
protected final ValueType type;

FieldParser(Parser<Value, Context> parser, EnumSet<XContentParser.Token> supportedTokens, ParseField parseField, ValueType type) {
this.parser = parser;
Expand Down
143 changes: 137 additions & 6 deletions core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.common.CheckedFunction;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
Expand All @@ -31,14 +32,16 @@
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.hasSize;

public class ObjectParserTests extends ESTestCase {

public void testBasics() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\n"
+ " \"test\" : \"foo\",\n"
+ " \"test_number\" : 2,\n"
Expand Down Expand Up @@ -448,7 +451,7 @@ public void setString_or_null(String string_or_null) {
}

public void testParseNamedObject() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": {\n"
+ " \"a\": {}"
+ "}}");
Expand All @@ -459,7 +462,7 @@ public void testParseNamedObject() throws IOException {
}

public void testParseNamedObjectInOrder() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}}"
+ "]}");
Expand All @@ -470,7 +473,7 @@ public void testParseNamedObjectInOrder() throws IOException {
}

public void testParseNamedObjectTwoFieldsInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {\"a\": {}, \"b\": {}}"
+ "]}");
Expand All @@ -482,7 +485,7 @@ public void testParseNamedObjectTwoFieldsInArray() throws IOException {
}

public void testParseNamedObjectNoFieldsInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " {}"
+ "]}");
Expand All @@ -494,7 +497,7 @@ public void testParseNamedObjectNoFieldsInArray() throws IOException {
}

public void testParseNamedObjectJunkInArray() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent,
XContentParser parser = createParser(JsonXContent.jsonXContent,
"{\"named\": [\n"
+ " \"junk\""
+ "]}");
Expand Down Expand Up @@ -593,6 +596,134 @@ class TestStruct {
assertEquals(s.test, "foo");
}

/**
* test parsing fields with an unknown field name
*/
public void testUnknownFieldParser() throws IOException {
XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent());
String randomName = "unknown#" + randomIntBetween(1, 9);
b.startObject();
{
b.field("test", "foo");
b.field("shouldBeIgnored", "foo3");
b.field(randomName, "foo2");
}
b.endObject();
b = shuffleXContent(b);
XContentParser parser = createParser(JsonXContent.jsonXContent, b.bytes());

class TestObject {
public String test;
public String unknown;
public String name;

public void setUnknown(String name, String unknown) {
this.name = name;
this.unknown = unknown;
}
}

boolean ignoreUnknown = randomBoolean();
ObjectParser<TestObject, Void> objectParser = new ObjectParser<>("testParser", ignoreUnknown, null);
objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("test"), ObjectParser.ValueType.STRING);
objectParser.declareMatchFieldParser((value, tuple) -> {
value.setUnknown(tuple.v1(), tuple.v2());
}, (p, c) -> {
String name = p.currentName();
String value = p.text();
return new Tuple<>(name, value);
}, s -> s.contains("#"), ObjectParser.ValueType.STRING);

if (ignoreUnknown) {
TestObject s = objectParser.parse(parser, new TestObject(), null);
assertEquals(s.test, "foo");
assertEquals(s.unknown, "foo2");
assertEquals(s.name, randomName);
} else {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> objectParser.parse(parser, new TestObject(), null));
assertEquals("[testParser] unknown field [shouldBeIgnored], parser not found", e.getMessage());
}
}

/**
* test parsing fields with an unknown field name that are itself json objects
*/
public void testUnknownFieldParserInnerObject() throws IOException {
XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent());
String randomType1 = randomAlphaOfLength(5);
String randomName1 = randomAlphaOfLength(5);
String randomType2 = randomAlphaOfLength(5);
String randomName2 = randomAlphaOfLength(5);
b.startObject();
{
b.field("value", "outerValue");
b.startObject(randomType1 + "#" + randomName1);
{
b.field("value", "innerValue1");
}
b.endObject();
b.startObject(randomType2 + "#" + randomName2);
{
b.field("value", "innerValue2");
}
b.endObject();
b.startObject("shouldBeIgnored");
{
b.field("thisShouldBeSkipped", "skip");
}
b.endObject();
}
b.endObject();
b = shuffleXContent(b);
XContentParser parser = createParser(JsonXContent.jsonXContent, b.bytes());

class TestObject {
public String value;
public String name;
public String type;
public Map<String, TestObject> innerObjects = new HashMap<>();

public void addInnerObject(TestObject inner) {
this.innerObjects.put(inner.name, inner);
}
}

boolean ignoreUnknown = randomBoolean();
ObjectParser<TestObject, Void> objectParser = new ObjectParser<>("testParser", ignoreUnknown, null);
objectParser.declareField((i, c, x) -> c.value = i.text(), new ParseField("value"),
ObjectParser.ValueType.STRING);
objectParser.declareMatchFieldParser(TestObject::addInnerObject, (p, c) -> {
String nameAndType = p.currentName();
int pos = nameAndType.indexOf("#");
TestObject innerObject = objectParser.parse(p, new TestObject(), null);
innerObject.type = nameAndType.substring(0, pos);
innerObject.name = nameAndType.substring(pos + 1);
return innerObject;
}, s -> s.contains("#"), ObjectParser.ValueType.OBJECT);

if (ignoreUnknown) {
TestObject s = objectParser.parse(parser, new TestObject(), null);
assertEquals(s.value, "outerValue");
assertNull(s.name);
assertNull(s.type);

assertNotNull(s.innerObjects);
assertEquals(s.innerObjects.get(randomName1).name, randomName1);
assertEquals(s.innerObjects.get(randomName1).type, randomType1);
assertEquals(s.innerObjects.get(randomName1).value, "innerValue1");

assertEquals(s.innerObjects.get(randomName2).name, randomName2);
assertEquals(s.innerObjects.get(randomName2).type, randomType2);
assertEquals(s.innerObjects.get(randomName2).value, "innerValue2");
} else {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> objectParser.parse(parser, new TestObject(), null));
assertEquals("[testParser] unknown field [shouldBeIgnored], parser not found", e.getMessage());
}

}

static class NamedObjectHolder {
public static final ObjectParser<NamedObjectHolder, Void> PARSER = new ObjectParser<>("named_object_holder",
NamedObjectHolder::new);
Expand Down