avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cutt...@apache.org
Subject svn commit: r1190627 - in /avro/trunk: ./ lang/java/thrift/src/main/java/org/apache/avro/thrift/ lang/java/thrift/src/test/java/org/apache/avro/thrift/ lang/java/thrift/src/test/java/org/apache/avro/thrift/test/ lang/java/thrift/src/test/thrift/
Date Fri, 28 Oct 2011 22:20:08 GMT
Author: cutting
Date: Fri Oct 28 22:20:07 2011
New Revision: 1190627

URL: http://svn.apache.org/viewvc?rev=1190627&view=rev
Log:
AVRO-948. Java: Fix to more correctly handle Thrift optional and nullable fields.

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java
    avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java
    avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java
    avro/trunk/lang/java/thrift/src/test/thrift/test.thrift

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1190627&r1=1190626&r2=1190627&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Fri Oct 28 22:20:07 2011
@@ -228,6 +228,9 @@ Avro 1.6.0 (27 October 2011)
 
     AVRO-945. C# port does not build under Ubuntu 11.10. (thiru)
 
+    AVRO-948. Java: Fix to more correctly handle Thrift optional and
+    nullable fields.  (cutting)
+
 Avro 1.5.4 (12 September 2011)
 
   IMPROVEMENTS

Modified: avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java?rev=1190627&r1=1190626&r2=1190627&view=diff
==============================================================================
--- avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java (original)
+++ avro/trunk/lang/java/thrift/src/main/java/org/apache/avro/thrift/ThriftData.java Fri Oct
28 22:20:07 2011
@@ -18,9 +18,11 @@
 package org.apache.avro.thrift;
 
 import java.util.List;
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.nio.ByteBuffer;
 
 import org.apache.avro.Schema;
 import org.apache.avro.AvroRuntimeException;
@@ -29,7 +31,9 @@ import org.apache.avro.generic.GenericDa
 import org.apache.avro.specific.SpecificData;
 
 import org.apache.thrift.TBase;
+import org.apache.thrift.TEnum;
 import org.apache.thrift.TFieldIdEnum;
+import org.apache.thrift.TFieldRequirementType;
 import org.apache.thrift.TUnion;
 import org.apache.thrift.protocol.TType;
 import org.apache.thrift.meta_data.FieldMetaData;
@@ -69,7 +73,11 @@ public class ThriftData extends GenericD
 
   @Override
   protected Object getField(Object record, String name, int pos, Object state) {
-    return ((TBase)record).getFieldValue(((TFieldIdEnum[])state)[pos]);
+    TFieldIdEnum f = ((TFieldIdEnum[])state)[pos];
+    TBase struct = (TBase)record;
+    if (struct.isSet(f))
+      return struct.getFieldValue(f);
+    return null;
   }
 
   private final Map<Schema,TFieldIdEnum[]> fieldCache =
@@ -94,6 +102,25 @@ public class ThriftData extends GenericD
   }
 
   @Override
