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 2010/10/23 00:17:38 UTC

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

Author: cutting
Date: Fri Oct 22 22:17:38 2010
New Revision: 1026511

URL: http://svn.apache.org/viewvc?rev=1026511&view=rev
Log:
AVRO-678. Implement ReflectData#compare().

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericData.java
    avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumReader.java
    avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumWriter.java
    avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectData.java
    avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumReader.java
    avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumWriter.java
    avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificData.java
    avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificDatumReader.java
    avro/trunk/lang/java/src/test/java/org/apache/avro/TestReflect.java

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Fri Oct 22 22:17:38 2010
@@ -10,6 +10,10 @@ Avro 1.5.0 (unreleased)
     AVRO-671. Java: Check that type and field names conform to
     specified requirements. (cutting)
 
+    AVRO-678. Java: Implement ReflectData#compare().  Incompatibly
+    moves some protected GenericDatumReader/Writer methods to
+    GenericData, potentially breaking subclasses. (cutting)
+
   BUG FIXES
 
     AVRO-675. C: Bytes and fixed setters don't update datum size.

Modified: avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericData.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericData.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericData.java Fri Oct 22 22:17:38 2010
@@ -370,6 +370,20 @@ public class GenericData {
     else throw new AvroTypeException("Can't create schema for: "+datum);
   }
 
