From db60266de7e2f68310fc81ce3aeacc1d6e2fd589 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Thu, 22 Aug 2024 14:53:28 +0200 Subject: [PATCH] Add support for shortcut properties of non-primitive type (#858) Co-authored-by: Laura Trotta <153528055+l-trotta@users.noreply.github.com> --- .../_types/query_dsl/FunctionScoreQuery.java | 2 + .../co/elastic/clients/json/JsonpUtils.java | 48 +++++++- .../clients/json/ObjectDeserializer.java | 114 ++++++++++++++---- .../clients/json/UnionDeserializer.java | 27 +---- .../json/jackson/JacksonJsonpParser.java | 47 ++++---- .../util/WithJsonObjectBuilderBase.java | 3 +- .../elasticsearch/model/BehaviorsTest.java | 8 +- .../json/jackson/JsonValueParserTest.java | 2 +- 8 files changed, 175 insertions(+), 76 deletions(-) diff --git a/java-client/src/main/java/co/elastic/clients/elasticsearch/_types/query_dsl/FunctionScoreQuery.java b/java-client/src/main/java/co/elastic/clients/elasticsearch/_types/query_dsl/FunctionScoreQuery.java index a6f704171..625c17a2c 100644 --- a/java-client/src/main/java/co/elastic/clients/elasticsearch/_types/query_dsl/FunctionScoreQuery.java +++ b/java-client/src/main/java/co/elastic/clients/elasticsearch/_types/query_dsl/FunctionScoreQuery.java @@ -325,6 +325,8 @@ protected static void setupFunctionScoreQueryDeserializer(ObjectDeserializer lookAheadFieldValue( throw new JsonpMappingException("Property '" + name + "' not found", location); } - JsonParser newParser = objectParser(object, mapper); + JsonParser newParser = jsonValueParser(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) { @@ -272,16 +272,60 @@ public String toString() { } } + /** + * 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 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. + */ + + public static Map.Entry findVariant( + Map variants, JsonParser parser, JsonpMapper mapper + ) { + if (parser instanceof LookAheadJsonParser) { + return ((LookAheadJsonParser) parser).findVariant(variants); + } else { + // If it's an object, find matching field names + Variant variant = null; + JsonValue value = parser.getValue(); + + if (value instanceof JsonObject) { + for (String field: value.asJsonObject().keySet()) { + variant = variants.get(field); + if (variant != null) { + break; + } + } + } + + // Traverse the object we have inspected + parser = JsonpUtils.jsonValueParser(value, mapper); + return new AbstractMap.SimpleImmutableEntry<>(variant, parser); + } + } + /** * Create a parser that traverses a JSON object + * + * @deprecated use {@link #jsonValueParser(JsonValue, JsonpMapper)} */ + @Deprecated public static JsonParser objectParser(JsonObject object, JsonpMapper mapper) { + return jsonValueParser(object, mapper); + } + + /** + * Create a parser that traverses a JSON value + */ + public static JsonParser jsonValueParser(JsonValue value, JsonpMapper mapper) { // FIXME: we should have used createParser(object), but this doesn't work as it creates a // org.glassfish.json.JsonStructureParser that doesn't implement the JsonP 1.0.1 features, in particular // parser.getObject(). So deserializing recursive internally-tagged union would fail with UnsupportedOperationException // While glassfish has this issue or until we write our own, we roundtrip through a string. - String strObject = object.toString(); + String strObject = value.toString(); return mapper.jsonProvider().createParser(new StringReader(strObject)); } diff --git a/java-client/src/main/java/co/elastic/clients/json/ObjectDeserializer.java b/java-client/src/main/java/co/elastic/clients/json/ObjectDeserializer.java index 8f60b23a8..f1eaba697 100644 --- a/java-client/src/main/java/co/elastic/clients/json/ObjectDeserializer.java +++ b/java-client/src/main/java/co/elastic/clients/json/ObjectDeserializer.java @@ -106,7 +106,6 @@ public EnumSet acceptedEvents() { //--------------------------------------------------------------------------------------------- private static final EnumSet EventSetObject = EnumSet.of(Event.START_OBJECT, Event.KEY_NAME); - private static final EnumSet EventSetObjectAndString = EnumSet.of(Event.START_OBJECT, Event.VALUE_STRING, Event.KEY_NAME); private EnumSet acceptedEvents = EventSetObject; // May be changed in `shortcutProperty()` private final Supplier constructor; @@ -115,6 +114,7 @@ public EnumSet acceptedEvents() { private String typeProperty; private String defaultType; private FieldDeserializer shortcutProperty; + private boolean shortcutIsObject; private QuadConsumer unknownFieldHandler; public ObjectDeserializer(Supplier constructor) { @@ -133,6 +133,10 @@ public Set fieldNames() { return this.shortcutProperty == null ? null : this.shortcutProperty.name; } + public boolean shortcutIsObject() { + return this.shortcutIsObject; + } + @Override public EnumSet nativeEvents() { // May also return string if we have a shortcut property. This is needed to identify ambiguous unions. @@ -145,33 +149,51 @@ public EnumSet acceptedEvents() { } public ObjectType deserialize(JsonParser parser, JsonpMapper mapper, Event event) { - return deserialize(constructor.get(), parser, mapper, event); - } - - public ObjectType deserialize(ObjectType value, JsonParser parser, JsonpMapper mapper, Event event) { if (event == Event.VALUE_NULL) { return null; } - String keyName = null; - String fieldName = null; + ObjectType value = constructor.get(); + deserialize(value, parser, mapper, event); + return value; + } - try { + public void deserialize(ObjectType value, JsonParser parser, JsonpMapper mapper, Event event) { + // Note: method is public as it's called by `withJson` to augment an already created object - if (singleKey != null) { - // There's a wrapping property whose name is the key value - if (event == Event.START_OBJECT) { - event = JsonpUtils.expectNextEvent(parser, Event.KEY_NAME); - } + if (singleKey == null) { + // Nominal case + deserializeInner(value, parser, mapper, event); + + } else { + // Single key dictionary: there's a wrapping property whose name is the key value + if (event == Event.START_OBJECT) { + event = JsonpUtils.expectNextEvent(parser, Event.KEY_NAME); + } + + String keyName = parser.getString(); + try { singleKey.deserialize(parser, mapper, null, value, event); event = parser.next(); + deserializeInner(value, parser, mapper, event); + } catch (Exception e) { + throw JsonpMappingException.from(e, value, keyName, parser); } - if (shortcutProperty != null && event != Event.START_OBJECT && event != Event.KEY_NAME) { - // This is the shortcut property (should be a value event, this will be checked by its deserializer) - shortcutProperty.deserialize(parser, mapper, shortcutProperty.name, value, event); + JsonpUtils.expectNextEvent(parser, Event.END_OBJECT); + } + } + + private void deserializeInner(ObjectType value, JsonParser parser, JsonpMapper mapper, Event event) { + String fieldName = null; - } else if (typeProperty == null) { + try { + if ((parser = deserializeWithShortcut(value, parser, mapper, event)) == null) { + // We found the shortcut form + return; + } + + if (typeProperty == null) { if (event != Event.START_OBJECT && event != Event.KEY_NAME) { // Report we're waiting for a start_object, since this is the most common beginning for object parser JsonpUtils.expectEvent(parser, Event.START_OBJECT, event); @@ -209,16 +231,52 @@ public ObjectType deserialize(ObjectType value, JsonParser parser, JsonpMapper m fieldDeserializer.deserialize(innerParser, mapper, variant, value); } } + } catch (Exception e) { + // Add field name if present + throw JsonpMappingException.from(e, value, fieldName, parser); + } + } - if (singleKey != null) { - JsonpUtils.expectNextEvent(parser, Event.END_OBJECT); + /** + * Try to deserialize the value with its shortcut property, if any. + * + * @return {@code null} if the shortcut form was found, and otherwise a parser that should be used to parse the + * non-shortcut form. It may be different from the orginal parser if look-ahead was needed. + */ + private JsonParser deserializeWithShortcut(ObjectType value, JsonParser parser, JsonpMapper mapper, Event event) { + if (shortcutProperty != null) { + if (!shortcutIsObject) { + if (event != Event.START_OBJECT && event != Event.KEY_NAME) { + // This is the shortcut property (should be a value or array event, this will be checked by its deserializer) + shortcutProperty.deserialize(parser, mapper, shortcutProperty.name, value, event); + return null; + } + } else { + // Fast path: we don't need to look ahead if the current event isn't an object + if (event != Event.START_OBJECT) { + shortcutProperty.deserialize(parser, mapper, shortcutProperty.name, value, event); + return null; + } + + // Look ahead: does the shortcut property exist? If yes, the shortcut is used + Map.Entry shortcut = JsonpUtils.findVariant( + Collections.singletonMap(shortcutProperty.name, Boolean.TRUE /* arbitrary non-null value */), + parser, mapper + ); + + // Parse the buffered events + parser = shortcut.getValue(); + event = parser.next(); + + // If shortcut property was not found, this is a shortcut. Otherwise, keep deserializing as usual + if (shortcut.getKey() == null) { + shortcutProperty.deserialize(parser, mapper, shortcutProperty.name, value, event); + return null; + } } - } catch (Exception e) { - // Add key name (for single key dicts) and field name if present - throw JsonpMappingException.from(e, value, fieldName, parser).prepend(value, keyName); } - return value; + return parser; } protected void parseUnknownField(JsonParser parser, JsonpMapper mapper, String fieldName, ObjectType object) { @@ -249,14 +307,18 @@ public void ignore(String name) { } public void shortcutProperty(String name) { + shortcutProperty(name, false); + } + + public void shortcutProperty(String name, boolean isObject) { this.shortcutProperty = this.fieldDeserializers.get(name); if (this.shortcutProperty == null) { throw new NoSuchElementException("No deserializer was setup for '" + name + "'"); } - //acceptedEvents = EnumSet.copyOf(acceptedEvents); - //acceptedEvents.addAll(shortcutProperty.acceptedEvents()); - acceptedEvents = EventSetObjectAndString; + acceptedEvents = EnumSet.copyOf(acceptedEvents); + acceptedEvents.addAll(shortcutProperty.acceptedEvents()); + this.shortcutIsObject = isObject; } //----- Object types diff --git a/java-client/src/main/java/co/elastic/clients/json/UnionDeserializer.java b/java-client/src/main/java/co/elastic/clients/json/UnionDeserializer.java index bf51efb33..d6ad6c9b2 100644 --- a/java-client/src/main/java/co/elastic/clients/json/UnionDeserializer.java +++ b/java-client/src/main/java/co/elastic/clients/json/UnionDeserializer.java @@ -20,7 +20,6 @@ package co.elastic.clients.json; 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; @@ -265,28 +264,12 @@ public Union deserialize(JsonParser parser, JsonpMapper mapper, Event event) { JsonLocation location = parser.getLocation(); if (member == null && event == Event.START_OBJECT && !objectMembers.isEmpty()) { - if (parser instanceof LookAheadJsonParser) { - Map.Entry, JsonParser> memberAndParser = - ((LookAheadJsonParser) parser).findVariant(objectMembers); + Map.Entry, JsonParser> memberAndParser = + JsonpUtils.findVariant(objectMembers, parser, mapper); - 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); - } + member = memberAndParser.getKey(); + // Parse the buffered parser + parser = memberAndParser.getValue(); if (member == null) { member = fallbackObjectMember; diff --git a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java index 22c841a26..4a7676ebb 100644 --- a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java +++ b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java @@ -372,29 +372,34 @@ public Map.Entry findVariant(Map 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), - mapper - ) - ); - } else { - tb.copyCurrentStructure(parser); + if (parser.currentToken() != JsonToken.START_OBJECT) { + // Primitive value or array + tb.copyCurrentStructure(parser); + } else { + // 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), + mapper + ) + ); + } else { + tb.copyCurrentStructure(parser); + } } + // Copy ending END_OBJECT + tb.copyCurrentEvent(parser); } - // Copy ending END_OBJECT - tb.copyCurrentEvent(parser); } catch (IOException e) { throw JacksonUtils.convertException(e); } diff --git a/java-client/src/main/java/co/elastic/clients/util/WithJsonObjectBuilderBase.java b/java-client/src/main/java/co/elastic/clients/util/WithJsonObjectBuilderBase.java index fc40dc3dc..811f65fc1 100644 --- a/java-client/src/main/java/co/elastic/clients/util/WithJsonObjectBuilderBase.java +++ b/java-client/src/main/java/co/elastic/clients/util/WithJsonObjectBuilderBase.java @@ -51,7 +51,8 @@ public B withJson(JsonParser parser, JsonpMapper mapper) { @SuppressWarnings("unchecked") ObjectDeserializer builderDeser = (ObjectDeserializer) DelegatingDeserializer.unwrap(classDeser); - return builderDeser.deserialize(self(), parser, mapper, parser.next()); + builderDeser.deserialize(self(), parser, mapper, parser.next()); + return self(); } private static class WithJsonMapper extends DelegatingJsonpMapper { diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java index 692302077..5de55665b 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java @@ -20,12 +20,15 @@ package co.elastic.clients.elasticsearch.model; import co.elastic.clients.elasticsearch._types.ErrorCause; +import co.elastic.clients.elasticsearch._types.FieldSort; import co.elastic.clients.elasticsearch._types.FieldValue; import co.elastic.clients.elasticsearch._types.GeoLocation; import co.elastic.clients.elasticsearch._types.GeoShapeRelation; import co.elastic.clients.elasticsearch._types.SortOptions; import co.elastic.clients.elasticsearch._types.SortOptionsBuilders; import co.elastic.clients.elasticsearch._types.SortOrder; +import co.elastic.clients.elasticsearch._types.query_dsl.FunctionScoreQuery; +import co.elastic.clients.elasticsearch._types.query_dsl.MultiValueMode; import co.elastic.clients.elasticsearch._types.query_dsl.Query; import co.elastic.clients.elasticsearch._types.query_dsl.ShapeQuery; import co.elastic.clients.elasticsearch._types.query_dsl.TermQuery; @@ -71,7 +74,6 @@ public void testAdditionalPropertyOnClass() { assertEquals("query-name", q.queryName()); assertTrue(q.ignoreUnmapped()); assertEquals(GeoShapeRelation.Disjoint, q.shape().relation()); - System.out.println(toJson(q)); } @Test @@ -132,7 +134,6 @@ public void testAdditionalPropertyOnContainer() { } } - @Test public void testAdditionalProperties() { // Check that additional property map is initialized even if not set explicitly @@ -159,7 +160,7 @@ public void testAdditionalProperties() { } @Test - public void testShortcutProperty() { + public void testPrimitiveShortcutProperty() { // All-in-one: a variant, wrapping a single-key dictionary with a shortcut property String json = "{\"term\":{\"some-field\":\"some-value\"}}"; @@ -167,5 +168,6 @@ public void testShortcutProperty() { assertEquals("some-field", q.term().field()); assertEquals("some-value", q.term().value().stringValue()); + } } diff --git a/java-client/src/test/java/co/elastic/clients/json/jackson/JsonValueParserTest.java b/java-client/src/test/java/co/elastic/clients/json/jackson/JsonValueParserTest.java index 7398141aa..0f49a6191 100644 --- a/java-client/src/test/java/co/elastic/clients/json/jackson/JsonValueParserTest.java +++ b/java-client/src/test/java/co/elastic/clients/json/jackson/JsonValueParserTest.java @@ -68,7 +68,7 @@ public void testFloatsShouldDeserializeAsFloats() throws Exception { assertEquals(v.hashCode(), v2.hashCode()); assertEquals(v, v2); - parser = JsonpUtils.objectParser(object, mapper); + parser = JsonpUtils.jsonValueParser(object, mapper); Data data = mapper.deserialize(parser, Data.class); Double d = (Double)data.data.get("value");