Skip to content

Commit

Permalink
Allow specifying an exclusive set of fields on ObjectParser (#52893)
Browse files Browse the repository at this point in the history
ObjectParser allows you to declare a set of required fields, such that at least one
of the set must appear in an xcontent object for it to be valid. This commit adds
the similar concept of a set of exclusive fields, such that at most one of the set
must be present. It also enables required fields on ConstructingObjectParser, and
re-implements PercolateQueryBuilder.fromXContent() to use object parsing as
an example of how this works.
  • Loading branch information
romseygeek committed Mar 3, 2020
1 parent 510db25 commit 3759063
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public abstract class AbstractObjectParser<Value, Context>
implements BiFunction<XContentParser, Context, Value>, ContextParser<Context, Value> {

final List<String[]> requiredFieldSets = new ArrayList<>();
final List<String[]> exclusiveFieldSets = new ArrayList<>();

/**
* Declare some field. Usually it is easier to use {@link #declareString(BiConsumer, ParseField)} or
Expand Down Expand Up @@ -294,6 +295,25 @@ public void declareRequiredFieldSet(String... requiredSet) {
this.requiredFieldSets.add(requiredSet);
}

/**
* Declares a set of fields of which at most one must appear for parsing to succeed
*
* E.g. <code>declareExclusiveFieldSet("foo", "bar");</code> means that only one of 'foo'
* or 'bar' must be present, and if both appear then an exception will be thrown. Note
* that this does not make 'foo' or 'bar' required - see {@link #declareRequiredFieldSet(String...)}
* for required fields.
*
* Multiple exclusive sets may be declared
*
* @param exclusiveSet a set of field names, at most one of which must appear
*/
public void declareExclusiveFieldSet(String... exclusiveSet) {
if (exclusiveSet.length == 0) {
return;
}
this.exclusiveFieldSets.add(exclusiveSet);
}

private interface IOSupplier<T> {
T get() throws IOException;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,16 @@ public String getName() {
return objectParser.getName();
}

@Override
public void declareRequiredFieldSet(String... requiredSet) {
objectParser.declareRequiredFieldSet(requiredSet);
}

@Override
public void declareExclusiveFieldSet(String... exclusiveSet) {
objectParser.declareExclusiveFieldSet(exclusiveSet);
}

private Consumer<Target> wrapOrderedModeCallBack(Consumer<Value> callback) {
return (target) -> {
if (target.targetObject != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
String currentFieldName = null;
XContentLocation currentPosition = null;
List<String[]> requiredFields = new ArrayList<>(this.requiredFieldSets);
List<List<String>> exclusiveFields = new ArrayList<>();
for (int i = 0; i < this.exclusiveFieldSets.size(); i++) {
exclusiveFields.add(new ArrayList<>());
}

while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
Expand Down Expand Up @@ -302,18 +306,41 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
}
}

// Check if this field is in an exclusive set, if it is then mark
// it as seen.
for (int i = 0; i < this.exclusiveFieldSets.size(); i++) {
for (String field : this.exclusiveFieldSets.get(i)) {
if (field.equals(currentFieldName)) {
exclusiveFields.get(i).add(currentFieldName);
}
}
}

parseSub(parser, fieldParser, currentFieldName, value, context);
}
fieldParser = null;
}
}

// Check for a) multiple entries appearing in exclusive field sets and b) empty
// required field entries
StringBuilder message = new StringBuilder();
for (List<String> fieldset : exclusiveFields) {
if (fieldset.size() > 1) {
message.append("The following fields are not allowed together: ").append(fieldset.toString()).append(" ");
}
}
if (message.length() > 0) {
throw new IllegalArgumentException(message.toString());
}

if (requiredFields.isEmpty() == false) {
StringBuilder message = new StringBuilder();
for (String[] fields : requiredFields) {
message.append("Required one of fields ").append(Arrays.toString(fields)).append(", but none were specified. ");
}
throw new IllegalArgumentException(message.toString());
}

return value;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,4 +530,45 @@ public void keepNamedInOrder() {
namedSuppliedInOrder = true;
}
}