+  /** Called by {@link GenericDatumReader#readRecord} to set a record fields
+   * value to a record instance.  The default implementation is for {@link
+   * IndexedRecord}.*/
+  public void setField(Object record, String name, int position, Object o) {
+    ((IndexedRecord)record).put(position, o);
+  }
+  
+  /** Called by {@link GenericDatumReader#readRecord} to retrieve a record
+   * field value from a reused instance.  The default implementation is for
+   * {@link IndexedRecord}.*/
+  public Object getField(Object record, String name, int position) {
+    return ((IndexedRecord)record).get(position);
+  }
+
   /** Return the index for a datum within a union.  Implemented with {@link
    * #instanceOf(Schema,Object)}.*/
   public int resolveUnion(Schema union, Object datum) {
@@ -495,15 +509,13 @@ public class GenericData {
     if (o1 == o2) return 0;
     switch (s.getType()) {
     case RECORD:
-      if (!(o1 instanceof IndexedRecord))
-        return ((Comparable)o1).compareTo(o2);
-      IndexedRecord r1 = (IndexedRecord)o1;
-      IndexedRecord r2 = (IndexedRecord)o2;
       for (Field f : s.getFields()) {
         if (f.order() == Field.Order.IGNORE)
           continue;                               // ignore this field
         int pos = f.pos();
-        int compare = compare(r1.get(pos), r2.get(pos), f.schema());
+        String name = f.name();
+        int compare =
+          compare(getField(o1, name, pos), getField(o2, name, pos), f.schema());
         if (compare != 0)                         // not equal
           return f.order() == Field.Order.DESCENDING ? -compare : compare;
       }

Modified: avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumReader.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumReader.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumReader.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumReader.java Fri Oct 22 22:17:38 2010
@@ -34,21 +34,28 @@ import org.apache.avro.util.WeakIdentity
 
 /** {@link DatumReader} for generic Java objects. */
 public class GenericDatumReader<D> implements DatumReader<D> {
+  private GenericData data;
   private Schema actual;
   private Schema expected;
 
-  public GenericDatumReader() {}
+  public GenericDatumReader() {
+    this(null, null, GenericData.get());
+  }
 
   /** Construct where the writer's and reader's schemas are the same. */
   public GenericDatumReader(Schema schema) {
-    this.actual = schema;
-    this.expected = schema;
+    this(schema, schema, GenericData.get());
   }
 
   /** Construct given writer's and reader's schema. */
   public GenericDatumReader(Schema writer, Schema reader) {
+    this(writer, reader, GenericData.get());
+  }
+
+  protected GenericDatumReader(Schema writer, Schema reader, GenericData data) {
     this.actual = writer;
     this.expected = reader;
+    this.data = data;
   }
 
   @Override
@@ -138,33 +145,12 @@ public class GenericDatumReader<D> imple
     for (Field f : in.readFieldOrder()) {
       int pos = f.pos();
       String name = f.name();
-      Object oldDatum = (old != null) ? getField(record, name, pos) : null;
-      setField(record, name, pos, read(oldDatum, f.schema(), in));
+      Object oldDatum = (old != null) ? data.getField(record, name, pos) : null;
+      data.setField(record, name, pos, read(oldDatum, f.schema(), in));
     }
 
     return record;
   }
-
-  /** Called by the default implementation of {@link #readRecord} to set a
-   * record fields value to a record instance.  The default implementation is
-   * for {@link IndexedRecord}.*/
-  protected void setField(Object record, String name, int position, Object o) {
-    ((IndexedRecord)record).put(position, o);
-  }
-  
-  /** Called by the default implementation of {@link #readRecord} to retrieve a
-   * record field value from a reused instance.  The default implementation is
-   * for {@link IndexedRecord}.*/
-  protected Object getField(Object record, String name, int position) {
-    return ((IndexedRecord)record).get(position);
-  }
-
-  /** Called by the default implementation of {@link #readRecord} to remove a
-   * record field value from a reused instance.  The default implementation is
-   * for {@link GenericRecord}.*/
-  protected void removeField(Object record, String field, int position) {
-    ((GenericRecord)record).put(position, null);
-  }
   
   /** Called to read an enum value. May be overridden for alternate enum
    * representations.  By default, returns a GenericEnumSymbol. */

Modified: avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumWriter.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumWriter.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumWriter.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/generic/GenericDatumWriter.java Fri Oct 22 22:17:38 2010
@@ -95,7 +95,7 @@ public class GenericDatumWriter<D> imple
   protected void writeRecord(Schema schema, Object datum, Encoder out)
     throws IOException {
     for (Field f : schema.getFields()) {
-      Object value = getField(datum, f.name(), f.pos());
+      Object value = data.getField(datum, f.name(), f.pos());
       try {
         write(f.schema(), value, out);
       } catch (NullPointerException e) {
@@ -104,13 +104,6 @@ public class GenericDatumWriter<D> imple
     }
   }
   
-  /** Called by the default implementation of {@link #writeRecord} to retrieve
-   * a record field value.  The default implementation is for {@link
-   * IndexedRecord}.*/
-  protected Object getField(Object record, String field, int position) {
-    return ((IndexedRecord) record).get(position);
-  }
-  
   /** Called to write an enum value.  May be overridden for alternate enum
    * representations.*/
   protected void writeEnum(Schema schema, Object datum, Encoder out)

Modified: avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectData.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectData.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectData.java Fri Oct 22 22:17:38 2010
@@ -42,6 +42,7 @@ import org.apache.avro.generic.IndexedRe
 import org.apache.avro.generic.GenericFixed;
 import org.apache.avro.specific.SpecificData;
 import org.apache.avro.specific.FixedSize;
+import org.apache.avro.io.BinaryData;
 import org.apache.avro.ipc.AvroRemoteException;
 
 import com.thoughtworks.paranamer.CachingParanamer;
@@ -74,6 +75,30 @@ public class ReflectData extends Specifi
   public static ReflectData get() { return INSTANCE; }
 
   @Override
+  public void setField(Object record, String name, int position, Object o) {
+    if (record instanceof IndexedRecord) {
+      super.setField(record, name, position, o);
+      return;
+    }
+    try {
+      getField(record.getClass(), name).set(record, o);
+    } catch (IllegalAccessException e) {
+      throw new AvroRuntimeException(e);
+    }
+  }
+
+  @Override
+  public Object getField(Object record, String name, int position) {
+    if (record instanceof IndexedRecord)
+      return super.getField(record, name, position);
+    try {
+      return getField(record.getClass(), name).get(record);
+    } catch (IllegalAccessException e) {
+      throw new AvroRuntimeException(e);
+    }
+  }
+
+  @Override
   protected boolean isRecord(Object datum) {
     if (datum == null) return false;
     return getSchema(datum.getClass()).getType() == Schema.Type.RECORD;
@@ -137,7 +162,7 @@ public class ReflectData extends Specifi
 
   /** Return the named field of the provided class.  Implementation caches
    * values, since this is used at runtime to get and set fields. */
-  protected static Field getField(Class c, String name) {
+  private static Field getField(Class c, String name) {
     Map<String,Field> fields = FIELD_CACHE.get(c);
     if (fields == null) {
       fields = new ConcurrentHashMap<String,Field>();
@@ -424,7 +449,29 @@ public class ReflectData extends Specifi
 
   @Override
   public int compare(Object o1, Object o2, Schema s) {
-    throw new UnsupportedOperationException();
+    switch (s.getType()) {
+    case ARRAY:
+      if (!o1.getClass().isArray())
+        break;
+      Schema elementType = s.getElementType();
+      int l1 = java.lang.reflect.Array.getLength(o1);
+      int l2 = java.lang.reflect.Array.getLength(o2);
+      int l = Math.min(l1, l2);
+      for (int i = 0; i < l; i++) {
+        int compare = compare(java.lang.reflect.Array.get(o1, i),
+                              java.lang.reflect.Array.get(o2, i),
+                              elementType);
+        if (compare != 0) return compare;
+      }
+      return l1 - l2;
+    case BYTES:
+      if (!o1.getClass().isArray())
+        break;
+      byte[] b1 = (byte[])o1; 
+      byte[] b2 = (byte[])o2; 
+      return BinaryData.compareBytes(b1, 0, b1.length, b2, 0, b2.length);
+    }
+    return super.compare(o1, o2, s);
   }
 
 }

Modified: avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumReader.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumReader.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumReader.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumReader.java Fri Oct 22 22:17:38 2010
@@ -26,7 +26,6 @@ import java.nio.ByteBuffer;
 
 import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Schema;
-import org.apache.avro.generic.IndexedRecord;
 import org.apache.avro.specific.SpecificDatumReader;
 import org.apache.avro.io.Decoder;
 
@@ -35,38 +34,26 @@ import org.apache.avro.io.Decoder;
  * Java reflection.
  */
 public class ReflectDatumReader<T> extends SpecificDatumReader<T> {
-  public ReflectDatumReader() {}
+  public ReflectDatumReader() {
+    this(null, null, ReflectData.get());
+  }
 
   public ReflectDatumReader(Class<T> c) {
     this(ReflectData.get().getSchema(c));
   }
 
+  /** Construct where the writer's and reader's schemas are the same. */
   public ReflectDatumReader(Schema root) {
-    super(root);
+    this(root, root, ReflectData.get());
   }
 
-  @Override
-  protected void setField(Object record, String name, int position, Object o) {
-    if (record instanceof IndexedRecord) {
-      super.setField(record, name, position, o);
-      return;
-    }
-      try {
-      ReflectData.getField(record.getClass(), name).set(record, o);
-    } catch (IllegalAccessException e) {
-      throw new AvroRuntimeException(e);
-    }
+  /** Construct given writer's and reader's schema. */
+  public ReflectDatumReader(Schema writer, Schema reader) {
+    this(writer, reader, ReflectData.get());
   }
 
-  @Override
-  protected Object getField(Object record, String name, int position) {
-    if (record instanceof IndexedRecord)
-      return super.getField(record, name, position);
-    try {
-      return ReflectData.getField(record.getClass(), name).get(record);
-    } catch (IllegalAccessException e) {
-      throw new AvroRuntimeException(e);
-    }
+  protected ReflectDatumReader(Schema writer, Schema reader, ReflectData data){
+    super(writer, reader, data);
   }
 
   @Override

Modified: avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumWriter.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumWriter.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumWriter.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/reflect/ReflectDatumWriter.java Fri Oct 22 22:17:38 2010
@@ -22,9 +22,7 @@ import java.io.IOException;
 import java.util.Iterator;
 import java.util.Collection;
 
-import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Schema;
-import org.apache.avro.generic.IndexedRecord;
 import org.apache.avro.specific.SpecificDatumWriter;
 import org.apache.avro.io.Encoder;
 
@@ -58,17 +56,6 @@ public class ReflectDatumWriter<T> exten
   }
   
   @Override
-  protected Object getField(Object record, String name, int position) {
-    if (record instanceof IndexedRecord)
-      return super.getField(record, name, position);
-    try {
-      return ReflectData.getField(record.getClass(), name).get(record);
-    } catch (IllegalAccessException e) {
-      throw new AvroRuntimeException(e);
-    }
-  }
-  
-  @Override
   @SuppressWarnings("unchecked")
   protected long getArraySize(Object array) {
     if (array instanceof Collection)

Modified: avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificData.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificData.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificData.java Fri Oct 22 22:17:38 2010
@@ -184,7 +184,7 @@ public class SpecificData extends Generi
   public int compare(Object o1, Object o2, Schema s) {
     switch (s.getType()) {
     case ENUM:
-      if (!(o1 instanceof String))                // not generic
+      if (o1 instanceof Enum)
         return ((Enum)o1).ordinal() - ((Enum)o2).ordinal();
     default:
       return super.compare(o1, o2, s);

Modified: avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificDatumReader.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificDatumReader.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificDatumReader.java (original)
+++ avro/trunk/lang/java/src/java/org/apache/avro/specific/SpecificDatumReader.java Fri Oct 22 22:17:38 2010
@@ -26,7 +26,9 @@ import org.apache.avro.generic.GenericDa
 
 /** {@link org.apache.avro.io.DatumReader DatumReader} for generated Java classes. */
 public class SpecificDatumReader<T> extends GenericDatumReader<T> {
-  public SpecificDatumReader() {}
+  public SpecificDatumReader() {
+    this(null, null, SpecificData.get());
+  }
 
   public SpecificDatumReader(Class<T> c) {
     this(SpecificData.get().getSchema(c));
@@ -34,12 +36,17 @@ public class SpecificDatumReader<T> exte
 
   /** Construct where the writer's and reader's schemas are the same. */
   public SpecificDatumReader(Schema schema) {
-    super(schema);
+    this(schema, schema, SpecificData.get());
   }
 
   /** Construct given writer's and reader's schema. */
   public SpecificDatumReader(Schema writer, Schema reader) {
-    super(writer, reader);
+    this(writer, reader, SpecificData.get());
+  }
+
+  protected SpecificDatumReader(Schema writer, Schema reader,
+                                SpecificData data) {
+    super(writer, reader, data);
   }
 
   @Override

Modified: avro/trunk/lang/java/src/test/java/org/apache/avro/TestReflect.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/src/test/java/org/apache/avro/TestReflect.java?rev=1026511&r1=1026510&r2=1026511&view=diff
==============================================================================
--- avro/trunk/lang/java/src/test/java/org/apache/avro/TestReflect.java (original)
+++ avro/trunk/lang/java/src/test/java/org/apache/avro/TestReflect.java Fri Oct 22 22:17:38 2010
@@ -523,10 +523,8 @@ public class TestReflect {
     ReflectData.get().getProtocol(Class.forName("NoPackage"));
   }
 
-  public static class Y implements Comparable {
+  public static class Y {
     int i;
-    @Override public boolean equals(Object o) { return ((Y)o).i == this.i; }
-    @Override public int compareTo(Object o) { return ((Y)o).i - this.i; }
   }
 
   @Test
@@ -546,9 +544,24 @@ public class TestReflect {
     record.put("f", y);
 
     // test that this instance can be written & re-read
-    TestSchema.checkBinary(schema, record,
-                           new ReflectDatumWriter<Object>(),
-                           new ReflectDatumReader<Object>());
+    checkBinary(schema, record);
   }
 
+  public static void checkBinary(Schema schema, Object datum)
+    throws IOException {
+    ReflectDatumWriter<Object> writer = new ReflectDatumWriter<Object>(schema);
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    writer.write(datum, new BinaryEncoder(out));
+    byte[] data = out.toByteArray();
+
+    ReflectDatumReader<Object> reader = new ReflectDatumReader<Object>(schema);
+    Object decoded =
+      reader.read(null, DecoderFactory.defaultFactory().createBinaryDecoder(
+          data, null));
+      
+    assertEquals(0, ReflectData.get().compare(datum, decoded, schema));
+  }
+
+
+
 }