Adds declareNamedObjects methods to ConstructingObjectParser (#24219)

colings86 · web-flow · commit 3c7c4bc82452 · 2017-04-21T09:50:30.000+01:00
* Adds declareNamedObjects methods to ConstructingObjectParser

* Addresses review comments
diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java
@@ -22,6 +22,7 @@
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 
@@ -30,6 +31,7 @@
 import java.util.List;
 import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
+import java.util.function.Consumer;
 
 /**
  * Superclass for {@link ObjectParser} and {@link ConstructingObjectParser}. Defines most of the "declare" methods so they can be shared.
@@ -44,6 +46,94 @@ public abstract class AbstractObjectParser<Value, Context>
     public abstract <T> void declareField(BiConsumer<Value, T> consumer, ContextParser<Context, T> parser, ParseField parseField,
             ValueType type);
 
+    /**
+     * Declares named objects in the style of aggregations. These are named
+     * inside and object like this:
+     * 
+     * <pre>
+     * <code>
+     * {
+     *   "aggregations": {
+     *     "name_1": { "aggregation_type": {} },
+     *     "name_2": { "aggregation_type": {} },
+     *     "name_3": { "aggregation_type": {} }
+     *     }
+     *   }
+     * }
+     * </code>
+     * </pre>
+     * 
+     * Unlike the other version of this method, "ordered" mode (arrays of
+     * objects) is not supported.
+     *
+     * See NamedObjectHolder in ObjectParserTests for examples of how to invoke
+     * this.
+     *
+     * @param consumer
+     *            sets the values once they have been parsed
+     * @param namedObjectParser
+     *            parses each named object
+     * @param parseField
+     *            the field to parse
+     */
+    public abstract <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
+            ParseField parseField);
+
+    /**
+     * Declares named objects in the style of highlighting's field element.
+     * These are usually named inside and object like this:
+     * 
+     * <pre>
+     * <code>
+     * {
+     *   "highlight": {
+     *     "fields": {        &lt;------ this one
+     *       "title": {},
+     *       "body": {},
+     *       "category": {}
+     *     }
+     *   }
+     * }
+     * </code>
+     * </pre>
+     * 
+     * but, when order is important, some may be written this way:
+     * 
+     * <pre>
+     * <code>
+     * {
+     *   "highlight": {
+     *     "fields": [        &lt;------ this one
+     *       {"title": {}},
+     *       {"body": {}},
+     *       {"category": {}}
+     *     ]
+     *   }
+     * }
+     * </code>
+     * </pre>
+     * 
+     * This is because json doesn't enforce ordering. Elasticsearch reads it in
+     * the order sent but tools that generate json are free to put object
+     * members in an unordered Map, jumbling them. Thus, if you care about order
+     * you can send the object in the second way.
+     *
+     * See NamedObjectHolder in ObjectParserTests for examples of how to invoke
+     * this.
+     *
+     * @param consumer
+     *            sets the values once they have been parsed
+     * @param namedObjectParser
+     *            parses each named object
+     * @param orderedModeCallback
+     *            called when the named object is parsed using the "ordered"
+     *            mode (the array of objects)
+     * @param parseField
+     *            the field to parse
+     */
+    public abstract <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
+            Consumer<Value> orderedModeCallback, ParseField parseField);
+
     public <T> void declareField(BiConsumer<Value, T> consumer, CheckedFunction<XContentParser, T, IOException> parser,
             ParseField parseField, ValueType type) {
         if (parser == null) {
diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 
 import java.io.IOException;
@@ -77,14 +78,14 @@ public final class ConstructingObjectParser<Value, Context> extends AbstractObje
     /**
      * Consumer that marks a field as a required constructor argument instead of a real object field.
      */
-    private static final BiConsumer<Object, Object> REQUIRED_CONSTRUCTOR_ARG_MARKER = (a, b) -> {
+    private static final BiConsumer<?, ?> REQUIRED_CONSTRUCTOR_ARG_MARKER = (a, b) -> {
         throw new UnsupportedOperationException("I am just a marker I should never be called.");
     };
 
     /**
      * Consumer that marks a field as an optional constructor argument instead of a real object field.
      */
-    private static final BiConsumer<Object, Object> OPTIONAL_CONSTRUCTOR_ARG_MARKER = (a, b) -> {
+    private static final BiConsumer<?, ?> OPTIONAL_CONSTRUCTOR_ARG_MARKER = (a, b) -> {
         throw new UnsupportedOperationException("I am just a marker I should never be called.");
     };
 
@@ -189,7 +190,7 @@ public <T> void declareField(BiConsumer<Value, T> consumer, ContextParser<Contex
 
         if (consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER || consumer == OPTIONAL_CONSTRUCTOR_ARG_MARKER) {
             /*
-             * Constructor arguments are detected by this "marker" consumer. It keeps the API looking clean even if it is a bit sleezy. We
+             * Constructor arguments are detected by these "marker" consumers. It keeps the API looking clean even if it is a bit sleezy. We
              * then build a new consumer directly against the object parser that triggers the "constructor arg just arrived behavior" of the
              * parser. Conveniently, we can close over the position of the constructor in the argument list so we don't need to do any fancy
              * or expensive lookups whenever the constructor args come in.
@@ -204,6 +205,91 @@ public <T> void declareField(BiConsumer<Value, T> consumer, ContextParser<Contex
         }
     }
 
+    @Override
+    public <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
+            ParseField parseField) {
+        if (consumer == null) {
+            throw new IllegalArgumentException("[consumer] is required");
+        }
+        if (namedObjectParser == null) {
+            throw new IllegalArgumentException("[parser] is required");
+        }
+        if (parseField == null) {
+            throw new IllegalArgumentException("[parseField] is required");
+        }
+
+        if (consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER || consumer == OPTIONAL_CONSTRUCTOR_ARG_MARKER) {
+            /*
+             * Constructor arguments are detected by this "marker" consumer. It
+             * keeps the API looking clean even if it is a bit sleezy. We then
+             * build a new consumer directly against the object parser that
+             * triggers the "constructor arg just arrived behavior" of the
+             * parser. Conveniently, we can close over the position of the
+             * constructor in the argument list so we don't need to do any fancy
+             * or expensive lookups whenever the constructor args come in.
+             */
+            int position = constructorArgInfos.size();
+            boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER;
+            constructorArgInfos.add(new ConstructorArgInfo(parseField, required));
+            objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, parseField, v), namedObjectParser, parseField);
+        } else {
+            numberOfFields += 1;
+            objectParser.declareNamedObjects(queueingConsumer(consumer, parseField), namedObjectParser, parseField);
+        }
+    }
+
+    @Override
+    public <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
+            Consumer<Value> orderedModeCallback, ParseField parseField) {
+        if (consumer == null) {
+            throw new IllegalArgumentException("[consumer] is required");
+        }
+        if (namedObjectParser == null) {
+            throw new IllegalArgumentException("[parser] is required");
+        }
+        if (orderedModeCallback == null) {
+            throw new IllegalArgumentException("[orderedModeCallback] is required");
+        }
+        if (parseField == null) {
+            throw new IllegalArgumentException("[parseField] is required");
+        }
+
+        if (consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER || consumer == OPTIONAL_CONSTRUCTOR_ARG_MARKER) {
+            /*
+             * Constructor arguments are detected by this "marker" consumer. It
+             * keeps the API looking clean even if it is a bit sleezy. We then
+             * build a new consumer directly against the object parser that
+             * triggers the "constructor arg just arrived behavior" of the
+             * parser. Conveniently, we can close over the position of the
+             * constructor in the argument list so we don't need to do any fancy
+             * or expensive lookups whenever the constructor args come in.
+             */
+            int position = constructorArgInfos.size();
+            boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER;
+            constructorArgInfos.add(new ConstructorArgInfo(parseField, required));
+            objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, parseField, v), namedObjectParser,
+                    wrapOrderedModeCallBack(orderedModeCallback), parseField);
+        } else {
+            numberOfFields += 1;
+            objectParser.declareNamedObjects(queueingConsumer(consumer, parseField), namedObjectParser,
+                    wrapOrderedModeCallBack(orderedModeCallback), parseField);
+        }
+    }
+
+    private Consumer<Target> wrapOrderedModeCallBack(Consumer<Value> callback) {
+        return (target) -> {
+            if (target.targetObject != null) {
+                // The target has already been built. Call the callback now.
+                callback.accept(target.targetObject);
+                return;
+            }
+            /*
+             * The target hasn't been built. Queue the callback.
+             */
+            target.queuedOrderedModeCallback = callback;
+        };
+    }
+
     /**
      * Creates the consumer that does the "field just arrived" behavior. If the targetObject hasn't been built then it queues the value.
      * Otherwise it just applies the value just like {@linkplain ObjectParser} does.
@@ -258,6 +344,11 @@ private class Target {
          * Fields to be saved to the target object when we can build it. This is only allocated if a field has to be queued.
          */
         private Consumer<Value>[] queuedFields;