public void testRequiredAndExclusiveFields() throws IOException {

class TestStruct {
final String a;
final long b;
TestStruct(String a) {
this.a = a;
this.b = 0;
}
TestStruct(long b) {
this.a = null;
this.b = b;
}
}

XContentParser ok = createParser(JsonXContent.jsonXContent, "{ \"a\" : \"a\" }");
XContentParser toomany = createParser(JsonXContent.jsonXContent, "{ \"a\" : \"a\", \"b\" : 1 }");
XContentParser notenough = createParser(JsonXContent.jsonXContent, "{ }");

ConstructingObjectParser<TestStruct, Void> parser = new ConstructingObjectParser<>("teststruct", args -> {
if (args[0] != null) {
return new TestStruct((String) args[0]);
}
return new TestStruct((Long) args[1]);
});
parser.declareString(optionalConstructorArg(), new ParseField("a"));
parser.declareLong(optionalConstructorArg(), new ParseField("b"));
parser.declareExclusiveFieldSet("a", "b");
parser.declareRequiredFieldSet("a", "b");

TestStruct actual = parser.parse(ok, null);
assertThat(actual.a, equalTo("a"));
assertThat(actual.b, equalTo(0L));

IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse(toomany, null));
assertThat(e.getMessage(), containsString("allowed together: [a, b]"));

e = expectThrows(IllegalArgumentException.class, () -> parser.parse(notenough, null));
assertThat(e.getMessage(), containsString("Required one of fields [a, b], but none were specified."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -851,29 +851,30 @@ private void setB(long value) {
assertThat(obj.b, equalTo(456L));
}

public void testMultipleRequiredFieldSet() throws IOException {
class TestStruct {
private Long a;
private Long b;
private Long c;
private Long d;
private static class TestStruct {
private Long a;
private Long b;
private Long c;
private Long d;

private void setA(long value) {
this.a = value;
}
private void setA(long value) {
this.a = value;
}

private void setB(long value) {
this.b = value;
}
private void setB(long value) {
this.b = value;
}

private void setC(long value) {
this.c = value;
}
private void setC(long value) {
this.c = value;
}

private void setD(long value) {
this.d = value;
}
private void setD(long value) {
this.d = value;
}
}

public void testMultipleRequiredFieldSet() throws IOException {

XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"unrelated\": \"123\"}");
ObjectParser<TestStruct, Void> objectParser = new ObjectParser<>("foo", true, TestStruct::new);
Expand All @@ -889,6 +890,32 @@ private void setD(long value) {
"Required one of fields [c, d], but none were specified. "));
}

public void testExclusiveFieldSet() throws IOException {

XContentParser goodA = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"c\" : 4}");
XContentParser bad = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"b\" : 2}");
XContentParser badmulti = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"b\" : 2, \"c\" : 3, \"d\" : 4 }");

ObjectParser<TestStruct, Void> parser = new ObjectParser<>("foo", TestStruct::new);
parser.declareLong(TestStruct::setA, new ParseField("a"));
parser.declareLong(TestStruct::setB, new ParseField("b"));
parser.declareLong(TestStruct::setC, new ParseField("c"));
parser.declareLong(TestStruct::setD, new ParseField("d"));
parser.declareExclusiveFieldSet("a", "b");
parser.declareExclusiveFieldSet("c", "d");

TestStruct actualA = parser.parse(goodA, null);
assertThat(actualA.a, equalTo(1L));
assertThat(actualA.c, equalTo(4L));

IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse(bad, null));
assertThat(e.getMessage(), containsString("The following fields are not allowed together: [a, b]"));

e = expectThrows(IllegalArgumentException.class, () -> parser.parse(badmulti, null));
assertThat(e.getMessage(),
containsString("allowed together: [a, b] The following fields are not allowed together: [c, d]"));
}

@Override
protected NamedXContentRegistry xContentRegistry() {
return new NamedXContentRegistry(Arrays.asList(
Expand Down
Loading

0 comments on commit 3759063

Please sign in to comment.