You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by cu...@apache.org on 2012/09/11 01:40:26 UTC

svn commit: r1383160 - in /avro/trunk: ./ lang/java/avro/src/main/java/org/apache/avro/generic/ lang/java/avro/src/main/java/org/apache/avro/reflect/ lang/java/avro/src/test/java/org/apache/avro/ lang/java/avro/src/test/java/org/apache/avro/reflect/

Author: cutting
Date: Mon Sep 10 23:40:25 2012
New Revision: 1383160

URL: http://svn.apache.org/viewvc?rev=1383160&view=rev
Log:
AVRO-1147. Java: Permit stringable map keys in reflect.  Contributed by Alexandre Normand.

Added:
    avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
      - copied, changed from r1383147, avro/trunk/lang/java/avro/src/test/java/org/apache/avro/TestReflect.java
Removed:
    avro/trunk/lang/java/avro/src/test/java/org/apache/avro/TestReflect.java
Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1383160&r1=1383159&r2=1383160&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Mon Sep 10 23:40:25 2012
@@ -10,6 +10,9 @@ Avro 1.7.2 (unreleased)
     strings, including BigDecimal, BigInteger, URI, URL, Date and
     File.  (Alexandre Normand and cutting)
 
+    AVRO-1147. Java: Permit stringable map keys in reflect.
+    (Alexandre Normand)
+
   BUG FIXES
 
     AVRO-1128. Java: Fix SpecificRecordBase#equals() to work for

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java?rev=1383160&r1=1383159&r2=1383160&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java Mon Sep 10 23:40:25 2012
@@ -236,13 +236,21 @@ public class GenericDatumReader<D> imple
     if (l > 0) {
       do {
         for (int i = 0; i < l; i++) {
-          addToMap(map, readString(null, expected, in), read(null, eValue, in));
+          addToMap(map, readMapKey(null, expected, in), read(null, eValue, in));
         }
       } while ((l = in.mapNext()) > 0);
     }
     return map;
   }
 
+  /** Called by the default implementation of {@link #readMap} to read a
+   * key value.  The default implementation returns delegates to
+   * {@link #readString(Object, org.apache.avro.io.Decoder)}.*/
+  protected Object readMapKey(Object old, Schema expected, Decoder in)
+    throws IOException{
+    return readString(old, expected, in);
+  }
+
   /** Called by the default implementation of {@link #readMap} to add a
    * key/value pair.  The default implementation is for {@link Map}.*/
   @SuppressWarnings("unchecked")

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java?rev=1383160&r1=1383159&r2=1383160&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java Mon Sep 10 23:40:25 2012
@@ -161,7 +161,7 @@ public class GenericDatumWriter<D> imple
     out.setItemCount(size);
     for (Map.Entry<Object,Object> entry : getMapEntries(datum)) {
       out.startItem();
-      writeString(entry.getKey(), out);
+      writeString(entry.getKey().toString(), out);
       write(value, entry.getValue(), out);
     }
     out.writeMapEnd();

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java?rev=1383160&r1=1383159&r2=1383160&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java Mon Sep 10 23:40:25 2012
@@ -217,6 +217,7 @@ public class ReflectData extends Specifi
   }
 
   static final String CLASS_PROP = "java-class";