+  protected boolean isEnum(Object datum) {
+    return datum instanceof TEnum;
+  }
+
+  @Override
+  protected Schema getEnumSchema(Object datum) {
+    return getSchema(datum.getClass());
+  }
+
+  @Override
+  // setFieldValue takes ByteBuffer but getFieldValue returns byte[]
+  protected boolean isBytes(Object datum) {
+    if (datum instanceof ByteBuffer) return true;
+    if (datum == null) return false;
+    Class c = datum.getClass();
+    return c.isArray() && c.getComponentType() == Byte.TYPE;
+  }
+
+  @Override
   public Object newRecord(Object old, Schema schema) {
     try {
       Class c = Class.forName(SpecificData.getClassName(schema));
@@ -121,14 +148,27 @@ public class ThriftData extends GenericD
 
     if (schema == null) {                         // cache miss
       try {
-        schema = Schema.createRecord(c.getName(), null, null,
-                                     Throwable.class.isAssignableFrom(c));
-        List<Field> fields = new ArrayList<Field>();
-        for (FieldMetaData f : FieldMetaData.getStructMetaDataMap(c).values()) {
-          Schema s = getSchema(f.valueMetaData);
-          fields.add(new Field(f.fieldName, s, null, null));
+        if (TEnum.class.isAssignableFrom(c)) {    // enum
+          List<String> symbols = new ArrayList<String>();
+          for (Enum e : ((Class<? extends Enum>)c).getEnumConstants())
+            symbols.add(e.name());
+          schema = Schema.createEnum(c.getName(), null, null, symbols);
+        } else if (TBase.class.isAssignableFrom(c)) { // struct
+          schema = Schema.createRecord(c.getName(), null, null,
+                                       Throwable.class.isAssignableFrom(c));
+          List<Field> fields = new ArrayList<Field>();
+          for (FieldMetaData f :
+                 FieldMetaData.getStructMetaDataMap(c).values()) {
+            Schema s = getSchema(f.valueMetaData);
+            if (f.requirementType == TFieldRequirementType.OPTIONAL
+                && (s.getType() != Schema.Type.UNION))
+              s = nullable(s);
+            fields.add(new Field(f.fieldName, s, null, null));
+          }
+          schema.setFields(fields);
+        } else {
+          throw new RuntimeException("Not a Thrift-generated class: "+c);
         }
-        schema.setFields(fields);
       } catch (Exception e) {
         throw new RuntimeException(e);
       }
@@ -140,7 +180,6 @@ public class ThriftData extends GenericD
   private static final Schema NULL = Schema.create(Schema.Type.NULL);
 
   private Schema getSchema(FieldValueMetaData f) {
-    Schema result;
     switch (f.type) {
     case TType.BOOL:
       return Schema.create(Schema.Type.BOOLEAN);
@@ -160,41 +199,41 @@ public class ThriftData extends GenericD
       return Schema.create(Schema.Type.DOUBLE);
     case TType.ENUM:
       EnumMetaData enumMeta = (EnumMetaData)f;
-      Class<? extends Enum> c = (Class<? extends Enum>)enumMeta.enumClass;
-      List<String> symbols = new ArrayList<String>();
-      for (Enum e : c.getEnumConstants())
-        symbols.add(e.name());
-      return Schema.createEnum(c.getName(), null, null, symbols);
+      return nullable(getSchema(enumMeta.enumClass));
     case TType.LIST:
       ListMetaData listMeta = (ListMetaData)f;
-      return Schema.createArray(getSchema(listMeta.elemMetaData));
+      return nullable(Schema.createArray(getSchema(listMeta.elemMetaData)));
     case TType.MAP:
       MapMetaData mapMeta = (MapMetaData)f;
       if (mapMeta.keyMetaData.type != TType.STRING)
         throw new AvroRuntimeException("Map keys must be strings: "+f);
       Schema map = Schema.createMap(getSchema(mapMeta.valueMetaData));
       GenericData.setStringType(map, GenericData.StringType.String);
-      return map;
+      return nullable(map);
     case TType.SET:
       SetMetaData setMeta = (SetMetaData)f;
       Schema set = Schema.createArray(getSchema(setMeta.elemMetaData));
       set.addProp(THRIFT_PROP, "set");
-      return set;
+      return nullable(set);
     case TType.STRING:
       if (f.isBinary())
-        return Schema.create(Schema.Type.BYTES);
+        return nullable(Schema.create(Schema.Type.BYTES));
       Schema string = Schema.create(Schema.Type.STRING);
       GenericData.setStringType(string, GenericData.StringType.String);
-      return string;
+      return nullable(string);
     case TType.STRUCT:
       StructMetaData structMeta = (StructMetaData)f;
       Schema record = getSchema(structMeta.structClass);
-      return record;
+      return nullable(record);
     case TType.VOID:
-      return Schema.create(Schema.Type.NULL);
+      return NULL;
     default:
       throw new RuntimeException("Unexpected type in field: "+f);
     }
   }
 
+  private Schema nullable(Schema schema) {
+    return Schema.createUnion(Arrays.asList(new Schema[] {NULL, schema}));
+  }
+
 }

Modified: avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java?rev=1190627&r1=1190626&r2=1190627&view=diff
==============================================================================
--- avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java (original)
+++ avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/TestThrift.java Fri Oct
28 22:20:07 2011
@@ -55,6 +55,26 @@ public class TestThrift {
 
     System.out.println(test);
 
+    check(test);
+  }
+
+  @org.junit.Test public void testOptionals() throws Exception {
+
+    Test test = new Test();
+    test.setBoolField(true);
+    test.setByteField((byte)2);
+    test.setI16Field((short)3);
+    test.setI64Field(5L);
+    test.setDoubleField(2.0);
+
+    System.out.println(test);
+
+    check(test);
+  }
+
+
+  private void check(Test test) throws Exception {
+
     ByteArrayOutputStream bao = new ByteArrayOutputStream();
     ThriftDatumWriter<Test> w = new ThriftDatumWriter<Test>(Test.class);
     Encoder e = EncoderFactory.get().binaryEncoder(bao, null);

Modified: avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java?rev=1190627&r1=1190626&r2=1190627&view=diff
==============================================================================
--- avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java (original)
+++ avro/trunk/lang/java/thrift/src/test/java/org/apache/avro/thrift/test/Test.java Fri Oct
28 22:20:07 2011
@@ -167,7 +167,7 @@ public class Test implements org.apache.
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.BYTE)));
     tmpMap.put(_Fields.I16_FIELD, new org.apache.thrift.meta_data.FieldMetaData("i16Field",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.I16)));
