HADOOP-1601. Change GenericWritable to use ReflectionUtils for instance creation. Contributed by Enis.

git-svn-id: https://svn.apache.org/repos/asf/lucene/hadoop/trunk@570878 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -109,7 +109,11 @@ Trunk (unreleased changes)
     errors and use them to trigger speculative re-execution of tasks.
     (Arun C Murthy via cutting)
 
-    
+    HADOOP-1601.  Change GenericWritable to use ReflectionUtils for
+    instance creation, avoiding classloader issues, and to implement
+    Configurable.  (Enis Soztutar via cutting)
+
+
 Release 0.14.1 - (unreleased)
 
   BUG FIXES

src/java/org/apache/hadoop/io/GenericWritable.java

@@ -22,6 +22,10 @@ import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 
+import org.apache.hadoop.conf.Configurable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.util.ReflectionUtils;
+
 /**
  * A wrapper for Writable instances.
  * <p>
@@ -36,6 +40,12 @@ import java.io.IOException;
  * into the output file in every Key-Value pair.
  * </p>
  * 
+ * <p>
+ * Generic Writable implements {@link Configurable} interface, so that it will be 
+ * configured by the framework. The configuration is passed to the wrapped objects
+ * implementing {@link Configurable} interface <i>before deserialization</i>. 
+ * </p>
+ * 
  * how to use it: <br>
  * 1. Write your own class, such as GenericObject, which extends GenericWritable.<br> 
  * 2. Implements the abstract method <code>getTypes()</code>, defines 
@@ -63,7 +73,7 @@ import java.io.IOException;
  * 
  * @since Nov 8, 2006
  */
