Allow specifying an exclusive set of fields on ObjectParser (#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.

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

@@ -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
@@ -294,6 +295,25 @@ public abstract class AbstractObjectParser<Value, Context>
         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;
     }

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

@@ -299,6 +299,16 @@ public final class ConstructingObjectParser<Value, Context> extends AbstractObje
         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) {

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

@@ -274,6 +274,10 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
         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) {
@@ -302,18 +306,41 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
                         }
                     }
 
+                    // 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;
     }
 

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

@@ -528,4 +528,45 @@ public class ConstructingObjectParserTests extends ESTestCase {
             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."));
+    }
 }

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

@@ -850,29 +850,30 @@ public class ObjectParserTests extends ESTestCase {
         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);
@@ -888,6 +889,32 @@ public class ObjectParserTests extends ESTestCase {
             "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(

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java

@@ -48,13 +48,13 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.common.ParseField;
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.InputStreamStreamInput;
 import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -90,6 +90,8 @@ import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
 
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
 import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
 
 public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBuilder> {
@@ -316,113 +318,52 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
         builder.endObject();
     }
 
-    public static PercolateQueryBuilder fromXContent(XContentParser parser) throws IOException {
-        float boost = AbstractQueryBuilder.DEFAULT_BOOST;
-
-        String field = null;
-        String name = null;
-
-        String indexedDocumentIndex = null;
-        String indexedDocumentId = null;
-        String indexedDocumentRouting = null;
-        String indexedDocumentPreference = null;
-        Long indexedDocumentVersion = null;
-
-        List<BytesReference> documents = new ArrayList<>();
-
-        String queryName = null;
-        String currentFieldName = null;
-
-        boolean documentsSpecified = false;
-        boolean documentSpecified = false;
-
-        XContentParser.Token token;
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                currentFieldName = parser.currentName();
-            } else if (token == XContentParser.Token.START_ARRAY) {
-                if (DOCUMENTS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    if (documentSpecified) {
-                        throw new IllegalArgumentException("[" + PercolateQueryBuilder.NAME +
-                            "] Either specified [document] or [documents], not both");
-                    }
-                    documentsSpecified = true;
-                    while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                        if (token == XContentParser.Token.START_OBJECT) {
-                            try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
-                                builder.copyCurrentStructure(parser);
-                                builder.flush();
-                                documents.add(BytesReference.bytes(builder));
-                            }
-                        } else {
-                            throw new ParsingException(parser.getTokenLocation(), "[" + PercolateQueryBuilder.NAME +
-                                "] query does not support [" + token + "]");
-                        }
-                    }
-                } else {
-                    throw new ParsingException(parser.getTokenLocation(), "[" + PercolateQueryBuilder.NAME +
-                        "] query does not field name [" + currentFieldName + "]");
-                }
-            } else if (token == XContentParser.Token.START_OBJECT) {
-                if (DOCUMENT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    if (documentsSpecified) {
-                        throw new IllegalArgumentException("[" + PercolateQueryBuilder.NAME +
-                            "] Either specified [document] or [documents], not both");
-                    }
-                    documentSpecified = true;
-                    try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
-                        builder.copyCurrentStructure(parser);
-                        builder.flush();
-                        documents.add(BytesReference.bytes(builder));
-                    }
-                } else {
-                    throw new ParsingException(parser.getTokenLocation(), "[" + PercolateQueryBuilder.NAME +
-                            "] query does not support field name [" + currentFieldName + "]");
-                }
-            } else if (token.isValue() || token == XContentParser.Token.VALUE_NULL) {
-                if (QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    field = parser.text();
-                } else if (NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    name = parser.textOrNull();
-                } else if (INDEXED_DOCUMENT_FIELD_INDEX.match(currentFieldName, parser.getDeprecationHandler())) {
-                    indexedDocumentIndex = parser.text();
-                } else if (INDEXED_DOCUMENT_FIELD_ID.match(currentFieldName, parser.getDeprecationHandler())) {
-                    indexedDocumentId = parser.text();
-                } else if (INDEXED_DOCUMENT_FIELD_ROUTING.match(currentFieldName, parser.getDeprecationHandler())) {
-                    indexedDocumentRouting = parser.text();
-                } else if (INDEXED_DOCUMENT_FIELD_PREFERENCE.match(currentFieldName, parser.getDeprecationHandler())) {
-                    indexedDocumentPreference = parser.text();
-                } else if (INDEXED_DOCUMENT_FIELD_VERSION.match(currentFieldName, parser.getDeprecationHandler())) {
-                    indexedDocumentVersion = parser.longValue();
-                } else if (AbstractQueryBuilder.BOOST_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    boost = parser.floatValue();
-                } else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    queryName = parser.text();
-                } else {
-                    throw new ParsingException(parser.getTokenLocation(), "[" + PercolateQueryBuilder.NAME +
-                            "] query does not support [" + currentFieldName + "]");
-                }
-            } else {
-                throw new ParsingException(parser.getTokenLocation(), "[" + PercolateQueryBuilder.NAME +
-                        "] query does not support [" + token + "]");
-            }
-        }
-
-        PercolateQueryBuilder queryBuilder;
-        if (documents.isEmpty() == false) {
-            queryBuilder = new PercolateQueryBuilder(field, documents, XContentType.JSON);
-        } else if (indexedDocumentId != null) {
-            queryBuilder = new PercolateQueryBuilder(field, indexedDocumentIndex, indexedDocumentId, indexedDocumentRouting,
-                indexedDocumentPreference, indexedDocumentVersion);
+    private static final ConstructingObjectParser<PercolateQueryBuilder, Void> PARSER = new ConstructingObjectParser<>(NAME, args -> {
+        String field = (String) args[0];
+        BytesReference document = (BytesReference) args[1];
+        @SuppressWarnings("unchecked")
+        List<BytesReference> documents = (List<BytesReference>) args[2];
+        String indexedDocId = (String) args[3];
+        String indexedDocIndex = (String) args[4];
+        String indexDocRouting = (String) args[5];
+        String indexDocPreference = (String) args[6];
+        Long indexedDocVersion = (Long) args[7];
+        if (indexedDocId != null) {
+            return new PercolateQueryBuilder(field, indexedDocIndex, indexedDocId, indexDocRouting, indexDocPreference, indexedDocVersion);
+        } else if (document != null) {
+            return new PercolateQueryBuilder(field, List.of(document), XContentType.JSON);
         } else {
-            throw new IllegalArgumentException("[" + PercolateQueryBuilder.NAME + "] query, nothing to percolate");
-        }
-        if (name != null) {
-            queryBuilder.setName(name);
+            return new PercolateQueryBuilder(field, documents, XContentType.JSON);
+        }
+    });
+    static {
+        PARSER.declareString(constructorArg(), QUERY_FIELD);
+        PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseDocument(p), DOCUMENT_FIELD);
+        PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> parseDocument(p), DOCUMENTS_FIELD);
+        PARSER.declareString(optionalConstructorArg(), INDEXED_DOCUMENT_FIELD_ID);
+        PARSER.declareString(optionalConstructorArg(), INDEXED_DOCUMENT_FIELD_INDEX);
+        PARSER.declareString(optionalConstructorArg(), INDEXED_DOCUMENT_FIELD_ROUTING);
+        PARSER.declareString(optionalConstructorArg(), INDEXED_DOCUMENT_FIELD_PREFERENCE);
+        PARSER.declareLong(optionalConstructorArg(), INDEXED_DOCUMENT_FIELD_VERSION);
+        PARSER.declareString(PercolateQueryBuilder::setName, NAME_FIELD);
+        PARSER.declareString(PercolateQueryBuilder::queryName, AbstractQueryBuilder.NAME_FIELD);
+        PARSER.declareFloat(PercolateQueryBuilder::boost, BOOST_FIELD);
+        PARSER.declareRequiredFieldSet(DOCUMENT_FIELD.getPreferredName(),
+            DOCUMENTS_FIELD.getPreferredName(), INDEXED_DOCUMENT_FIELD_ID.getPreferredName());
+        PARSER.declareExclusiveFieldSet(DOCUMENT_FIELD.getPreferredName(),
+            DOCUMENTS_FIELD.getPreferredName(), INDEXED_DOCUMENT_FIELD_ID.getPreferredName());
+    }
+
+    private static BytesReference parseDocument(XContentParser parser) throws IOException {
+        try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
+            builder.copyCurrentStructure(parser);
+            builder.flush();
+            return BytesReference.bytes(builder);
         }
-        queryBuilder.queryName(queryName);
-        queryBuilder.boost(boost);
-        return queryBuilder;
+    }
+
+    public static PercolateQueryBuilder fromXContent(XContentParser parser) throws IOException {
+        return PARSER.parse(parser, null);
     }
 
     @Override

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java

@@ -57,6 +57,7 @@ import java.util.Set;
 import java.util.function.Supplier;
 
 import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -244,9 +245,10 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
         rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext);
     }
 
-    public void testBothDocumentAndDocumentsSpecified() throws IOException {
-        expectThrows(IllegalArgumentException.class,
+    public void testBothDocumentAndDocumentsSpecified() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
             () -> parseQuery("{\"percolate\" : { \"document\": {}, \"documents\": [{}, {}], \"field\":\"" + queryField + "\"}}"));
+        assertThat(e.getMessage(), containsString("The following fields are not allowed together: [document, documents]"));
     }
 
     private static BytesReference randomSource(Set<String> usedFields) {