Repository: crunch Updated Branches: refs/heads/master afcfb66e8 -> 63a2e29b8
CRUNCH-442 Auto-configure Avro specific classloader Automatically configure the class loader to use when using an Avro specific or reflection PType. This prevents ClassCastException when locally materializing specific or reflect records when Avro is in the lib directory of Hadoop. Project: http://git-wip-us.apache.org/repos/asf/crunch/repo Commit: http://git-wip-us.apache.org/repos/asf/crunch/commit/63a2e29b Tree: http://git-wip-us.apache.org/repos/asf/crunch/tree/63a2e29b Diff: http://git-wip-us.apache.org/repos/asf/crunch/diff/63a2e29b Branch: refs/heads/master Commit: 63a2e29b82d112d1fff6804378db2457052819b4 Parents: afcfb66 Author: Gabriel Reid <[email protected]> Authored: Wed Jul 9 17:49:47 2014 +0200 Committer: Gabriel Reid <[email protected]> Committed: Thu Jul 10 11:40:27 2014 +0200 ---------------------------------------------------------------------- .../org/apache/crunch/types/avro/AvroMode.java | 39 +++++++++++++++++++- .../org/apache/crunch/types/avro/Avros.java | 2 + .../apache/crunch/types/avro/AvroModeTest.java | 37 ++++++++++++++++--- .../org/apache/crunch/types/avro/AvrosTest.java | 28 ++++++++++++++ 4 files changed, 99 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/crunch/blob/63a2e29b/crunch-core/src/main/java/org/apache/crunch/types/avro/AvroMode.java ---------------------------------------------------------------------- diff --git a/crunch-core/src/main/java/org/apache/crunch/types/avro/AvroMode.java b/crunch-core/src/main/java/org/apache/crunch/types/avro/AvroMode.java index 3d7fbfa..9653f25 100644 --- a/crunch-core/src/main/java/org/apache/crunch/types/avro/AvroMode.java +++ b/crunch-core/src/main/java/org/apache/crunch/types/avro/AvroMode.java @@ -45,7 +45,7 @@ public class AvroMode implements ReaderWriterFactory { * Internal enum which represents the various Avro data types. */ public static enum ModeType { - SPECIFIC, REFLECT, GENERIC; + SPECIFIC, REFLECT, GENERIC } /** @@ -117,11 +117,42 @@ public class AvroMode implements ReaderWriterFactory { private static ClassLoader specificLoader = null; + /** + * Set the {@code ClassLoader} that will be used for loading Avro {@code org.apache.avro.specific.SpecificRecord} + * and reflection implementation classes. It is typically not necessary to call this method -- it should only be used + * if a specific class loader is needed in order to load the specific datum classes. + * + * @param loader the {@code ClassLoader} to be used for loading specific datum classes + */ public static void setSpecificClassLoader(ClassLoader loader) { specificLoader = loader; } /** + * Get the configured {@code ClassLoader} to be used for loading Avro {@code org.apache.specific.SpecificRecord} + * and reflection implementation classes. The return value may be null. + * + * @return the configured {@code ClassLoader} for loading specific or reflection datum classes, may be null + */ + public static ClassLoader getSpecificClassLoader() { + return specificLoader; + } + + /** + * Internal method for setting the specific class loader if none is already set. If no specific class loader is set, + * the given class loader will be set as the specific class loader. If a specific class loader is already set, this + * will be a no-op. + * + * @param loader the {@code ClassLoader} to be registered as the specific class loader if no specific class loader + * is already set + */ + static void registerSpecificClassLoaderInternal(ClassLoader loader) { + if (specificLoader == null) { + setSpecificClassLoader(loader); + } + } + + /** * the factory methods in this class may be overridden in ReaderWriterFactory */ private final ReaderWriterFactory factory; @@ -178,7 +209,11 @@ public class AvroMode implements ReaderWriterFactory { switch (this.modeType) { case REFLECT: - return new ReflectDatumReader<T>(schema); + if (specificLoader != null) { + return new ReflectDatumReader<T>(schema, schema, new ReflectData(specificLoader)); + } else { + return new ReflectDatumReader<T>(schema); + } case SPECIFIC: if (specificLoader != null) { return new SpecificDatumReader<T>( http://git-wip-us.apache.org/repos/asf/crunch/blob/63a2e29b/crunch-core/src/main/java/org/apache/crunch/types/avro/Avros.java ---------------------------------------------------------------------- diff --git a/crunch-core/src/main/java/org/apache/crunch/types/avro/Avros.java b/crunch-core/src/main/java/org/apache/crunch/types/avro/Avros.java index 6e11f1b..f950145 100644 --- a/crunch-core/src/main/java/org/apache/crunch/types/avro/Avros.java +++ b/crunch-core/src/main/java/org/apache/crunch/types/avro/Avros.java @@ -264,6 +264,7 @@ public class Avros { } public static final <T extends SpecificRecord> AvroType<T> specifics(Class<T> clazz) { + AvroMode.registerSpecificClassLoaderInternal(clazz.getClassLoader()); T t = ReflectionUtils.newInstance(clazz, null); Schema schema = t.getSchema(); return new AvroType<T>(clazz, schema, new AvroDeepCopier.AvroSpecificDeepCopier<T>(schema)); @@ -275,6 +276,7 @@ public class Avros { } public static final <T> AvroType<T> reflects(Class<T> clazz, Schema schema) { + AvroMode.registerSpecificClassLoaderInternal(clazz.getClassLoader()); return new AvroType<T>(clazz, schema, new AvroDeepCopier.AvroReflectDeepCopier<T>(clazz, schema)); } http://git-wip-us.apache.org/repos/asf/crunch/blob/63a2e29b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvroModeTest.java ---------------------------------------------------------------------- diff --git a/crunch-core/src/test/java/org/apache/crunch/types/avro/AvroModeTest.java b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvroModeTest.java index d1fdbb1..fe36520 100644 --- a/crunch-core/src/test/java/org/apache/crunch/types/avro/AvroModeTest.java +++ b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvroModeTest.java @@ -17,6 +17,14 @@ */ package org.apache.crunch.types.avro; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; + import org.apache.avro.Schema; import org.apache.avro.generic.GenericData; import org.apache.avro.io.DatumReader; @@ -27,11 +35,6 @@ import org.apache.crunch.io.FormatBundle; import org.apache.hadoop.conf.Configuration; import org.junit.Test; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.not; -import static org.junit.Assert.assertThat; - public class AvroModeTest { @Test @@ -149,6 +152,30 @@ public class AvroModeTest { assertThat(returnedMode.getFactory(), is(instanceOf(FakeReaderWriterFactory.class))); } + @Test + public void testRegisterClassLoader() { + // First make sure things are in the default situation + AvroMode.setSpecificClassLoader(null); + + ClassLoader classLoaderA = mock(ClassLoader.class); + ClassLoader classLoaderB = mock(ClassLoader.class); + + // Basic sanity check to ensure that the class loader was really nulled out + assertNull(AvroMode.getSpecificClassLoader()); + + // Do an internal registration of a class loader. Because there is currently no internal class loader set, + // this should set the internal specific class loader + AvroMode.registerSpecificClassLoaderInternal(classLoaderA); + + assertEquals(classLoaderA, AvroMode.getSpecificClassLoader()); + + // Now we do an internal register of another class loader. Because there already is an internal specific class + // loader set, this should have no impact (as opposed to calling setSpecificClassLoader) + AvroMode.registerSpecificClassLoaderInternal(classLoaderB); + + assertEquals(classLoaderA, AvroMode.getSpecificClassLoader()); + } + private static class FakeReaderWriterFactory implements ReaderWriterFactory{ @Override http://git-wip-us.apache.org/repos/asf/crunch/blob/63a2e29b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvrosTest.java ---------------------------------------------------------------------- diff --git a/crunch-core/src/test/java/org/apache/crunch/types/avro/AvrosTest.java b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvrosTest.java index bbfcee5..46c295e 100644 --- a/crunch-core/src/test/java/org/apache/crunch/types/avro/AvrosTest.java +++ b/crunch-core/src/test/java/org/apache/crunch/types/avro/AvrosTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.nio.ByteBuffer; @@ -321,4 +322,31 @@ public class AvrosTest { assertNotSame(outputMappedValueA, outputMappedValueB); } + @Test + public void testSpecific_AutoRegisterClassLoader() { + AvroMode.setSpecificClassLoader(null); + + // Initial sanity check + assertNull(AvroMode.getSpecificClassLoader()); + + // Calling Avros.specifics should automatically register a class loader for Avro specifics, + // unless there is already a specifics class loader set + Avros.specifics(Person.class); + + assertEquals(Person.class.getClassLoader(), AvroMode.getSpecificClassLoader()); + } + + @Test + public void testReflect_AutoRegisterClassLoader() { + AvroMode.setSpecificClassLoader(null); + + // Initial sanity check + assertNull(AvroMode.getSpecificClassLoader()); + + // Calling Avros.specifics should automatically register a class loader for Avro specifics, + // unless there is already a specifics class loader set + Avros.reflects(ReflectedPerson.class); + + assertEquals(ReflectedPerson.class.getClassLoader(), AvroMode.getSpecificClassLoader()); + } }