-public abstract class GenericWritable implements Writable {
+public abstract class GenericWritable implements Writable, Configurable {
 
   private static final byte NOT_SET = -1;
 
@@ -71,6 +81,8 @@ public abstract class GenericWritable implements Writable {
 
   private Writable instance;
 
+  private Configuration conf = null;
+  
   /**
    * Set the instance that is wrapped.
    * 
@@ -78,10 +90,11 @@ public abstract class GenericWritable implements Writable {
    */
   public void set(Writable obj) {
     instance = obj;
-    Class[] clazzes = getTypes();
+    Class<? extends Writable> instanceClazz = instance.getClass();
+    Class<? extends Writable>[] clazzes = getTypes();
     for (int i = 0; i < clazzes.length; i++) {
-      Class clazz = clazzes[i];
-      if (clazz.isInstance(instance)) {
+      Class<? extends Writable> clazz = clazzes[i];
+      if (clazz.equals(instanceClazz)) {
         type = (byte) i;
         return;
       }
@@ -106,7 +119,7 @@ public abstract class GenericWritable implements Writable {
     type = in.readByte();
     Class<? extends Writable> clazz = getTypes()[type & 0xff];
     try {
-      instance = clazz.newInstance();
+      instance = (Writable)ReflectionUtils.newInstance(clazz, conf);
     } catch (Exception e) {
       e.printStackTrace();
       throw new IOException("Cannot initialize the class: " + clazz);
@@ -128,4 +141,12 @@ public abstract class GenericWritable implements Writable {
    */
   abstract protected Class<? extends Writable>[] getTypes();
 
+  public Configuration getConf() {
+    return conf;
+  }
+
+  public void setConf(Configuration conf) {
+    this.conf = conf;
+  }
+  
 }

src/test/org/apache/hadoop/io/TestGenericWritable.java

@@ -0,0 +1,171 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF 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 org.apache.hadoop.io;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configurable;
+import org.apache.hadoop.conf.Configuration;
+
+import junit.framework.TestCase;
+
+/**
+ * TestCase for {@link GenericWritable} class.
+ * @see TestWritable#testWritable(Writable)
+ */
+public class TestGenericWritable extends TestCase {
+
+  private Configuration conf;
+  private static final String CONF_TEST_KEY = "test.generic.writable";
+  private static final String CONF_TEST_VALUE = "dummy";
+
+  protected void setUp() throws Exception {
+    super.setUp();
+    conf = new Configuration();
+    //set the configuration parameter
+    conf.set(CONF_TEST_KEY, CONF_TEST_VALUE);
+  }
+
+  /** Dummy class for testing {@link GenericWritable} */
+  public static class Foo implements Writable {
+    private String foo = "foo";
+    public void readFields(DataInput in) throws IOException {
+      foo = Text.readString(in);
+    }
+    public void write(DataOutput out) throws IOException {
+      Text.writeString(out, foo);
+    }
+    public boolean equals(Object obj) {
+      if (!(obj instanceof Foo))
+        return false;
+      return this.foo.equals(((Foo)obj).foo);
+    }
+  }
+  /** Dummy class for testing {@link GenericWritable} */
+  public static class Bar implements Writable, Configurable {
+    private int bar = 42; //The Answer to The Ultimate Question Of Life, the Universe and Everything
+    private Configuration conf = null;
+    public void readFields(DataInput in) throws IOException {
+      bar = in.readInt();
+    }
+    public void write(DataOutput out) throws IOException {
+      out.writeInt(bar);
+    }
+    public Configuration getConf() {
+      return conf;
+    }
+    public void setConf(Configuration conf) {
+      this.conf = conf;
+    }
+    public boolean equals(Object obj) {
+      if (!(obj instanceof Bar))
+        return false;
+      return this.bar == ((Bar)obj).bar;
+    }
+  }
+
+  /** Dummy class for testing {@link GenericWritable} */
+  public static class Baz extends Bar {
+    public void readFields(DataInput in) throws IOException {
+      super.readFields(in);
+      //needs a configuration parameter
+      assertEquals("Configuration is not set for the wrapped object", 
+          CONF_TEST_VALUE, getConf().get(CONF_TEST_KEY)); 
+    }
+    public void write(DataOutput out) throws IOException {
+      super.write(out);
+    }
+  }
+
+  /** Dummy class for testing {@link GenericWritable} */ 
+  public static class FooGenericWritable extends GenericWritable {
+    @SuppressWarnings("unchecked")
+    protected Class<? extends Writable>[] getTypes() {
+      return new Class[] {Foo.class, Bar.class, Baz.class};
+    }
+    public boolean equals(Object obj) {
+      if(! (obj instanceof FooGenericWritable))
+        return false;
+      return get().equals(((FooGenericWritable)obj).get());
+    }
+  }
+
+  public void testFooWritable() throws Exception {
+    System.out.println("Testing Writable wrapped in GenericWritable");
+    FooGenericWritable generic = new FooGenericWritable();
+    generic.setConf(conf);
+    Foo foo = new Foo();
+    generic.set(foo);
+    TestWritable.testWritable(generic);
+  }
+
+  public void testBarWritable() throws Exception {
+    System.out.println("Testing Writable, Configurable wrapped in GenericWritable");
+    FooGenericWritable generic = new FooGenericWritable();
+    generic.setConf(conf);
+    Bar bar = new Bar();
+    bar.setConf(conf);
+    generic.set(bar);
+
+    //test writing generic writable
+    FooGenericWritable after 
+    = (FooGenericWritable)TestWritable.testWritable(generic, conf);
+
+    //test configuration
+    System.out.println("Testing if Configuration is passed to wrapped classes");
+    assertTrue(after.get() instanceof Configurable);
+    assertNotNull(((Configurable)after.get()).getConf());
+  }
+
+  public void testBazWritable() throws Exception {
+    System.out.println("Testing for GenericWritable to find class names");
+    FooGenericWritable generic = new FooGenericWritable();
+    generic.setConf(conf);
+    Baz baz = new Baz();
+    generic.set(baz);
+    TestWritable.testWritable(generic, conf);
+  }
+
+  public void testSet() throws Exception {
+    Foo foo = new Foo();
+    FooGenericWritable generic = new FooGenericWritable();
+    //exception should not occur
+    generic.set(foo);
+
+    try {
+      //exception should occur, since IntWritable is not registered
+      generic = new FooGenericWritable();
+      generic.set(new IntWritable(1));
+      fail("Generic writable should have thrown an exception for a Writable not registered");
+    }catch (RuntimeException e) {
+      //ignore
+    }
+
+  }
+
+  public void testGet() throws Exception {
+    Foo foo = new Foo();
+    FooGenericWritable generic = new FooGenericWritable();
+    generic.set(foo);
+    assertEquals(foo, generic.get());
+  }
+
+}

src/test/org/apache/hadoop/io/TestWritable.java

@@ -20,6 +20,11 @@ package org.apache.hadoop.io;
 
 import java.io.*;
 import java.util.Random;
+
+import org.apache.hadoop.conf.Configurable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.util.ReflectionUtils;
+
 import junit.framework.TestCase;
 
 /** Unit tests for Writable. */
@@ -61,17 +66,26 @@ public class TestWritable extends TestCase {
   }
 
   /** Utility method for testing writables. */
-  public static void testWritable(Writable before) throws Exception {
+  public static Writable testWritable(Writable before) 
+  	throws Exception {
+  	return testWritable(before, null);
+  }
+  
+  /** Utility method for testing writables. */
+  public static Writable testWritable(Writable before
+  		, Configuration conf) throws Exception {
     DataOutputBuffer dob = new DataOutputBuffer();
     before.write(dob);
 
     DataInputBuffer dib = new DataInputBuffer();
     dib.reset(dob.getData(), dob.getLength());
     
-    Writable after = (Writable)before.getClass().newInstance();
+    Writable after = (Writable)ReflectionUtils.newInstance(
+    		before.getClass(), conf);
     after.readFields(dib);
 
     assertEquals(before, after);
+    return after;
   }
 	
 }