+  static final String KEY_CLASS_PROP = "java-key-class";
   static final String ELEMENT_PROP = "java-element-class";
 
   static Class getClassProp(Schema schema, String prop) {
@@ -269,11 +270,15 @@ public class ReflectData extends Specifi
       Class raw = (Class)ptype.getRawType();
       Type[] params = ptype.getActualTypeArguments();
       if (Map.class.isAssignableFrom(raw)) {                 // Map
-        Type key = params[0];
-        Type value = params[1];
-        if (!(key == String.class))
+        Schema schema = Schema.createMap(createSchema(params[1], names));
+        Class key = (Class)params[0];
+        if (key.isAnnotationPresent(Stringable.class) || // Stringable key
+            stringableClasses.contains(key)) {
+          schema.addProp(KEY_CLASS_PROP, key.getName());
+        } else if (key != String.class) {
           throw new AvroTypeException("Map key class not String: "+key);
-        return Schema.createMap(createSchema(value, names));
+        }
+        return schema;
       } else if (Collection.class.isAssignableFrom(raw)) {   // Collection
         if (params.length != 1)
           throw new AvroTypeException("No array type specified.");

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java?rev=1383160&r1=1383159&r2=1383160&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java Mon Sep 10 23:40:25 2012
@@ -100,11 +100,22 @@ public class ReflectDatumReader<T> exten
   }
 
   @Override
+  protected Object readMapKey(Object old, Schema s, Decoder in)
+    throws IOException {
+    Class c = ReflectData.getClassProp(s, ReflectData.KEY_CLASS_PROP);
+    return readString(in, c);
+  }
+
+  @Override
   @SuppressWarnings(value="unchecked")
   protected Object readString(Object old, Schema s,
                               Decoder in) throws IOException {
-    String value = (String)readString(null, in);
     Class c = ReflectData.getClassProp(s, ReflectData.CLASS_PROP);
+    return readString(in, c);
+  }
+
+  private Object readString(Decoder in, Class c) throws IOException {
+    String value = (String)readString(null, in);
     if (c != null)                                // Stringable annotated class
       try {                                       // use String-arg ctor
         return c.getConstructor(String.class).newInstance(value);

Copied: avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java (from r1383147, avro/trunk/lang/java/avro/src/test/java/org/apache/avro/TestReflect.java)
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java?p2=avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java&p1=avro/trunk/lang/java/avro/src/test/java/org/apache/avro/TestReflect.java&r1=1383147&r2=1383160&rev=1383160&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/test/java/org/apache/avro/TestReflect.java (original)
+++ avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java Mon Sep 10 23:40:25 2012
@@ -15,7 +15,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.avro;
+package org.apache.avro.reflect;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -33,21 +33,19 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Protocol;
+import org.apache.avro.Schema;
 import org.codehaus.jackson.node.NullNode;
 
 import org.apache.avro.Schema.Field;
-import org.apache.avro.TestReflect.SampleRecord.AnotherSampleRecord;
+import org.apache.avro.reflect.TestReflect.SampleRecord.AnotherSampleRecord;
 import org.apache.avro.io.DecoderFactory;
 import org.apache.avro.io.Decoder;
 import org.apache.avro.io.Encoder;
 import org.apache.avro.io.EncoderFactory;
 import org.apache.avro.generic.GenericData;
-import org.apache.avro.reflect.ReflectData;
-import org.apache.avro.reflect.ReflectDatumReader;
-import org.apache.avro.reflect.ReflectDatumWriter;
-import org.apache.avro.reflect.Stringable;
-import org.apache.avro.reflect.Nullable;
-import org.apache.avro.reflect.Union;
 
 import org.junit.Test;
 
@@ -414,13 +412,13 @@ public class TestReflect {
   public static enum E { A, B };
   @Test public void testEnum() throws Exception {
     check(E.class, "{\"type\":\"enum\",\"name\":\"E\",\"namespace\":"
-          +"\"org.apache.avro.TestReflect$\",\"symbols\":[\"A\",\"B\"]}");
+          +"\"org.apache.avro.reflect.TestReflect$\",\"symbols\":[\"A\",\"B\"]}");
   }
 
   public static class R { int a; long b; }
   @Test public void testRecord() throws Exception {
     check(R.class, "{\"type\":\"record\",\"name\":\"R\",\"namespace\":"
-          +"\"org.apache.avro.TestReflect$\",\"fields\":["
+          +"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
           +"{\"name\":\"a\",\"type\":\"int\"},"
           +"{\"name\":\"b\",\"type\":\"long\"}]}");
   }
@@ -622,7 +620,35 @@ public class TestReflect {
     checkBinary(schema, c.getConstructor(String.class).newInstance(value));
   }
 
-  public static void checkBinary(Schema schema, Object datum)
+  public static class M1 {
+    Map<Integer, String> integerKeyMap;
+    Map<java.math.BigInteger, String> bigIntegerKeyMap;
+    Map<java.math.BigDecimal, String> bigDecimalKeyMap;
+    Map<java.io.File, String> fileKeyMap;
+  }
+
+  /** Test Map with stringable key classes. */
+  @Test public void testStringableMapKeys() throws Exception {
+    M1 record = new M1();
+    record.integerKeyMap = new HashMap<Integer, String>(1);
+    record.integerKeyMap.put(10, "foo");
+
+    record.bigIntegerKeyMap = new HashMap<java.math.BigInteger, String>(1);
+    record.bigIntegerKeyMap.put(java.math.BigInteger.TEN, "bar");
+
+    record.bigDecimalKeyMap = new HashMap<java.math.BigDecimal, String>(1);
+    record.bigDecimalKeyMap.put(java.math.BigDecimal.ONE, "bigDecimal");
+
+    record.fileKeyMap = new HashMap<java.io.File, String>(1);
+    record.fileKeyMap.put(new java.io.File("foo.bar"), "file");
+
+    ReflectData data = new ReflectData().addStringable(Integer.class);
+
+    checkBinary(data, data.getSchema(M1.class), record, true);
+  }
+
+  public static void checkBinary(ReflectData reflectData, Schema schema,
+                                 Object datum, boolean equals)
     throws IOException {
     ReflectDatumWriter<Object> writer = new ReflectDatumWriter<Object>(schema);
     ByteArrayOutputStream out = new ByteArrayOutputStream();
@@ -633,8 +659,13 @@ public class TestReflect {
     Object decoded =
       reader.read(null, DecoderFactory.get().binaryDecoder(
           data, null));
-      
-    assertEquals(0, ReflectData.get().compare(datum, decoded, schema));
+
+    assertEquals(0, reflectData.compare(datum, decoded, schema, equals));
+  }
+
+  public static void checkBinary(Schema schema, Object datum)
+    throws IOException {
+    checkBinary(ReflectData.get(), schema, datum, false);
   }
 
   /** Test that the error message contains the name of the class. */