From 230d7c92099a5f594c219bb678d40b8b04d7f0a6 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Fri, 1 Apr 2022 15:16:07 +0200 Subject: [PATCH] Add mapping exception with JSON path and location (#234) --- .../clients/json/DelegatingJsonParser.java | 124 ++++++++++++++++ .../clients/json/DelegatingJsonpMapper.java | 2 +- .../clients/json/ExternallyTaggedUnion.java | 54 ++++--- .../co/elastic/clients/json/JsonEnum.java | 2 +- .../clients/json/JsonLocationImpl.java | 55 +++++++ .../clients/json/JsonpDeserializer.java | 2 +- .../clients/json/JsonpDeserializerBase.java | 75 +++++----- .../elastic/clients/json/JsonpMapperBase.java | 2 +- .../clients/json/JsonpMappingException.java | 140 ++++++++++++++++++ .../co/elastic/clients/json/JsonpUtils.java | 30 ++-- .../clients/json/NamedDeserializer.java | 5 +- .../clients/json/ObjectDeserializer.java | 114 ++++++++------ .../clients/json/UnionDeserializer.java | 5 +- .../experiments/containers/SomeUnionTest.java | 3 +- .../json/JsonpMappingExceptionTest.java | 112 ++++++++++++++ 15 files changed, 591 insertions(+), 134 deletions(-) create mode 100644 java-client/src/main/java/co/elastic/clients/json/DelegatingJsonParser.java create mode 100644 java-client/src/main/java/co/elastic/clients/json/JsonLocationImpl.java create mode 100644 java-client/src/main/java/co/elastic/clients/json/JsonpMappingException.java create mode 100644 java-client/src/test/java/co/elastic/clients/json/JsonpMappingExceptionTest.java diff --git a/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonParser.java b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonParser.java new file mode 100644 index 000000000..8f880f0b0 --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonParser.java @@ -0,0 +1,124 @@ +/* + * 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.JsonArray; +import jakarta.json.JsonObject; +import jakarta.json.JsonValue; +import jakarta.json.stream.JsonLocation; +import jakarta.json.stream.JsonParser; + +import java.math.BigDecimal; +import java.util.Map; +import java.util.stream.Stream; + +public abstract class DelegatingJsonParser implements JsonParser { + + private final JsonParser parser; + + public DelegatingJsonParser(JsonParser parser) { + this.parser = parser; + } + + @Override + public boolean hasNext() { + return parser.hasNext(); + } + + @Override + public Event next() { + return parser.next(); + } + + @Override + public String getString() { + return parser.getString(); + } + + @Override + public boolean isIntegralNumber() { + return parser.isIntegralNumber(); + } + + @Override + public int getInt() { + return parser.getInt(); + } + + @Override + public long getLong() { + return parser.getLong(); + } + + @Override + public BigDecimal getBigDecimal() { + return parser.getBigDecimal(); + } + + @Override + public JsonLocation getLocation() { + return parser.getLocation(); + } + + @Override + public JsonObject getObject() { + return parser.getObject(); + } + + @Override + public JsonValue getValue() { + return parser.getValue(); + } + + @Override + public JsonArray getArray() { + return parser.getArray(); + } + + @Override + public Stream getArrayStream() { + return parser.getArrayStream(); + } + + @Override + public Stream> getObjectStream() { + return parser.getObjectStream(); + } + + @Override + public Stream getValueStream() { + return parser.getValueStream(); + } + + @Override + public void skipArray() { + parser.skipArray(); + } + + @Override + public void skipObject() { + parser.skipObject(); + } + + @Override + public void close() { + parser.close(); + } +} diff --git a/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonpMapper.java b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonpMapper.java index 708bd442a..c4b69a647 100644 --- a/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonpMapper.java +++ b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonpMapper.java @@ -25,7 +25,7 @@ import javax.annotation.Nullable; -public class DelegatingJsonpMapper implements JsonpMapper { +public abstract class DelegatingJsonpMapper implements JsonpMapper { protected final JsonpMapper mapper; diff --git a/java-client/src/main/java/co/elastic/clients/json/ExternallyTaggedUnion.java b/java-client/src/main/java/co/elastic/clients/json/ExternallyTaggedUnion.java index e83ebb771..ec5d52111 100644 --- a/java-client/src/main/java/co/elastic/clients/json/ExternallyTaggedUnion.java +++ b/java-client/src/main/java/co/elastic/clients/json/ExternallyTaggedUnion.java @@ -22,7 +22,6 @@ import co.elastic.clients.util.TaggedUnion; import jakarta.json.stream.JsonGenerator; import jakarta.json.stream.JsonParser; -import jakarta.json.stream.JsonParsingException; import java.util.ArrayList; import java.util.EnumSet; @@ -62,7 +61,7 @@ public Deserializer(Map> deserialize public Union deserialize(String type, JsonParser parser, JsonpMapper mapper, Event event) { JsonpDeserializer deserializer = deserializers.get(type); if (deserializer == null) { - throw new JsonParsingException("Unknown variant type '" + type + "'", parser.getLocation()); + throw new JsonpMappingException("Unknown variant type '" + type + "'", parser.getLocation()); } return unionCtor.apply(type, deserializer.deserialize(parser, mapper, event)); @@ -97,7 +96,7 @@ public Map deserialize(JsonParser parser, JsonpMapper mapper, Eve public void deserializeEntry(String key, JsonParser parser, JsonpMapper mapper, Map targetMap) { int hashPos = key.indexOf('#'); if (hashPos == -1) { - throw new JsonParsingException( + throw new JsonpMappingException( "Property name '" + key + "' is not in the 'type#name' format. Make sure the request has 'typed_keys' set.", parser.getLocation() ); @@ -117,27 +116,36 @@ public void deserializeEntry(String key, JsonParser parser, JsonpMapper mapper, EnumSet.of(Event.START_OBJECT), (parser, mapper, event) -> { Map> result = new HashMap<>(); - while ((event = parser.next()) != Event.END_OBJECT) { - JsonpUtils.expectEvent(parser, event, Event.KEY_NAME); - // Split key and type - String key = parser.getString(); - int hashPos = key.indexOf('#'); - if (hashPos == -1) { - throw new JsonParsingException( - "Property name '" + key + "' is not in the 'type#name' format. Make sure the request has 'typed_keys' set.", - parser.getLocation() - ); + String key = null; + try { + while ((event = parser.next()) != Event.END_OBJECT) { + JsonpUtils.expectEvent(parser, event, Event.KEY_NAME); + // Split key and type + key = parser.getString(); + int hashPos = key.indexOf('#'); + if (hashPos == -1) { + throw new JsonpMappingException( + "Property name '" + key + "' is not in the 'type#name' format. Make sure the request has 'typed_keys' set.", + parser.getLocation() + ).prepend(null, key); + } + + String type = key.substring(0, hashPos); + String name = key.substring(hashPos + 1); + + List list = new ArrayList<>(); + JsonpUtils.expectNextEvent(parser, Event.START_ARRAY); + try { + while ((event = parser.next()) != Event.END_ARRAY) { + list.add(deserializer.deserializer.deserialize(type, parser, mapper, event)); + } + } catch (Exception e) { + throw JsonpMappingException.from(e, list.size(), parser); + } + result.put(name, list); } - - String type = key.substring(0, hashPos); - String name = key.substring(hashPos + 1); - - List list = new ArrayList<>(); - JsonpUtils.expectNextEvent(parser, Event.START_ARRAY); - while ((event = parser.next()) != Event.END_ARRAY) { - list.add(deserializer.deserializer.deserialize(type, parser, mapper, event)); - } - result.put(name, list); + } catch (Exception e) { + throw JsonpMappingException.from(e, null, key, parser); } return result; } diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonEnum.java b/java-client/src/main/java/co/elastic/clients/json/JsonEnum.java index a4856c971..68d8073ff 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonEnum.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonEnum.java @@ -85,7 +85,7 @@ public T deserialize(JsonParser parser, JsonpMapper mapper, JsonParser.Event eve public T deserialize(String value, JsonParser parser) { T result = this.lookupTable.get(value); if (result == null) { - throw new JsonParsingException("Invalid enum '" + value + "'", parser.getLocation()); + throw new JsonpMappingException("Invalid enum '" + value + "'", parser.getLocation()); } return result; } diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonLocationImpl.java b/java-client/src/main/java/co/elastic/clients/json/JsonLocationImpl.java new file mode 100644 index 000000000..cd3124217 --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/json/JsonLocationImpl.java @@ -0,0 +1,55 @@ +/* + * 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.JsonLocation; + +class JsonLocationImpl implements JsonLocation { + + private final long columnNo; + private final long lineNo; + private final long offset; + + JsonLocationImpl(long lineNo, long columnNo, long streamOffset) { + this.lineNo = lineNo; + this.columnNo = columnNo; + this.offset = streamOffset; + } + + @Override + public long getLineNumber() { + return lineNo; + } + + @Override + public long getColumnNumber() { + return columnNo; + } + + @Override + public long getStreamOffset() { + return offset; + } + + @Override + public String toString() { + return "(line no=" + lineNo + ", column no=" + columnNo + ", offset=" + offset + ")"; + } +} diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializer.java b/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializer.java index 8af9248f1..b8736e2a6 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializer.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializer.java @@ -214,6 +214,6 @@ static JsonpDeserializer> stringMapDeserializer(JsonpDeserial static JsonpDeserializer> enumMapDeserializer( JsonpDeserializer keyDeserializer, JsonpDeserializer valueDeserializer ) { - return new JsonpDeserializerBase.EnumMapDeserializer(keyDeserializer, valueDeserializer); + return new JsonpDeserializerBase.EnumMapDeserializer<>(keyDeserializer, valueDeserializer); } } diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializerBase.java b/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializerBase.java index fae7408a8..bd9b9485c 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializerBase.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpDeserializerBase.java @@ -23,7 +23,6 @@ import jakarta.json.JsonValue; import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParser.Event; -import jakarta.json.stream.JsonParsingException; import java.util.ArrayList; import java.util.Collections; @@ -267,16 +266,12 @@ public JsonValue deserialize(JsonParser parser, JsonpMapper mapper, Event event) }; static final JsonpDeserializer VOID = new JsonpDeserializerBase( - EnumSet.noneOf(Event.class) + EnumSet.allOf(Event.class) ) { - @Override - public Void deserialize(JsonParser parser, JsonpMapper mapper) { - throw new JsonParsingException("Void types should not have any value", parser.getLocation()); - } - @Override public Void deserialize(JsonParser parser, JsonpMapper mapper, Event event) { - return deserialize(parser, mapper); + JsonpUtils.skipValue(parser, event); + return null; } }; @@ -311,14 +306,18 @@ public EnumSet acceptedEvents() { public List deserialize(JsonParser parser, JsonpMapper mapper, Event event) { if (event == Event.START_ARRAY) { List result = new ArrayList<>(); - while ((event = parser.next()) != Event.END_ARRAY) { - // JSON null: add null unless the deserializer can handle it - if (event == Event.VALUE_NULL && !itemDeserializer.accepts(event)) { - result.add(null); - } else { - JsonpUtils.ensureAccepts(itemDeserializer, parser, event); - result.add(itemDeserializer.deserialize(parser, mapper, event)); + try { + while ((event = parser.next()) != Event.END_ARRAY) { + // JSON null: add null unless the deserializer can handle it + if (event == Event.VALUE_NULL && !itemDeserializer.accepts(event)) { + result.add(null); + } else { + JsonpUtils.ensureAccepts(itemDeserializer, parser, event); + result.add(itemDeserializer.deserialize(parser, mapper, event)); + } } + } catch (Exception e) { + throw JsonpMappingException.from(e, result.size(), parser); } return result; } else { @@ -340,11 +339,16 @@ protected StringMapDeserializer(JsonpDeserializer itemDeserializer) { @Override public Map deserialize(JsonParser parser, JsonpMapper mapper, Event event) { Map result = new HashMap<>(); - while ((event = parser.next()) != Event.END_OBJECT) { - JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); - String key = parser.getString(); - T value = itemDeserializer.deserialize(parser, mapper); - result.put(key, value); + String key = null; + try { + while ((event = parser.next()) != Event.END_OBJECT) { + JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); + key = parser.getString(); + T value = itemDeserializer.deserialize(parser, mapper); + result.put(key, value); + } + } catch (Exception e) { + throw JsonpMappingException.from(e, null, key, parser); } return result; } @@ -363,28 +367,19 @@ protected EnumMapDeserializer(JsonpDeserializer keyDeserializer, JsonpDeseria @Override public Map deserialize(JsonParser parser, JsonpMapper mapper, Event event) { Map result = new HashMap<>(); - while ((event = parser.next()) != Event.END_OBJECT) { - JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); - K key = keyDeserializer.deserialize(parser, mapper, event); - V value = valueDeserializer.deserialize(parser, mapper); - result.put(key, value); + String keyName = null; + try { + while ((event = parser.next()) != Event.END_OBJECT) { + JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); + keyName = parser.getString(); + K key = keyDeserializer.deserialize(parser, mapper, event); + V value = valueDeserializer.deserialize(parser, mapper); + result.put(key, value); + } + } catch (Exception e) { + throw JsonpMappingException.from(e, null, keyName, parser); } return result; } } - - static class VoidDeserializer extends JsonpDeserializerBase { - - public static final VoidDeserializer INSTANCE = new VoidDeserializer(); - - VoidDeserializer() { - super(EnumSet.allOf(Event.class)); - } - - @Override - public Void deserialize(JsonParser parser, JsonpMapper mapper, Event event) { - JsonpUtils.skipValue(parser, event); - return null; - } - } } diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpMapperBase.java b/java-client/src/main/java/co/elastic/clients/json/JsonpMapperBase.java index 3f7e6b110..b3204170d 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpMapperBase.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpMapperBase.java @@ -55,7 +55,7 @@ public static JsonpDeserializer findDeserializer(Class clazz) { } if (clazz == Void.class) { - return (JsonpDeserializer)JsonpDeserializerBase.VoidDeserializer.INSTANCE; + return (JsonpDeserializer)JsonpDeserializerBase.VOID; } return null; diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpMappingException.java b/java-client/src/main/java/co/elastic/clients/json/JsonpMappingException.java new file mode 100644 index 000000000..2f7a866b3 --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpMappingException.java @@ -0,0 +1,140 @@ +/* + * 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.JsonLocation; +import jakarta.json.stream.JsonParser; +import jakarta.json.stream.JsonParsingException; + +import java.util.LinkedList; +import java.util.regex.Pattern; + +/** + * A mapping exception. The exception message contains the JSON path and location where the problem happened. + */ +public class JsonpMappingException extends JsonParsingException { + private final LinkedList path = new LinkedList<>(); + private Object ref; + + public JsonpMappingException(String message, JsonLocation location) { + super(message, location); + } + + public JsonpMappingException(String message, Throwable cause, JsonLocation location) { + super(message, cause, location); + } + + public JsonpMappingException(Throwable cause, JsonLocation location) { + super(cause.toString(), cause, location); + } + + private static final Pattern identifier = Pattern.compile("[_a-zA-Z][_a-zA-Z0-9]*"); + + @Override + public String getMessage() { + StringBuilder sb = new StringBuilder("Error deserializing"); + if (ref != null) { + sb.append(' '); + String className = ref.getClass().getName(); + if (className.endsWith("$Builder")) { + sb.append(className, 0, className.length() - "$Builder".length()); + } else { + sb.append(className); + } + } + sb.append(": ").append(super.getMessage()); + + if (!path.isEmpty()) { + sb.append(" (JSON path: "); + path(sb); + sb.append(") "); + } + + sb.append(getLocation()); + return sb.toString(); + } + + /** + * The JSON path where this exception happened. + */ + public String path() { + StringBuilder sb = new StringBuilder(); + path(sb); + return sb.toString(); + } + + // Package-visible for testing + void path(StringBuilder sb) { + String sep = ""; + for (Object item : path) { + if (item instanceof Integer) { + sb.append("[").append(((Integer) item).intValue()).append("]"); + } else { + String str = item.toString(); + if (identifier.matcher(str).matches()) { + sb.append(sep).append(item); + } else { + sb.append("['").append(str).append("']"); + } + } + sep = "."; + } + } + + public JsonpMappingException prepend(Object ref, String name) { + return prepend0(ref, name); + } + + public JsonpMappingException prepend(Object ref, int idx) { + return prepend0(ref, idx); + } + + private JsonpMappingException prepend0 (Object ref, Object pathItem) { + if (pathItem != null) { + this.path.addFirst(pathItem); + } + // Keep the deepest object reference in the JSON hierarchy + if (this.ref == null) { + this.ref = ref; + } + return this; + } + + public static JsonpMappingException from(Throwable cause, Object ref, String name, JsonParser parser) { + return from0(cause, ref, name, parser); + } + + public static JsonpMappingException from(Throwable cause, int index, JsonParser parser) { + return from0(cause, null, index, parser); + } + + private static JsonpMappingException from0(Throwable cause, Object ref, Object pathItem, JsonParser parser) { + JsonpMappingException jme; + + if (cause instanceof JsonpMappingException) { + jme = (JsonpMappingException)cause; + } else { + jme = new JsonpMappingException(cause, parser.getLocation()); + } + + return jme.prepend0(ref, pathItem); + } +} + diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java index 6e53a2aef..655c2c706 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java @@ -20,13 +20,13 @@ package co.elastic.clients.json; import co.elastic.clients.util.AllowForbiddenApis; -import co.elastic.clients.util.ObjectBuilder; import jakarta.json.JsonException; import jakarta.json.JsonObject; import jakarta.json.JsonString; import jakarta.json.JsonValue; import jakarta.json.spi.JsonProvider; import jakarta.json.stream.JsonGenerator; +import jakarta.json.stream.JsonLocation; import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParser.Event; import jakarta.json.stream.JsonParsingException; @@ -130,13 +130,6 @@ public static void skipValue(JsonParser parser, Event event) { } } - public static T buildVariant(JsonParser parser, ObjectBuilder builder) { - if (builder == null) { - throw new JsonParsingException("No variant found" , parser.getLocation()); - } - return builder.build(); - } - public static void serialize(T value, JsonGenerator generator, @Nullable JsonpSerializer serializer, JsonpMapper mapper) { if (serializer != null) { serializer.serialize(value, generator, mapper); @@ -158,7 +151,7 @@ public static Map.Entry 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 - // FIXME: resulting parser should return locations that are offset with the original parser's location + JsonLocation location = parser.getLocation(); JsonObject object = parser.getObject(); String result = object.getString(name, null); @@ -167,10 +160,25 @@ public static Map.Entry lookAheadFieldValue( } if (result == null) { - throw new JsonParsingException("Property '" + name + "' not found", parser.getLocation()); + throw new JsonpMappingException("Property '" + name + "' not found", location); } - return new AbstractMap.SimpleImmutableEntry<>(result, objectParser(object, mapper)); + 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); } /** diff --git a/java-client/src/main/java/co/elastic/clients/json/NamedDeserializer.java b/java-client/src/main/java/co/elastic/clients/json/NamedDeserializer.java index e3ce5e678..bed0a64d9 100644 --- a/java-client/src/main/java/co/elastic/clients/json/NamedDeserializer.java +++ b/java-client/src/main/java/co/elastic/clients/json/NamedDeserializer.java @@ -21,7 +21,6 @@ import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParser.Event; -import jakarta.json.stream.JsonParsingException; import java.util.EnumSet; @@ -60,7 +59,7 @@ public EnumSet acceptedEvents() { public T deserialize(JsonParser parser, JsonpMapper mapper) { JsonpDeserializer deserializer = mapper.attribute(name); if (deserializer == null) { - throw new JsonParsingException("Missing deserializer for generic type: " + name, parser.getLocation()); + throw new JsonpMappingException("Missing deserializer for generic type: " + name, parser.getLocation()); } return deserializer.deserialize(parser, mapper); } @@ -69,7 +68,7 @@ public T deserialize(JsonParser parser, JsonpMapper mapper) { public T deserialize(JsonParser parser, JsonpMapper mapper, JsonParser.Event event) { JsonpDeserializer deserializer = mapper.attribute(name); if (deserializer == null) { - throw new JsonParsingException("Missing deserializer for generic type: " + name, parser.getLocation()); + throw new JsonpMappingException("Missing deserializer for generic type: " + name, parser.getLocation()); } return deserializer.deserialize(parser, mapper, event); } 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 b74bfc7ea..2a35c32de 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 @@ -22,7 +22,6 @@ import co.elastic.clients.util.QuadConsumer; import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParser.Event; -import jakarta.json.stream.JsonParsingException; import javax.annotation.Nullable; import java.util.Collections; @@ -142,59 +141,69 @@ public ObjectType deserialize(ObjectType value, JsonParser parser, JsonpMapper m return null; } - 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); - } - singleKey.deserialize(parser, mapper, null, value, event); - event = parser.next(); - } - - 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); + String keyName = null; + String fieldName = null; - } else 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); - } + try { - if (event == Event.START_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); + } + singleKey.deserialize(parser, mapper, null, value, event); event = parser.next(); } - // Regular object: read all properties until we reach the end of the object - while (event != Event.END_OBJECT) { - JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); - String fieldName = parser.getString(); - FieldDeserializer fieldDeserializer = fieldDeserializers.get(fieldName); + 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); + + } else 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); + } + + if (event == Event.START_OBJECT) { + event = parser.next(); + } + // Regular object: read all properties until we reach the end of the object + while (event != Event.END_OBJECT) { + JsonpUtils.expectEvent(parser, Event.KEY_NAME, event); + fieldName = parser.getString(); + + FieldDeserializer fieldDeserializer = fieldDeserializers.get(fieldName); + if (fieldDeserializer == null) { + parseUnknownField(parser, mapper, fieldName, value); + } else { + fieldDeserializer.deserialize(parser, mapper, fieldName, value); + } + event = parser.next(); + } + fieldName = null; + } else { + // Union variant: find the property to find the proper deserializer + // We cannot start with a key name here. + JsonpUtils.expectEvent(parser, Event.START_OBJECT, event); + Map.Entry unionInfo = JsonpUtils.lookAheadFieldValue(typeProperty, defaultType, parser, mapper); + String variant = unionInfo.getKey(); + JsonParser innerParser = unionInfo.getValue(); + + FieldDeserializer fieldDeserializer = fieldDeserializers.get(variant); if (fieldDeserializer == null) { - parseUnknownField(parser, mapper, fieldName, value); + parseUnknownField(parser, mapper, variant, value); } else { - fieldDeserializer.deserialize(parser, mapper, fieldName, value); + fieldDeserializer.deserialize(innerParser, mapper, variant, value); } - event = parser.next(); - } - } else { - // Union variant: find the property to find the proper deserializer - // We cannot start with a key name here. - JsonpUtils.expectEvent(parser, Event.START_OBJECT, event); - Map.Entry unionInfo = JsonpUtils.lookAheadFieldValue(typeProperty, defaultType, parser, mapper); - String variant = unionInfo.getKey(); - JsonParser innerParser = unionInfo.getValue(); - - FieldDeserializer fieldDeserializer = fieldDeserializers.get(variant); - if (fieldDeserializer == null) { - parseUnknownField(parser, mapper, variant, value); - } else { - fieldDeserializer.deserialize(innerParser, mapper, variant, value); } - } - if (singleKey != null) { - JsonpUtils.expectNextEvent(parser, Event.END_OBJECT); + if (singleKey != null) { + JsonpUtils.expectNextEvent(parser, Event.END_OBJECT); + } + } 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; @@ -208,13 +217,16 @@ protected void parseUnknownField(JsonParser parser, JsonpMapper mapper, String f JsonpUtils.skipValue(parser); } else { - throw new JsonParsingException( - "Unknown field '" + fieldName + "' for type '" + object.getClass().getName() +"'", - parser.getLocation() - ); + // Context is added by the caller + throw new JsonpMappingException("Unknown field '" + fieldName + "'", parser.getLocation()); } } + /** + * Sets a handler for unknown fields. + *

+ * Note: on failure, handlers should not report the field name in their exception: this is handled by the caller. + */ public void setUnknownFieldHandler(QuadConsumer unknownFieldHandler) { this.unknownFieldHandler = unknownFieldHandler; } @@ -265,6 +277,12 @@ public void setKey(BiConsumer setter, JsonpDe public void setTypeProperty(String name, String defaultType) { this.typeProperty = name; this.defaultType = defaultType; + if (this.unknownFieldHandler == null) { + this.unknownFieldHandler = (o, value, parser, mapper) -> { + // Context is added by the caller + throw new JsonpMappingException("Unknown '" + name + "' value: '" + value + "'", parser.getLocation()); + }; + } } //----- Primitive 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 579735da9..8510852b8 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 @@ -23,7 +23,6 @@ import jakarta.json.JsonObject; import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParser.Event; -import jakarta.json.stream.JsonParsingException; import java.util.ArrayList; import java.util.Collections; @@ -102,7 +101,7 @@ Union deserialize(JsonParser parser, JsonpMapper mapper, Event event, BiFunction exception = ex; } } - throw new JsonParsingException("Couldn't find a suitable union member deserializer", exception, parser.getLocation()); + throw JsonpMappingException.from(exception, null, null, parser); } } @@ -286,7 +285,7 @@ public Union deserialize(JsonParser parser, JsonpMapper mapper, Event event) { } if (member == null) { - throw new JsonParsingException("Cannot determine what union member to deserialize", parser.getLocation()); + throw new JsonpMappingException("Cannot determine what union member to deserialize", parser.getLocation()); } return member.deserialize(parser, mapper, event, buildFn); diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/experiments/containers/SomeUnionTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/experiments/containers/SomeUnionTest.java index c82f8c4c3..8481916b0 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/experiments/containers/SomeUnionTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/experiments/containers/SomeUnionTest.java @@ -79,7 +79,6 @@ public void testMissingVariantDeserialization() { SomeUnion c = SomeUnion._DESERIALIZER.deserialize(parser, new JsonbJsonpMapper()); }); - assertEquals("Property 'type' not found", e.getMessage()); + assertTrue(e.getMessage().contains("Property 'type' not found")); } - } diff --git a/java-client/src/test/java/co/elastic/clients/json/JsonpMappingExceptionTest.java b/java-client/src/test/java/co/elastic/clients/json/JsonpMappingExceptionTest.java new file mode 100644 index 000000000..2b9837abd --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/json/JsonpMappingExceptionTest.java @@ -0,0 +1,112 @@ +/* + * 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 co.elastic.clients.elasticsearch._types.mapping.TypeMapping; +import co.elastic.clients.elasticsearch.core.SearchResponse; +import co.elastic.clients.elasticsearch.model.ModelTestCase; +import org.junit.Test; + +import java.io.StringReader; + +public class JsonpMappingExceptionTest extends ModelTestCase { + + @Test + public void testObjectAndArrayPath() { + + String json = "{" + + " \"took\" : 9," + + " \"timed_out\" : false," + + " \"_shards\" : {" + + " \"total\" : 1," + + " \"successful\" : 1," + + " \"skipped\" : 0," + + " \"failed\" : 0" + + " }," + + " \"hits\" : {" + + " \"total\" : {" + + " \"value\" : 1," + + " \"relation\" : \"eq\"" + + " }," + + " \"max_score\" : 1.0," + + " \"hits\" : [" + + " {" + + " \"_index\" : \"test\"," + + " \"_id\" : \"8aSerXUBs1w7Wkuj31zd\"," + + " \"_score\" : \"1.0\"," + + " \"_source\" : {" + + " \"foo\" : \"bar\"" + + " }" + + " }," + + " {" + + " \"_index\" : \"test\"," + + " \"_id\" : \"8aSerXUBs1w7Wkuj31zd\"," + + " \"_score\" : \"abc\"," + // <====== error here + " \"_source\" : {" + + " \"foo\" : \"bar\"" + + " }" + + " }" + + " ]" + + " }" + + "}"; + + JsonpMappingException e = assertThrows(JsonpMappingException.class, () -> { + // withJson() will read values of the generic parameter type as JsonData + SearchResponse r = SearchResponse.searchResponseOf(b -> b + .withJson(new StringReader(json)) + ); + }); + + assertTrue(e.getMessage().contains("Error deserializing co.elastic.clients.elasticsearch.core.search.Hit")); + assertTrue(e.getMessage().contains("java.lang.NumberFormatException")); + + // Also checks array index in path + assertEquals("hits.hits[1]._score", e.path()); + } + + @Test + public void testLookAhead() { + + String json = + "{" + + " \"properties\": { " + + " \"foo-bar\": {" + + " \"type\": \"text\"," + + " \"baz\": false" + + " }" + + " }" + + "}"; + + // 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) + + JsonpMappingException e = assertThrows(JsonpMappingException.class, () -> { + fromJson(json, TypeMapping.class); + }); + + // Check escaping of non identifier path elements and path from map elements + assertEquals("properties['foo-bar'].baz", e.path()); + + 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=")); + } +}