Add buffered lookahead for Jackson (#489)

Author: swallez

java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java

@@ -163,42 +163,53 @@ public static <T> void serialize(T value, JsonGenerator generator, @Nullable Jso
     /**
      * Looks ahead a field value in the Json object from the upcoming object in a parser, which should be on the
      * START_OBJECT event.
-     *
+     * <p>
      * Returns a pair containing that value and a parser that should be used to actually parse the object
      * (the object has been consumed from the original one).
      */
     public static Map.Entry<String, JsonParser> lookAheadFieldValue(
         String name, String defaultValue, JsonParser parser, JsonpMapper mapper
     ) {
-        // FIXME: need a buffering parser wrapper so that we don't roundtrip through a JsonObject and a String
         JsonLocation location = parser.getLocation();
-        JsonObject object = parser.getObject();
-        String result = object.getString(name, null);
 
-        if (result == null) {
-            result = defaultValue;
-        }
+        if (parser instanceof LookAheadJsonParser) {
+            // Fast buffered path
+            Map.Entry<String, JsonParser> result = ((LookAheadJsonParser) parser).lookAheadFieldValue(name, defaultValue);
+            if (result.getKey() == null) {
+                throw new JsonpMappingException("Property '" + name + "' not found", location);
+            }
+            return result;
 
-        if (result == null) {
-            throw new JsonpMappingException("Property '" + name + "' not found", location);
-        }
+        } else {
+            // Unbuffered path: parse the object into a JsonObject, then extract the value and parse it again
+            JsonObject object = parser.getObject();
+            String result = object.getString(name, null);
 
-        JsonParser newParser = objectParser(object, mapper);
+            if (result == null) {
+                result = defaultValue;
+            }
 
-        // Pin location to the start of the look ahead, as the new parser will return locations in its own buffer
-        newParser = new DelegatingJsonParser(newParser) {
-            @Override
-            public JsonLocation getLocation() {
-                return new JsonLocationImpl(location.getLineNumber(), location.getColumnNumber(), location.getStreamOffset()) {
-                    @Override
-                    public String toString() {
-                        return "(in object at " + super.toString().substring(1);
-                    }
-                };
+            if (result == null) {
+                throw new JsonpMappingException("Property '" + name + "' not found", location);
             }
-        };
 
-        return new AbstractMap.SimpleImmutableEntry<>(result, newParser);
+            JsonParser newParser = objectParser(object, mapper);
+
+            // Pin location to the start of the look ahead, as the new parser will return locations in its own buffer
+            newParser = new DelegatingJsonParser(newParser) {
+                @Override
+                public JsonLocation getLocation() {
+                    return new JsonLocationImpl(location.getLineNumber(), location.getColumnNumber(), location.getStreamOffset()) {
+                        @Override
+                        public String toString() {
+                            return "(in object at " + super.toString().substring(1);
+                        }
+                    };
+                }
+            };
+
+            return new AbstractMap.SimpleImmutableEntry<>(result, newParser);
+        }
     }
 
     /**

java-client/src/main/java/co/elastic/clients/json/LookAheadJsonParser.java

@@ -0,0 +1,46 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package co.elastic.clients.json;
+
+import jakarta.json.stream.JsonParser;
+
+import java.util.Map;
+
+public interface LookAheadJsonParser extends JsonParser {
+
+    /**
+     * Look ahead the value of a text property in the JSON stream. The parser must be on the {@code START_OBJECT} event.
+     *
+     * @param name the field name to look up.
+     * @param defaultValue default value if the field is not found.
+     * @return a pair containing the field value (or {@code null} if not found), and a parser to be used to read the JSON object.
+     */
+    Map.Entry<String, JsonParser> lookAheadFieldValue(String name, String defaultValue);
+
+    /**
+     * In union types, find the variant to be used by looking up property names in the JSON stream until we find one that
+     * uniquely identifies the variant.
+     *
+     * @param <Variant> the type of variant descriptors used by the caller.
+     * @param variants a map of variant descriptors, keyed by the property name that uniquely identifies the variant.
+     * @return a pair containing the variant descriptor (or {@code null} if not found), and a parser to be used to read the JSON object.
+     */
+    <Variant> Map.Entry<Variant, JsonParser> findVariant(Map<String, Variant> variants);
+}

java-client/src/main/java/co/elastic/clients/json/UnionDeserializer.java

@@ -21,6 +21,7 @@
 
 import co.elastic.clients.util.ObjectBuilder;
 import jakarta.json.JsonObject;
+import jakarta.json.stream.JsonLocation;
 import jakarta.json.stream.JsonParser;
 import jakarta.json.stream.JsonParser.Event;
 
@@ -203,12 +204,12 @@ public JsonpDeserializer<Union> build() {
     private final BiFunction<Kind, Member, Union> buildFn;
     private final EnumSet<Event> nativeEvents;
     private final Map<String, EventHandler<Union, Kind, Member>> objectMembers;
-    private final Map<Event, EventHandler<Union, Kind, Member>> otherMembers;
+    private final Map<Event, EventHandler<Union, Kind, Member>> nonObjectMembers;
     private final EventHandler<Union, Kind, Member> fallbackObjectMember;
 
     public UnionDeserializer(
         List<SingleMemberHandler<Union, Kind, Member>> objectMembers,
-        Map<Event, EventHandler<Union, Kind, Member>> otherMembers,
+        Map<Event, EventHandler<Union, Kind, Member>> nonObjectMembers,
         BiFunction<Kind, Member, Union> buildFn
     ) {
         this.buildFn = buildFn;
@@ -225,17 +226,17 @@ public UnionDeserializer(
             }
         }
 
-        this.otherMembers = otherMembers;
+        this.nonObjectMembers = nonObjectMembers;
 
         this.nativeEvents = EnumSet.noneOf(Event.class);
-        for (EventHandler<Union, Kind, Member> member: otherMembers.values()) {
+        for (EventHandler<Union, Kind, Member> member: nonObjectMembers.values()) {
             this.nativeEvents.addAll(member.nativeEvents());
         }
 
         if (objectMembers.isEmpty()) {
             fallbackObjectMember = null;
         } else {
-            fallbackObjectMember = this.otherMembers.remove(Event.START_OBJECT);
+            fallbackObjectMember = this.nonObjectMembers.remove(Event.START_OBJECT);
             this.nativeEvents.add(Event.START_OBJECT);
         }
     }
@@ -260,32 +261,44 @@ public Union deserialize(JsonParser parser, JsonpMapper mapper) {
 
     @Override
     public Union deserialize(JsonParser parser, JsonpMapper mapper, Event event) {
-        EventHandler<Union, Kind, Member> member = otherMembers.get(event);
+        EventHandler<Union, Kind, Member> member = nonObjectMembers.get(event);
+        JsonLocation location = parser.getLocation();
 
         if (member == null && event == Event.START_OBJECT && !objectMembers.isEmpty()) {
-            // Parse as an object to find matching field names
-            JsonObject object = parser.getObject();
+            if (parser instanceof LookAheadJsonParser) {
+                Map.Entry<EventHandler<Union, Kind, Member>, JsonParser> memberAndParser =
+                    ((LookAheadJsonParser) parser).findVariant(objectMembers);
 
-            for (String field: object.keySet()) {
-                member = objectMembers.get(field);
-                if (member != null) {
-                    break;
+                member = memberAndParser.getKey();
+                // Parse the buffered parser
+                parser = memberAndParser.getValue();
+
+            } else {
+                // Parse as an object to find matching field names
+                JsonObject object = parser.getObject();
+
+                for (String field: object.keySet()) {
+                    member = objectMembers.get(field);
+                    if (member != null) {
+                        break;
+                    }
                 }
+
+                // Traverse the object we have inspected
+                parser = JsonpUtils.objectParser(object, mapper);
             }
 
             if (member == null) {
                 member = fallbackObjectMember;
             }
 
             if (member != null) {
-                // Traverse the object we have inspected
-                parser = JsonpUtils.objectParser(object, mapper);
                 event = parser.next();
             }
         }
 
         if (member == null) {
-            throw new JsonpMappingException("Cannot determine what union member to deserialize", parser.getLocation());
+            throw new JsonpMappingException("Cannot determine what union member to deserialize", location);
         }
 
         return member.deserialize(parser, mapper, event, buildFn);

java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java

@@ -19,7 +19,11 @@
 
 package co.elastic.clients.json.jackson;
 
+import co.elastic.clients.json.LookAheadJsonParser;
+import co.elastic.clients.json.UnexpectedJsonEventException;
 import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.core.util.JsonParserSequence;
+import com.fasterxml.jackson.databind.util.TokenBuffer;
 import jakarta.json.JsonArray;
 import jakarta.json.JsonObject;
 import jakarta.json.JsonValue;
@@ -29,6 +33,7 @@
 
 import java.io.IOException;
 import java.math.BigDecimal;
+import java.util.AbstractMap;
 import java.util.EnumMap;
 import java.util.Map;
 import java.util.NoSuchElementException;
@@ -42,7 +47,7 @@
  * getter method (e.g. {@link #getInt()} or {@link #getString()} should be called until the next call to {@link #next()}.
  * Such calls will throw an {@code IllegalStateException}.
  */
-public class JacksonJsonpParser implements JsonParser {
+public class JacksonJsonpParser implements LookAheadJsonParser {
 
     private final com.fasterxml.jackson.core.JsonParser parser;
 
@@ -306,7 +311,100 @@ public Stream<JsonValue> getArrayStream() {
      */
     @Override
     public Stream<JsonValue> getValueStream() {
-        return JsonParser.super.getValueStream();
+        return LookAheadJsonParser.super.getValueStream();
+    }
+
+    //----- Look ahead methods
+
+    public Map.Entry<String, JsonParser> lookAheadFieldValue(String name, String defaultValue) {
+
+        TokenBuffer tb = new TokenBuffer(parser, null);
+
+        try {
+            // The resulting parser must contain the full object, including START_EVENT
+            tb.copyCurrentEvent(parser);
+            while (parser.nextToken() != JsonToken.END_OBJECT) {
+
+                expectEvent(JsonToken.FIELD_NAME);
+                // Do not copy current event here, each branch will take care of it
+
+                String fieldName = parser.getCurrentName();
+                if (fieldName.equals(name)) {
+                    // Found
+                    tb.copyCurrentEvent(parser);
+                    expectNextEvent(JsonToken.VALUE_STRING);
+                    tb.copyCurrentEvent(parser);
+
+                    return new AbstractMap.SimpleImmutableEntry<>(
+                        parser.getText(),
+                        new JacksonJsonpParser(JsonParserSequence.createFlattened(false, tb.asParser(), parser))
+                    );
+                } else {
+                    tb.copyCurrentStructure(parser);
+                }
+            }
+            // Copy ending END_OBJECT
+            tb.copyCurrentEvent(parser);
+        } catch (IOException e) {
+            throw JacksonUtils.convertException(e);
+        }
+
+        // Field not found
+        return new AbstractMap.SimpleImmutableEntry<>(
+            defaultValue,
+            new JacksonJsonpParser(JsonParserSequence.createFlattened(false, tb.asParser(), parser))
+        );
+    }
+
+    @Override
+    public <Variant> Map.Entry<Variant, JsonParser> findVariant(Map<String, Variant> variants) {
+        // We're on a START_OBJECT event
+        TokenBuffer tb = new TokenBuffer(parser, null);
+
+        try {
+            // The resulting parser must contain the full object, including START_EVENT
+            tb.copyCurrentEvent(parser);
+            while (parser.nextToken() != JsonToken.END_OBJECT) {
+
+                expectEvent(JsonToken.FIELD_NAME);
+                String fieldName = parser.getCurrentName();
+
+                Variant variant = variants.get(fieldName);
+                if (variant != null) {
+                    tb.copyCurrentEvent(parser);
+                    return new AbstractMap.SimpleImmutableEntry<>(
+                        variant,
+                        new JacksonJsonpParser(JsonParserSequence.createFlattened(false, tb.asParser(), parser))
+                    );
+                } else {
+                    tb.copyCurrentStructure(parser);
+                }
+            }
+            // Copy ending END_OBJECT
+            tb.copyCurrentEvent(parser);
+        } catch (IOException e) {
+            throw JacksonUtils.convertException(e);
+        }
+
+        // No variant found: return the buffered parser and let the caller decide what to do.
+        return new AbstractMap.SimpleImmutableEntry<>(
+            null,
+            new JacksonJsonpParser(JsonParserSequence.createFlattened(false, tb.asParser(), parser))
+        );
+    }
+
+    private void expectNextEvent(JsonToken expected) throws IOException {
+        JsonToken event = parser.nextToken();
+        if (event != expected) {
+            throw new UnexpectedJsonEventException(this, tokenToEvent.get(event), tokenToEvent.get(expected));
+        }
+    }
+
+    private void expectEvent(JsonToken expected) {
+        JsonToken event = parser.currentToken();
+        if (event != expected) {
+            throw new UnexpectedJsonEventException(this, tokenToEvent.get(event), tokenToEvent.get(expected));
+        }
     }
 }
 

java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java

@@ -185,6 +185,14 @@ public void testNestedTaggedUnionWithDefaultTag() {
         assertEquals(256, mappings.properties().get("id").text().fields().get("keyword").keyword().ignoreAbove().longValue());
     }
 
+    @Test
+    public void testEmptyProperty() {
+        // Edge case where we have a property with no fields and no type
+        String json = "{}";
+        Property property = fromJson(json, Property.class);
+        assertEquals(Property.Kind.Object, property._kind());
+    }
+
     @Test
     public void testNestedVariantsWithContainerProperties() {
 

java-client/src/test/java/co/elastic/clients/json/JsonpMappingExceptionTest.java

@@ -95,7 +95,7 @@ public void testLookAhead() {
             "}";
 
         // Error deserializing co.elastic.clients.elasticsearch._types.mapping.TextProperty:
-        // Unknown field 'baz' (JSON path: properties['foo-bar'].baz) (in object at line no=1, column no=36, offset=35)
+        // Unknown field 'baz' (JSON path: properties['foo-bar'].baz) (...line no=1, column no=36, offset=35)
 
         JsonpMappingException e = assertThrows(JsonpMappingException.class, () -> {
             fromJson(json, TypeMapping.class);
@@ -106,7 +106,5 @@ public void testLookAhead() {
 
         String msg = e.getMessage();
         assertTrue(msg.contains("Unknown field 'baz'"));
-        // Check look ahead position (see JsonpUtils.lookAheadFieldValue)
-        assertTrue(msg.contains("(in object at line no="));
     }
 }