+        /**
+         * OrderedModeCallback to be called with the target object when we can
+         * build it. This is only allocated if the callback has to be queued.
+         */
+        private Consumer<Value> queuedOrderedModeCallback;
         /**
          * The count of fields already queued.
          */
@@ -343,6 +434,9 @@ private Value finish() {
         private void buildTarget() {
             try {
                 targetObject = builder.apply(constructorArgs);
+                if (queuedOrderedModeCallback != null) {
+                    queuedOrderedModeCallback.accept(targetObject);
+                }
                 while (queuedFieldsCount > 0) {
                     queuedFieldsCount -= 1;
                     queuedFields[queuedFieldsCount].accept(targetObject);
diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java
@@ -227,41 +227,7 @@ public <T> void declareObjectOrDefault(BiConsumer<Value, T> consumer, BiFunction
         }, field, ValueType.OBJECT_OR_BOOLEAN);
     }
 
-    /**
-     * Declares named objects in the style of highlighting's field element. These are usually named inside and object like this:
-     * <pre><code>
-     * {
-     *   "highlight": {
-     *     "fields": {        &lt;------ this one
-     *       "title": {},
-     *       "body": {},
-     *       "category": {}
-     *     }
-     *   }
-     * }
-     * </code></pre>
-     * but, when order is important, some may be written this way:
-     * <pre><code>
-     * {
-     *   "highlight": {
-     *     "fields": [        &lt;------ this one
-     *       {"title": {}},
-     *       {"body": {}},
-     *       {"category": {}}
-     *     ]
-     *   }
-     * }
-     * </code></pre>
-     * This is because json doesn't enforce ordering. Elasticsearch reads it in the order sent but tools that generate json are free to put
-     * object members in an unordered Map, jumbling them. Thus, if you care about order you can send the object in the second way.
-     *
-     * See NamedObjectHolder in ObjectParserTests for examples of how to invoke this.
-     *
-     * @param consumer sets the values once they have been parsed
-     * @param namedObjectParser parses each named object
-     * @param orderedModeCallback called when the named object is parsed using the "ordered" mode (the array of objects)
-     * @param field the field to parse
-     */
+    @Override
     public <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
             Consumer<Value> orderedModeCallback, ParseField field) {
         // This creates and parses the named object
@@ -311,26 +277,7 @@ public <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedOb
         }, field, ValueType.OBJECT_ARRAY);
     }
 
-    /**
-     * Declares named objects in the style of aggregations. These are named inside and object like this:
-     * <pre><code>
-     * {
-     *   "aggregations": {
-     *     "name_1": { "aggregation_type": {} },
-     *     "name_2": { "aggregation_type": {} },
-     *     "name_3": { "aggregation_type": {} }
-     *     }
-     *   }
-     * }
-     * </code></pre>
-     * Unlike the other version of this method, "ordered" mode (arrays of objects) is not supported.
-     *
-     * See NamedObjectHolder in ObjectParserTests for examples of how to invoke this.
-     *
-     * @param consumer sets the values once they have been parsed
-     * @param namedObjectParser parses each named object
-     * @param field the field to parse
-     */
+    @Override
     public <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
             ParseField field) {
         Consumer<Value> orderedModeCallback = (v) -> {
diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java
