From c5705492399daa9012845f45232675498bcebf23 Mon Sep 17 00:00:00 2001 From: Bryan Harclerode Date: Fri, 21 Feb 2020 00:00:33 -0600 Subject: [PATCH 1/2] [Avro] Remove dependencies upon Jackson 1.X and Avro's JacksonUtils --- .../avro/deser/AvroFieldDefaulters.java | 10 ++-- .../avro/deser/AvroReaderFactory.java | 38 +++++++------- .../avro/schema/AvroSchemaHelper.java | 52 +++++++++++++++++-- .../dataformat/avro/schema/RecordVisitor.java | 41 +++------------ .../avro/interop/ApacheAvroInteropUtil.java | 9 ++++ .../interop/annotations/AvroDefaultTest.java | 2 +- 6 files changed, 90 insertions(+), 62 deletions(-) diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroFieldDefaulters.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroFieldDefaulters.java index b71fbf72d..797f70333 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroFieldDefaulters.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroFieldDefaulters.java @@ -2,7 +2,7 @@ import java.util.*; -import org.codehaus.jackson.JsonNode; +import com.fasterxml.jackson.databind.JsonNode; /** * Factory class for various default providers @@ -19,7 +19,7 @@ public static AvroFieldReader createDefaulter(String name, case VALUE_NULL: return new ScalarDefaults.NullDefaults(name); case VALUE_NUMBER_FLOAT: - switch (defaultAsNode.getNumberType()) { + switch (defaultAsNode.numberType()) { case FLOAT: return new ScalarDefaults.FloatDefaults(name, (float) defaultAsNode.asDouble()); case DOUBLE: @@ -28,7 +28,7 @@ public static AvroFieldReader createDefaulter(String name, return new ScalarDefaults.DoubleDefaults(name, defaultAsNode.asDouble()); } case VALUE_NUMBER_INT: - switch (defaultAsNode.getNumberType()) { + switch (defaultAsNode.numberType()) { case INT: return new ScalarDefaults.FloatDefaults(name, defaultAsNode.asInt()); case BIG_INTEGER: // TODO: maybe support separately? @@ -40,7 +40,7 @@ public static AvroFieldReader createDefaulter(String name, return new ScalarDefaults.StringDefaults(name, defaultAsNode.asText()); case START_OBJECT: { - Iterator> it = defaultAsNode.getFields(); + Iterator> it = defaultAsNode.fields(); List readers = new ArrayList(); while (it.hasNext()) { Map.Entry entry = it.next(); @@ -64,7 +64,7 @@ public static AvroFieldReader createDefaulter(String name, // 23-Jul-2019, tatu: With Avro 1.9, likely changed to use "raw" JDK containers? // Code would look more like this: -/* +/* public static AvroFieldReader createDefaulter(String name, Object defaultValue) { diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java index 517e7fc2c..6cd91be05 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java @@ -3,8 +3,8 @@ import java.util.*; import org.apache.avro.Schema; -import org.apache.avro.util.internal.JacksonUtils; +import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.avro.deser.ScalarDecoder.*; import com.fasterxml.jackson.dataformat.avro.schema.AvroSchemaHelper; @@ -22,6 +22,7 @@ public abstract class AvroReaderFactory protected final static ScalarDecoder READER_LONG = new LongReader(); protected final static ScalarDecoder READER_NULL = new NullReader(); protected final static ScalarDecoder READER_STRING = new StringReader(); + private final static ObjectMapper DEFAULT_MAPPER = new ObjectMapper(); /** * To resolve cyclic types, need to keep track of resolved named @@ -56,24 +57,24 @@ public ScalarDecoder createScalarValueDecoder(Schema type) switch (type.getType()) { case BOOLEAN: return READER_BOOLEAN; - case BYTES: + case BYTES: return READER_BYTES; - case DOUBLE: + case DOUBLE: return READER_DOUBLE; - case ENUM: + case ENUM: return new EnumDecoder(AvroSchemaHelper.getFullName(type), type.getEnumSymbols()); - case FIXED: + case FIXED: return new FixedDecoder(type.getFixedSize(), AvroSchemaHelper.getFullName(type)); - case FLOAT: + case FLOAT: return READER_FLOAT; case INT: if (AvroSchemaHelper.getTypeId(type) != null) { return new IntReader(AvroSchemaHelper.getTypeId(type)); } return READER_INT; - case LONG: + case LONG: return READER_LONG; - case NULL: + case NULL: return READER_NULL; case STRING: if (AvroSchemaHelper.getTypeId(type) != null) { @@ -127,7 +128,7 @@ public AvroStructureReader createReader(Schema schema) switch (schema.getType()) { case ARRAY: return createArrayReader(schema); - case MAP: + case MAP: return createMapReader(schema); case RECORD: return createRecordReader(schema); @@ -252,7 +253,7 @@ public AvroStructureReader createReader(Schema writerSchema, Schema readerSchema switch (writerSchema.getType()) { case ARRAY: return createArrayReader(writerSchema, readerSchema); - case MAP: + case MAP: return createMapReader(writerSchema, readerSchema); case RECORD: return createRecordReader(writerSchema, readerSchema); @@ -303,7 +304,7 @@ protected AvroStructureReader createRecordReader(Schema writerSchema, Schema rea final List writerFields = writerSchema.getFields(); // but first: find fields that only exist in reader-side and need defaults, - // and remove those from + // and remove those from Map readerFields = new HashMap(); List defaultFields = new ArrayList(); { @@ -320,7 +321,7 @@ protected AvroStructureReader createRecordReader(Schema writerSchema, Schema rea } } } - + // note: despite changes, we will always have known number of field entities, // ones from writer schema -- some may skip, but there's entry there AvroFieldReader[] fieldReaders = new AvroFieldReader[writerFields.size() @@ -339,12 +340,13 @@ protected AvroStructureReader createRecordReader(Schema writerSchema, Schema rea : createFieldReader(readerField.name(), writerField.schema(), readerField.schema()); } - + // Any defaults to consider? if (!defaultFields.isEmpty()) { for (Schema.Field defaultField : defaultFields) { - AvroFieldReader fr = - AvroFieldDefaulters.createDefaulter(defaultField.name(), JacksonUtils.toJsonNode(defaultField.defaultVal())); + AvroFieldReader fr = AvroFieldDefaulters.createDefaulter(defaultField.name(), + AvroSchemaHelper.parseDefaultValue(defaultField.defaultVal()) + ); if (fr == null) { throw new IllegalArgumentException("Unsupported default type: "+defaultField.schema().getType()); } @@ -359,9 +361,9 @@ protected AvroStructureReader createUnionReader(Schema writerSchema, Schema read final List types = writerSchema.getTypes(); AvroStructureReader[] typeReaders = new AvroStructureReader[types.size()]; int i = 0; - + // !!! TODO: actual resolution !!! - + for (Schema type : types) { typeReaders[i++] = createReader(type); } @@ -414,7 +416,7 @@ private Schema _verifyMatchingStructure(Schema readerSchema, Schema writerSchema // in case there are multiple alternatives of same structured type. // But since that is quite non-trivial let's wait for a good example of actual // usage before tackling that. - + if (actualType == Schema.Type.UNION) { for (Schema sch : readerSchema.getTypes()) { if (sch.getType() == expectedType) { diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java index ea487b814..e34d9e8e7 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java @@ -14,8 +14,8 @@ import org.apache.avro.specific.SpecificData; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.introspect.AnnotatedClass; import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor; import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonFormatTypes; @@ -23,6 +23,11 @@ public abstract class AvroSchemaHelper { + /** + * Dedicated mapper for handling default values (String <-> JsonNode <-> Object) + */ + private static final ObjectMapper DEFAULT_VALUE_MAPPER = new ObjectMapper(); + /** * Constant used by native Avro Schemas for indicating more specific * physical class of a value; referenced indirectly to reduce direct @@ -317,6 +322,10 @@ public static String getTypeId(Schema schema) { /** * Returns the full name of a schema; This is similar to {@link Schema#getFullName()}, except that it properly handles namespaces for * nested classes. (package.name.ClassName$NestedClassName instead of package.name.ClassName$.NestedClassName) + *

+ * WARNING! This method has to probe for nested classes in order to resolve whether or not a schema references a top-level + * class or a nested class and return the corresponding name for each. + *

*/ public static String getFullName(Schema schema) { switch (schema.getType()) { @@ -332,9 +341,44 @@ public static String getFullName(Schema schema) { return namespace + name; } StringBuilder sb = new StringBuilder(1 + namespace.length() + name.length()); - return sb.append(namespace).append('.').append(name).toString(); - default: + // Check if this is a nested class + String nestedClassName = sb.append(namespace).append('$').append(name).toString(); + try { + Class.forName(nestedClassName); + return nestedClassName; + } catch (ClassNotFoundException e) { + // Could not find a nested class, must be a regular class + sb.setLength(0); + return sb.append(namespace).append('.').append(name).toString(); + } + default: return schema.getType().getName(); } } + + public static JsonNode parseDefaultValue(Object defaultValue) { + return DEFAULT_VALUE_MAPPER.convertValue(defaultValue, JsonNode.class); + } + + public static Object convertDefaultValueToObject(JsonNode defaultJsonValue) { + return DEFAULT_VALUE_MAPPER.convertValue(defaultJsonValue, Object.class); + } + + /** + * Converts a default value represented as a string (such as from a schema specification) into a JsonNode for further handling. + * + * @param defaultValue The default value to parse, in the form of a JSON string + * @return a parsed JSON representation of the default value + * @throws JsonMappingException If {@code defaultValue} is not valid JSON + */ + public static JsonNode parseDefaultValue(String defaultValue) throws JsonMappingException { + if (defaultValue == null) { + return null; + } + try { + return DEFAULT_VALUE_MAPPER.readTree(defaultValue); + } catch (JsonProcessingException e) { + throw new JsonMappingException(null, "Failed to parse default value", e); + } + } } diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java index f1d6867bb..53dd828ee 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java @@ -1,6 +1,5 @@ package com.fasterxml.jackson.dataformat.avro.schema; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -9,9 +8,6 @@ import org.apache.avro.Schema.Type; import org.apache.avro.reflect.AvroMeta; import org.apache.avro.reflect.AvroSchema; -import org.apache.avro.util.internal.JacksonUtils; -import org.codehaus.jackson.JsonNode; -import org.codehaus.jackson.map.ObjectMapper; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonFormatVisitable; @@ -36,9 +32,9 @@ public class RecordVisitor protected final boolean _overridden; protected Schema _avroSchema; - + protected List _fields = new ArrayList(); - + public RecordVisitor(SerializerProvider p, JavaType type, DefinedSchemas schemas) { super(p); @@ -75,7 +71,7 @@ public RecordVisitor(SerializerProvider p, JavaType type, DefinedSchemas schemas } schemas.addSchema(type, _avroSchema); } - + @Override public Schema builtAvroSchema() { if (!_overridden) { @@ -90,7 +86,7 @@ public Schema builtAvroSchema() { /* JsonObjectFormatVisitor implementation /********************************************************** */ - + @Override public void property(BeanProperty writer) throws JsonMappingException { @@ -142,7 +138,7 @@ public void optionalProperty(String name, JsonFormatVisitable handler, /* Internal methods /********************************************************************** */ - + protected Schema.Field schemaFieldForWriter(BeanProperty prop, boolean optional) throws JsonMappingException { Schema writerSchema; @@ -187,10 +183,10 @@ protected Schema.Field schemaFieldForWriter(BeanProperty prop, boolean optional) writerSchema = AvroSchemaHelper.unionWithNull(writerSchema); } } - JsonNode defaultValue = parseJson(prop.getMetadata().getDefaultValue()); + JsonNode defaultValue = AvroSchemaHelper.parseDefaultValue(prop.getMetadata().getDefaultValue()); writerSchema = reorderUnionToMatchDefaultType(writerSchema, defaultValue); Schema.Field field = new Schema.Field(prop.getName(), writerSchema, prop.getMetadata().getDescription(), - JacksonUtils.toObject(defaultValue)); + AvroSchemaHelper.convertDefaultValueToObject(defaultValue)); AvroMeta meta = prop.getAnnotation(AvroMeta.class); if (meta != null) { @@ -206,29 +202,6 @@ protected Schema.Field schemaFieldForWriter(BeanProperty prop, boolean optional) return field; } - /** - * Parses a JSON-encoded string for use as the default value of a field - * - * @param defaultValue - * Default value as a JSON-encoded string - * - * @return Jackson V1 {@link JsonNode} for use as the default value in a {@link Schema.Field} - * - * @throws JsonMappingException - * if {@code defaultValue} is not valid JSON - */ - protected JsonNode parseJson(String defaultValue) throws JsonMappingException { - if (defaultValue == null) { - return null; - } - ObjectMapper mapper = new ObjectMapper(); - try { - return mapper.readTree(defaultValue); - } catch (IOException e) { - throw JsonMappingException.from(getProvider(), "Unable to parse default value as JSON: " + defaultValue, e); - } - } - /** * A union schema with a default value must always have the schema branch corresponding to the default value first, or Avro will print a * warning complaining that the default value is not compatible. If {@code schema} is a {@link Schema.Type#UNION UNION} schema and diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/ApacheAvroInteropUtil.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/ApacheAvroInteropUtil.java index 8ad6f9123..b96f6466d 100644 --- a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/ApacheAvroInteropUtil.java +++ b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/ApacheAvroInteropUtil.java @@ -126,6 +126,15 @@ protected Schema createSchema(Type type, Map names) { /* 14-Jun-2018, tatu: This is 99.9% certainly wrong; IDE gives warnings and * it just... doesn't fly. Either keys are NOT Strings or... */ + /* + * 26-Jun-2019, baharclerode: The keys are NOT always strings. It's a horrible hack I used to enable support for generics + * in the apache implementation because otherwise it flat out doesn't support generic types correctly except for the handful + * that they hand-coded support for, resulting in oddities like Set not working, or List working but LinkedList not + * working. This regression suite is a lot simpler when we don't have to disable half the tests because the reference + * implementation doesn't support them. + * + * Note the "((Map) names).put(...)" above this else/if case. + */ if (names.containsKey(type)) { return names.get(type); } diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/annotations/AvroDefaultTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/annotations/AvroDefaultTest.java index 2078cee58..926bd4c2a 100644 --- a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/annotations/AvroDefaultTest.java +++ b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/interop/annotations/AvroDefaultTest.java @@ -15,7 +15,7 @@ static class RecordWithDefaults { @AvroDefault("1234") public Integer intField; @AvroDefault("true") - public Integer booleanField; + public Boolean booleanField; } @Test From 0295a5e3f5be0dc0094044a95479687e345a2bc4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 28 Feb 2020 18:21:37 -0800 Subject: [PATCH 2/2] Minor clean up for #195 --- .../avro/deser/AvroReaderFactory.java | 4 +--- .../avro/schema/AvroSchemaHelper.java | 21 ++++++++++++++++--- .../dataformat/avro/schema/RecordVisitor.java | 2 +- release-notes/VERSION-2.x | 2 ++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java index 6cd91be05..380b58319 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroReaderFactory.java @@ -4,7 +4,6 @@ import org.apache.avro.Schema; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.avro.deser.ScalarDecoder.*; import com.fasterxml.jackson.dataformat.avro.schema.AvroSchemaHelper; @@ -22,7 +21,6 @@ public abstract class AvroReaderFactory protected final static ScalarDecoder READER_LONG = new LongReader(); protected final static ScalarDecoder READER_NULL = new NullReader(); protected final static ScalarDecoder READER_STRING = new StringReader(); - private final static ObjectMapper DEFAULT_MAPPER = new ObjectMapper(); /** * To resolve cyclic types, need to keep track of resolved named @@ -345,7 +343,7 @@ protected AvroStructureReader createRecordReader(Schema writerSchema, Schema rea if (!defaultFields.isEmpty()) { for (Schema.Field defaultField : defaultFields) { AvroFieldReader fr = AvroFieldDefaulters.createDefaulter(defaultField.name(), - AvroSchemaHelper.parseDefaultValue(defaultField.defaultVal()) + AvroSchemaHelper.objectToJsonNode(defaultField.defaultVal()) ); if (fr == null) { throw new IllegalArgumentException("Unsupported default type: "+defaultField.schema().getType()); diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java index e34d9e8e7..e7449dbe3 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java @@ -25,6 +25,8 @@ public abstract class AvroSchemaHelper { /** * Dedicated mapper for handling default values (String <-> JsonNode <-> Object) + * + * @since 2.11 */ private static final ObjectMapper DEFAULT_VALUE_MAPPER = new ObjectMapper(); @@ -341,7 +343,9 @@ public static String getFullName(Schema schema) { return namespace + name; } StringBuilder sb = new StringBuilder(1 + namespace.length() + name.length()); - // Check if this is a nested class + // 28-Feb-2020: [dataformats-binary#195] somewhat complicated logic of trying + // to support differences between avro-lib 1.8 and 1.9... + // Check if this is a nested class String nestedClassName = sb.append(namespace).append('$').append(name).toString(); try { Class.forName(nestedClassName); @@ -356,11 +360,17 @@ public static String getFullName(Schema schema) { } } - public static JsonNode parseDefaultValue(Object defaultValue) { + /** + * @since 2.11 + */ + public static JsonNode objectToJsonNode(Object defaultValue) { return DEFAULT_VALUE_MAPPER.convertValue(defaultValue, JsonNode.class); } - public static Object convertDefaultValueToObject(JsonNode defaultJsonValue) { + /** + * @since 2.11 + */ + public static Object jsonNodeToObject(JsonNode defaultJsonValue) { return DEFAULT_VALUE_MAPPER.convertValue(defaultJsonValue, Object.class); } @@ -370,6 +380,8 @@ public static Object convertDefaultValueToObject(JsonNode defaultJsonValue) { * @param defaultValue The default value to parse, in the form of a JSON string * @return a parsed JSON representation of the default value * @throws JsonMappingException If {@code defaultValue} is not valid JSON + * + * @since 2.11 */ public static JsonNode parseDefaultValue(String defaultValue) throws JsonMappingException { if (defaultValue == null) { @@ -378,6 +390,9 @@ public static JsonNode parseDefaultValue(String defaultValue) throws JsonMapping try { return DEFAULT_VALUE_MAPPER.readTree(defaultValue); } catch (JsonProcessingException e) { + if (e instanceof JsonMappingException) { + throw (JsonMappingException) e; + } throw new JsonMappingException(null, "Failed to parse default value", e); } } diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java index 53dd828ee..b355f549d 100644 --- a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java +++ b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java @@ -186,7 +186,7 @@ protected Schema.Field schemaFieldForWriter(BeanProperty prop, boolean optional) JsonNode defaultValue = AvroSchemaHelper.parseDefaultValue(prop.getMetadata().getDefaultValue()); writerSchema = reorderUnionToMatchDefaultType(writerSchema, defaultValue); Schema.Field field = new Schema.Field(prop.getName(), writerSchema, prop.getMetadata().getDescription(), - AvroSchemaHelper.convertDefaultValueToObject(defaultValue)); + AvroSchemaHelper.jsonNodeToObject(defaultValue)); AvroMeta meta = prop.getAnnotation(AvroMeta.class); if (meta != null) { diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 79c1819f8..90cb2cc9e 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -15,6 +15,8 @@ Project: jackson-datatypes-binaryModules: #192: (ion) Allow `IonObjectMapper` with class name annotation introspector to deserialize generic subtypes (reported, fix provided by Binh T) +#195: Remove dependencies upon Jackson 1.X and Avro's JacksonUtils + (contributed by Bryan H) - `AvroGenerator` overrides `getOutputContext()` properly 2.10.2 (05-Jan-2020)