-    tmpMap.put(_Fields.I32_FIELD, new org.apache.thrift.meta_data.FieldMetaData("i32Field",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
+    tmpMap.put(_Fields.I32_FIELD, new org.apache.thrift.meta_data.FieldMetaData("i32Field",
org.apache.thrift.TFieldRequirementType.OPTIONAL, 
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.I32)));
     tmpMap.put(_Fields.I64_FIELD, new org.apache.thrift.meta_data.FieldMetaData("i64Field",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.I64)));
@@ -175,7 +175,7 @@ public class Test implements org.apache.
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.DOUBLE)));
     tmpMap.put(_Fields.STRING_FIELD, new org.apache.thrift.meta_data.FieldMetaData("stringField",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING)));
-    tmpMap.put(_Fields.BINARY_FIELD, new org.apache.thrift.meta_data.FieldMetaData("binaryField",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
+    tmpMap.put(_Fields.BINARY_FIELD, new org.apache.thrift.meta_data.FieldMetaData("binaryField",
org.apache.thrift.TFieldRequirementType.OPTIONAL, 
         new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING
       , true)));
     tmpMap.put(_Fields.MAP_FIELD, new org.apache.thrift.meta_data.FieldMetaData("mapField",
org.apache.thrift.TFieldRequirementType.DEFAULT, 
         new org.apache.thrift.meta_data.MapMetaData(org.apache.thrift.protocol.TType.MAP,

@@ -202,11 +202,9 @@ public class Test implements org.apache.
     boolean boolField,
     byte byteField,
     short i16Field,
-    int i32Field,
     long i64Field,
     double doubleField,
     String stringField,
-    ByteBuffer binaryField,
     Map<String,Integer> mapField,
     List<Integer> listField,
     Set<Integer> setField,
@@ -220,14 +218,11 @@ public class Test implements org.apache.
     setByteFieldIsSet(true);
     this.i16Field = i16Field;
     setI16FieldIsSet(true);
-    this.i32Field = i32Field;
-    setI32FieldIsSet(true);
     this.i64Field = i64Field;
     setI64FieldIsSet(true);
     this.doubleField = doubleField;
     setDoubleFieldIsSet(true);
     this.stringField = stringField;
-    this.binaryField = binaryField;
     this.mapField = mapField;
     this.listField = listField;
     this.setField = setField;
@@ -900,8 +895,8 @@ public class Test implements org.apache.
         return false;
     }
 
-    boolean this_present_i32Field = true;
-    boolean that_present_i32Field = true;
+    boolean this_present_i32Field = true && this.isSetI32Field();
+    boolean that_present_i32Field = true && that.isSetI32Field();
     if (this_present_i32Field || that_present_i32Field) {
       if (!(this_present_i32Field && that_present_i32Field))
         return false;
@@ -1305,9 +1300,11 @@ public class Test implements org.apache.
     oprot.writeFieldBegin(I16_FIELD_FIELD_DESC);
     oprot.writeI16(this.i16Field);
     oprot.writeFieldEnd();
-    oprot.writeFieldBegin(I32_FIELD_FIELD_DESC);
-    oprot.writeI32(this.i32Field);
-    oprot.writeFieldEnd();
+    if (isSetI32Field()) {
+      oprot.writeFieldBegin(I32_FIELD_FIELD_DESC);
+      oprot.writeI32(this.i32Field);
+      oprot.writeFieldEnd();
+    }
     oprot.writeFieldBegin(I64_FIELD_FIELD_DESC);
     oprot.writeI64(this.i64Field);
     oprot.writeFieldEnd();
@@ -1320,9 +1317,11 @@ public class Test implements org.apache.
       oprot.writeFieldEnd();
     }
     if (this.binaryField != null) {
-      oprot.writeFieldBegin(BINARY_FIELD_FIELD_DESC);
-      oprot.writeBinary(this.binaryField);
-      oprot.writeFieldEnd();
+      if (isSetBinaryField()) {
+        oprot.writeFieldBegin(BINARY_FIELD_FIELD_DESC);
+        oprot.writeBinary(this.binaryField);
+        oprot.writeFieldEnd();
+      }
     }
     if (this.mapField != null) {
       oprot.writeFieldBegin(MAP_FIELD_FIELD_DESC);
@@ -1391,10 +1390,12 @@ public class Test implements org.apache.
     sb.append("i16Field:");
     sb.append(this.i16Field);
     first = false;
-    if (!first) sb.append(", ");
-    sb.append("i32Field:");
-    sb.append(this.i32Field);
-    first = false;
+    if (isSetI32Field()) {
+      if (!first) sb.append(", ");
+      sb.append("i32Field:");
+      sb.append(this.i32Field);
+      first = false;
+    }
     if (!first) sb.append(", ");
     sb.append("i64Field:");
     sb.append(this.i64Field);
@@ -1411,14 +1412,16 @@ public class Test implements org.apache.
       sb.append(this.stringField);
     }
     first = false;
-    if (!first) sb.append(", ");
-    sb.append("binaryField:");
-    if (this.binaryField == null) {
-      sb.append("null");
-    } else {
-      org.apache.thrift.TBaseHelper.toString(this.binaryField, sb);
+    if (isSetBinaryField()) {
+      if (!first) sb.append(", ");
+      sb.append("binaryField:");
+      if (this.binaryField == null) {
+        sb.append("null");
+      } else {
+        org.apache.thrift.TBaseHelper.toString(this.binaryField, sb);
+      }
+      first = false;
     }
-    first = false;
     if (!first) sb.append(", ");
     sb.append("mapField:");
     if (this.mapField == null) {

Modified: avro/trunk/lang/java/thrift/src/test/thrift/test.thrift
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/thrift/src/test/thrift/test.thrift?rev=1190627&r1=1190626&r2=1190627&view=diff
==============================================================================
--- avro/trunk/lang/java/thrift/src/test/thrift/test.thrift (original)
+++ avro/trunk/lang/java/thrift/src/test/thrift/test.thrift Fri Oct 28 22:20:07 2011
@@ -33,11 +33,11 @@ struct Test {
   1: bool boolField
   2: byte byteField
   3: i16 i16Field
-  4: i32 i32Field
+  4: optional i32 i32Field
   5: i64 i64Field
   6: double doubleField
   7: string stringField
-  8: binary binaryField
+  8: optional binary binaryField
   9: map<string,i32> mapField
  10: list<i32> listField
  11: set<i32> setField



Mime
View raw message