From 6c04cb3f3daec9e87e49d440faa35a248767fb3b Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Sun, 4 Sep 2016 14:54:40 -0700 Subject: [PATCH 1/3] AVRO-1908: Fix TestSpecificCompiler reference to private method. AVRO-1884 changed makePath to a private method from a package-private static method. This broke the test that references the method in IPC. The fix is to make the instance method package-private and update the test to use an instance of SpecificCompiler. --- .../org/apache/avro/compiler/specific/SpecificCompiler.java | 2 +- .../apache/avro/compiler/specific/TestSpecificCompiler.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java index d1b594f79f5..5b6b3bb0ebf 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java @@ -406,7 +406,7 @@ OutputFile compileInterface(Protocol protocol) { return outputFile; } - private String makePath(String name, String space) { + String makePath(String name, String space) { if (space == null || space.isEmpty()) { return name + suffix; } else { diff --git a/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java b/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java index 7710be8ac21..28aa9981a34 100644 --- a/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java +++ b/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java @@ -79,8 +79,9 @@ public void testEsc() { @Test public void testMakePath() { - assertEquals("foo/bar/Baz.java".replace("/", File.separator), SpecificCompiler.makePath("Baz", "foo.bar")); - assertEquals("baz.java", SpecificCompiler.makePath("baz", "")); + SpecificCompiler compiler = new SpecificCompiler(); + assertEquals("foo/bar/Baz.java".replace("/", File.separator), compiler.makePath("Baz", "foo.bar")); + assertEquals("baz.java", compiler.makePath("baz", "")); } @Test From 31be7fa9d4589e7553d274d089aa3bf97c0297b3 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Mon, 5 Sep 2016 12:15:09 -0700 Subject: [PATCH 2/3] AVRO-1891: Add conversion getters to datum readers. This adds getConversionFor and getConversionByClass to GenericDatumReader so that SpecificDatumReader can override them to resolve conversions using the set of conversions used when a specific class was compiled, determined by the CONVERSIONS static variable or by adding the conversions for each field returned by getConversion(String). --- .../avro/generic/GenericDatumReader.java | 15 +++- .../avro/reflect/ReflectDatumReader.java | 11 +-- .../apache/avro/specific/SpecificData.java | 77 +++++++++++++++++++ .../avro/specific/SpecificDatumReader.java | 53 +++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java index 7cde13ffefa..498df22cfe7 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java @@ -153,7 +153,7 @@ protected Object read(Object old, Schema expected, Object datum = readWithoutConversion(old, expected, in); LogicalType logicalType = expected.getLogicalType(); if (logicalType != null) { - Conversion conversion = getData().getConversionFor(logicalType); + Conversion conversion = getConversionFor(logicalType); if (conversion != null) { return convert(datum, expected, logicalType, conversion); } @@ -161,6 +161,15 @@ protected Object read(Object old, Schema expected, return datum; } + protected Conversion getConversionFor(LogicalType logicalType) { + return getData().getConversionFor(logicalType); + } + + protected Conversion getConversionByClass(Class type, + LogicalType logicalType) { + return getData().getConversionByClass(type, logicalType); + } + protected Object readWithConversion(Object old, Schema expected, LogicalType logicalType, Conversion conversion, @@ -253,7 +262,7 @@ protected Object readArray(Object old, Schema expected, long base = 0; if (l > 0) { LogicalType logicalType = expectedType.getLogicalType(); - Conversion conversion = getData().getConversionFor(logicalType); + Conversion conversion = getConversionFor(logicalType); Object array = newArray(old, (int) l, expected); do { if (logicalType != null && conversion != null) { @@ -299,7 +308,7 @@ protected Object readMap(Object old, Schema expected, Schema eValue = expected.getValueType(); long l = in.readMapStart(); LogicalType logicalType = eValue.getLogicalType(); - Conversion conversion = getData().getConversionFor(logicalType); + Conversion conversion = getConversionFor(logicalType); Object map = newMap(old, (int) l); if (l > 0) { do { diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java index 3d5a3010235..671e9c3763f 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java @@ -80,8 +80,8 @@ protected Object newArray(Object old, int size, Schema schema) { if (elementClass == null) { // see if the element class will be converted and use that class // logical types cannot conflict with java-element-class - Conversion elementConversion = getData() - .getConversionFor(schema.getElementType().getLogicalType()); + Conversion elementConversion = getConversionFor( + schema.getElementType().getLogicalType()); if (elementConversion != null) { elementClass = elementConversion.getConvertedType(); } @@ -176,7 +176,8 @@ private Object readPrimitiveArray(Object array, Class c, long l, ResolvingDec private Object readObjectArray(Object[] array, Schema expectedType, long l, ResolvingDecoder in) throws IOException { LogicalType logicalType = expectedType.getLogicalType(); - Conversion conversion = getData().getConversionFor(logicalType); + Conversion conversion = getConversionByClass( + array.getClass().getComponentType(), logicalType); int index = 0; if (logicalType != null && conversion != null) { do { @@ -204,7 +205,7 @@ private Object readObjectArray(Object[] array, Schema expectedType, long l, private Object readCollection(Collection c, Schema expectedType, long l, ResolvingDecoder in) throws IOException { LogicalType logicalType = expectedType.getLogicalType(); - Conversion conversion = getData().getConversionFor(logicalType); + Conversion conversion = getConversionFor(logicalType); if (logicalType != null && conversion != null) { do { for (int i = 0; i < l; i++) { @@ -285,7 +286,7 @@ protected void readField(Object record, Field f, Object oldDatum, } LogicalType logicalType = f.schema().getLogicalType(); if (logicalType != null) { - Conversion conversion = getData().getConversionByClass( + Conversion conversion = getConversionByClass( accessor.getField().getType(), logicalType); if (conversion != null) { try { diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java index be6bde8d2b0..fd6c9e983e7 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java @@ -18,11 +18,13 @@ package org.apache.avro.specific; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Collection; import java.util.List; import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.LinkedHashMap; import java.nio.ByteBuffer; @@ -30,10 +32,13 @@ import java.lang.reflect.ParameterizedType; import java.io.ObjectInput; import java.io.ObjectOutput; +import java.util.concurrent.ExecutionException; +import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import org.apache.avro.Conversion; import org.apache.avro.Schema; import org.apache.avro.Protocol; import org.apache.avro.AvroRuntimeException; @@ -386,4 +391,76 @@ public static BinaryEncoder getEncoder(ObjectOutput out) { .directBinaryEncoder(new ExternalizableOutput(out), null); } + /** + * Returns the conversions used to compile a specific record. + * + * @return a Map from logical type name to Conversion + */ + Map> getConversionMap(final Schema schema) { + try { + // This uses get(Schema, Callable) so that the buildConversionMap method + // can be an instance method of this SpecificData instance. Otherwise, it + // would not use the correct ClassLoader to load the specific class for + // the schema. + return conversionMapCache.get(schema, + new Callable>>() { + @Override + public Map> call() { + return buildConversionMap(schema); + } + }); + } catch (ExecutionException e) { + throw new AvroRuntimeException( + "Could not get conversions needed for schema: " + schema); + } + } + + private static Cache>> conversionMapCache = + CacheBuilder.>>newBuilder() + .weakKeys().build(); + + @SuppressWarnings("unchecked") + private Map> buildConversionMap(Schema schema) { + Class specificClass = getClass(schema); + + Map> conversionMap = null; + if (specificClass != null && + SpecificRecordBase.class.isAssignableFrom(specificClass)) { + try { + Collection> conversions = (Collection>) + specificClass.getDeclaredField("CONVERSIONS").get(null); + if (conversions != null) { + conversionMap = new HashMap>(); + for (Conversion conversion : conversions) { + conversionMap.put(conversion.getLogicalTypeName(), conversion); + } + } + } catch (IllegalAccessException e) { + throw new AvroRuntimeException( + "Cannot access conversion map for specific class: " + + specificClass.getName()); + } catch (NoSuchFieldException e) { + // the conversion map will be built from getConversion results below + } + + if (conversionMap == null) { + SpecificRecordBase emptyRecord = + (SpecificRecordBase) newRecord(null, schema); + // attempt to construct the conversions map from the record itself + conversionMap = new HashMap>(); + for (Schema.Field field : emptyRecord.getSchema().getFields()) { + Conversion conversion = emptyRecord.getConversion(field.pos()); + if (conversion != null) { + conversionMap.put(conversion.getLogicalTypeName(), conversion); + } + } + } + } + + if (conversionMap == null) { + conversionMap = new HashMap>(); + } + + return conversionMap; + } } diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index 774ca094451..274e797aeeb 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -18,12 +18,15 @@ package org.apache.avro.specific; import org.apache.avro.Conversion; +import org.apache.avro.LogicalType; import org.apache.avro.Schema; import org.apache.avro.AvroRuntimeException; import org.apache.avro.generic.GenericDatumReader; import org.apache.avro.io.ResolvingDecoder; import org.apache.avro.util.ClassUtils; import java.io.IOException; +import java.util.LinkedList; +import java.util.Map; /** {@link org.apache.avro.io.DatumReader DatumReader} for generated Java classes. */ public class SpecificDatumReader extends GenericDatumReader { @@ -62,6 +65,42 @@ public SpecificDatumReader(SpecificData data) { /** Return the contained {@link SpecificData}. */ public SpecificData getSpecificData() { return (SpecificData)getData(); } + private final ThreadLocal>>> conversions + = new ThreadLocal>>>() { + @Override + protected LinkedList>> initialValue() { + return new LinkedList>>(); + } + }; + + protected Conversion getConversionFor(LogicalType logicalType) { + if (logicalType == null) { + return null; + } + Conversion conversion = conversions.get().peekLast() + .get(logicalType.getName()); + if (conversion == null) { + conversion = super.getConversionFor(logicalType); + } + return conversion; + } + + protected Conversion getConversionByClass(Class type, + LogicalType logicalType) { + if (logicalType == null) { + return null; + } + // Check for logical types conversions used by the specific compiler. No + // need to use the class because the compiler always produces code with the + // same class if it used a conversion. + Conversion conversion = conversions.get().peekLast() + .get(logicalType.getName()); + if (conversion == null) { + conversion = super.getConversionByClass(type, logicalType); + } + return conversion; + } + @Override public void setSchema(Schema actual) { // if expected is unset and actual is a specific record, @@ -122,5 +161,19 @@ protected void readField(Object r, Schema.Field f, Object oldDatum, super.readField(r, f, oldDatum, in, state); } } + + @Override + protected Object readRecord(Object old, final Schema expected, + ResolvingDecoder in) + throws IOException { + // Update the current set of conversions for the record class + Map> conversionMap = + getSpecificData().getConversionMap(expected); + conversions.get().addLast(conversionMap); + Object record = super.readRecord(old, expected, in); + conversions.get().removeLast(); + return record; + } + } From cf5bff3b6f7d312a65c28dbb053e6dbb6ab61c58 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Mon, 5 Sep 2016 12:44:13 -0700 Subject: [PATCH 3/3] AVRO-1891: Java: Add converions to compiled specific records. This adds a CONVERSIONS static field with the converisons that the specific compiler used to produce a record. Together with the last commit, this will enable new specific classes to use nested logical types. --- .../avro/specific/TestRecordWithLogicalTypes.java | 12 ++++++++++++ .../avro/compiler/specific/SpecificCompiler.java | 7 +++++++ .../specific/templates/java/classic/record.vm | 12 ++++++++++++ 3 files changed, 31 insertions(+) diff --git a/lang/java/avro/src/test/java/org/apache/avro/specific/TestRecordWithLogicalTypes.java b/lang/java/avro/src/test/java/org/apache/avro/specific/TestRecordWithLogicalTypes.java index da3f878bd67..654d16c82b1 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/specific/TestRecordWithLogicalTypes.java +++ b/lang/java/avro/src/test/java/org/apache/avro/specific/TestRecordWithLogicalTypes.java @@ -5,10 +5,13 @@ */ package org.apache.avro.specific; +import org.apache.avro.Conversion; import org.apache.avro.message.BinaryMessageDecoder; import org.apache.avro.message.BinaryMessageEncoder; import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.Collection; @SuppressWarnings("all") @org.apache.avro.specific.AvroGenerated @@ -268,6 +271,15 @@ public void setTs(org.joda.time.DateTime value) { protected static final org.apache.avro.data.TimeConversions.TimeConversion TIME_CONVERSION = new org.apache.avro.data.TimeConversions.TimeConversion(); protected static final org.apache.avro.data.TimeConversions.TimestampConversion TIMESTAMP_CONVERSION = new org.apache.avro.data.TimeConversions.TimestampConversion(); protected static final org.apache.avro.Conversions.DecimalConversion DECIMAL_CONVERSION = new org.apache.avro.Conversions.DecimalConversion(); + + public static final Collection> CONVERSIONS = new ArrayList>(); + static { + CONVERSIONS.add(DATE_CONVERSION); + CONVERSIONS.add(TIME_CONVERSION); + CONVERSIONS.add(TIMESTAMP_CONVERSION); + CONVERSIONS.add(DECIMAL_CONVERSION); + } + private final org.apache.avro.Conversion[] conversions = new org.apache.avro.Conversion[] { null, diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java index 5b6b3bb0ebf..ef1368b8b72 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java @@ -225,6 +225,13 @@ public void setEnableDecimalLogicalType(boolean enableDecimalLogicalType) { this.enableDecimalLogicalType = enableDecimalLogicalType; } + /** + * @return true if decimal logical type conversion is enabled + */ + public boolean isDecimalEnabled() { + return enableDecimalLogicalType; + } + private static String logChuteName = null; private void initializeVelocity() { diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm index 1170d79f040..2a697ef9c26 100644 --- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm +++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm @@ -136,7 +136,19 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError()) extends or protected static final org.apache.avro.data.TimeConversions.DateConversion DATE_CONVERSION = new org.apache.avro.data.TimeConversions.DateConversion(); protected static final org.apache.avro.data.TimeConversions.TimeConversion TIME_CONVERSION = new org.apache.avro.data.TimeConversions.TimeConversion(); protected static final org.apache.avro.data.TimeConversions.TimestampConversion TIMESTAMP_CONVERSION = new org.apache.avro.data.TimeConversions.TimestampConversion(); +#if ($this.isDecimalEnabled()) protected static final org.apache.avro.Conversions.DecimalConversion DECIMAL_CONVERSION = new org.apache.avro.Conversions.DecimalConversion(); +#end + + public static final Collection> CONVERSIONS = new ArrayList>(); + static { + CONVERSIONS.add(DATE_CONVERSION); + CONVERSIONS.add(TIME_CONVERSION); + CONVERSIONS.add(TIMESTAMP_CONVERSION); +#if ($this.isDecimalEnabled()) + CONVERSIONS.add(DECIMAL_CONVERSION); +#end + } private static final org.apache.avro.Conversion[] conversions = new org.apache.avro.Conversion[] {