Skip to content

Commit fbd8d79

Browse files
authored
Allow specifying an exclusive set of fields on ObjectParser (elastic#52893)
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.
1 parent 98b4e8c commit fbd8d79

File tree

7 files changed

+195
-127
lines changed

7 files changed

+195
-127
lines changed

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public abstract class AbstractObjectParser<Value, Context>
3838
implements BiFunction<XContentParser, Context, Value>, ContextParser<Context, Value> {
3939

4040
final List<String[]> requiredFieldSets = new ArrayList<>();
41+
final List<String[]> exclusiveFieldSets = new ArrayList<>();
4142

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

298+
/**
299+
* Declares a set of fields of which at most one must appear for parsing to succeed
300+
*
301+
* E.g. <code>declareExclusiveFieldSet("foo", "bar");</code> means that only one of 'foo'
302+
* or 'bar' must be present, and if both appear then an exception will be thrown. Note
303+
* that this does not make 'foo' or 'bar' required - see {@link #declareRequiredFieldSet(String...)}
304+
* for required fields.
305+
*
306+
* Multiple exclusive sets may be declared
307+
*
308+
* @param exclusiveSet a set of field names, at most one of which must appear
309+
*/
310+
public void declareExclusiveFieldSet(String... exclusiveSet) {
311+
if (exclusiveSet.length == 0) {
312+
return;
313+
}
314+
this.exclusiveFieldSets.add(exclusiveSet);
315+
}
316+
297317
private interface IOSupplier<T> {
298318
T get() throws IOException;
299319
}

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,16 @@ public String getName() {
299299
return objectParser.getName();
300300
}
301301

302+
@Override
303+
public void declareRequiredFieldSet(String... requiredSet) {
304+
objectParser.declareRequiredFieldSet(requiredSet);
305+
}
306+
307+
@Override
308+
public void declareExclusiveFieldSet(String... exclusiveSet) {
309+
objectParser.declareExclusiveFieldSet(exclusiveSet);
310+
}
311+
302312
private Consumer<Target> wrapOrderedModeCallBack(Consumer<Value> callback) {
303313
return (target) -> {
304314
if (target.targetObject != null) {

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
274274
String currentFieldName = null;
275275
XContentLocation currentPosition = null;
276276
List<String[]> requiredFields = new ArrayList<>(this.requiredFieldSets);
277+
List<List<String>> exclusiveFields = new ArrayList<>();
278+
for (int i = 0; i < this.exclusiveFieldSets.size(); i++) {
279+
exclusiveFields.add(new ArrayList<>());
280+
}
277281

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

309+
// Check if this field is in an exclusive set, if it is then mark
310+
// it as seen.
311+
for (int i = 0; i < this.exclusiveFieldSets.size(); i++) {
312+
for (String field : this.exclusiveFieldSets.get(i)) {
313+
if (field.equals(currentFieldName)) {
314+
exclusiveFields.get(i).add(currentFieldName);
315+
}
316+
}
317+
}
318+
305319
parseSub(parser, fieldParser, currentFieldName, value, context);
306320
}
307321
fieldParser = null;
308322
}
309323
}
324+
325+
// Check for a) multiple entries appearing in exclusive field sets and b) empty
326+
// required field entries
327+
StringBuilder message = new StringBuilder();
328+
for (List<String> fieldset : exclusiveFields) {
329+
if (fieldset.size() > 1) {
330+
message.append("The following fields are not allowed together: ").append(fieldset.toString()).append(" ");
331+
}
332+
}
333+
if (message.length() > 0) {
334+
throw new IllegalArgumentException(message.toString());
335+
}
336+
310337
if (requiredFields.isEmpty() == false) {
311-
StringBuilder message = new StringBuilder();
312338
for (String[] fields : requiredFields) {
313339
message.append("Required one of fields ").append(Arrays.toString(fields)).append(", but none were specified. ");
314340
}
315341
throw new IllegalArgumentException(message.toString());
316342
}
343+
317344
return value;
318345
}
319346

libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,4 +528,45 @@ public void keepNamedInOrder() {
528528
namedSuppliedInOrder = true;
529529
}
530530
}
531+
532+
public void testRequiredAndExclusiveFields() throws IOException {
533+
534+
class TestStruct {
535+
final String a;
536+
final long b;
537+
TestStruct(String a) {
538+
this.a = a;
539+
this.b = 0;
540+
}
541+
TestStruct(long b) {
542+
this.a = null;
543+
this.b = b;
544+
}
545+
}
546+
547+
XContentParser ok = createParser(JsonXContent.jsonXContent, "{ \"a\" : \"a\" }");
548+
XContentParser toomany = createParser(JsonXContent.jsonXContent, "{ \"a\" : \"a\", \"b\" : 1 }");
549+
XContentParser notenough = createParser(JsonXContent.jsonXContent, "{ }");
550+
551+
ConstructingObjectParser<TestStruct, Void> parser = new ConstructingObjectParser<>("teststruct", args -> {
552+
if (args[0] != null) {
553+
return new TestStruct((String) args[0]);
554+
}
555+
return new TestStruct((Long) args[1]);
556+
});
557+
parser.declareString(optionalConstructorArg(), new ParseField("a"));
558+
parser.declareLong(optionalConstructorArg(), new ParseField("b"));
559+
parser.declareExclusiveFieldSet("a", "b");
560+
parser.declareRequiredFieldSet("a", "b");
561+
562+
TestStruct actual = parser.parse(ok, null);
563+
assertThat(actual.a, equalTo("a"));
564+
assertThat(actual.b, equalTo(0L));
565+
566+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse(toomany, null));
567+
assertThat(e.getMessage(), containsString("allowed together: [a, b]"));
568+
569+
e = expectThrows(IllegalArgumentException.class, () -> parser.parse(notenough, null));
570+
assertThat(e.getMessage(), containsString("Required one of fields [a, b], but none were specified."));
571+
}
531572
}

libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -850,29 +850,30 @@ private void setB(long value) {
850850
assertThat(obj.b, equalTo(456L));
851851
}
852852

853-
public void testMultipleRequiredFieldSet() throws IOException {
854-
class TestStruct {
855-
private Long a;
856-
private Long b;
857-
private Long c;
858-
private Long d;
853+
private static class TestStruct {
854+
private Long a;
855+
private Long b;
856+
private Long c;
857+
private Long d;
859858

860-
private void setA(long value) {
861-
this.a = value;
862-
}
859+
private void setA(long value) {
860+
this.a = value;
861+
}
863862

864-
private void setB(long value) {
865-
this.b = value;
866-
}
863+
private void setB(long value) {
864+
this.b = value;
865+
}
867866

868-
private void setC(long value) {
869-
this.c = value;
870-
}
867+
private void setC(long value) {
868+
this.c = value;
869+
}
871870

872-
private void setD(long value) {
873-
this.d = value;
874-
}
871+
private void setD(long value) {
872+
this.d = value;
875873
}
874+
}
875+
876+
public void testMultipleRequiredFieldSet() throws IOException {
876877

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

892+
public void testExclusiveFieldSet() throws IOException {
893+
894+
XContentParser goodA = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"c\" : 4}");
895+
XContentParser bad = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"b\" : 2}");
896+
XContentParser badmulti = createParser(JsonXContent.jsonXContent, "{\"a\" : 1, \"b\" : 2, \"c\" : 3, \"d\" : 4 }");
897+
898+
ObjectParser<TestStruct, Void> parser = new ObjectParser<>("foo", TestStruct::new);
899+
parser.declareLong(TestStruct::setA, new ParseField("a"));
900+
parser.declareLong(TestStruct::setB, new ParseField("b"));
901+
parser.declareLong(TestStruct::setC, new ParseField("c"));
902+
parser.declareLong(TestStruct::setD, new ParseField("d"));
903+
parser.declareExclusiveFieldSet("a", "b");
904+
parser.declareExclusiveFieldSet("c", "d");
905+
906+
TestStruct actualA = parser.parse(goodA, null);
907+
assertThat(actualA.a, equalTo(1L));
908+
assertThat(actualA.c, equalTo(4L));
909+
910+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse(bad, null));
911+
assertThat(e.getMessage(), containsString("The following fields are not allowed together: [a, b]"));
912+
913+
e = expectThrows(IllegalArgumentException.class, () -> parser.parse(badmulti, null));
914+
assertThat(e.getMessage(),
915+
containsString("allowed together: [a, b] The following fields are not allowed together: [c, d]"));
916+
}
917+
891918
@Override
892919
protected NamedXContentRegistry xContentRegistry() {
893920
return new NamedXContentRegistry(Arrays.asList(

0 commit comments

Comments
 (0)