THRIFT-624. java: compareTo is broken for Unions with binary fields

This patch adds a special case for byte[] values in TUnion. It also fixes a related bug in TBaseHelper for comparing two byte arrays.

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@835065 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc
index e330099..230d0d0 100644 (file)
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -964,7 +964,12 @@ void t_java_generator::generate_union_comparisons(ofstream& out, t_struct* tstru
     indent(out) << "public int compareTo(" << type_name(tstruct) << " other) {" << endl;
     indent(out) << "  int lastComparison = TBaseHelper.compareTo(getSetField(), other.getSetField());" << endl;
     indent(out) << "  if (lastComparison == 0) {" << endl;
-    indent(out) << "    return TBaseHelper.compareTo((Comparable)getFieldValue(), (Comparable)other.getFieldValue());" << endl;
+    indent(out) << "    Object myValue = getFieldValue();" << endl;
+    indent(out) << "    if (myValue instanceof byte[]) {" << endl;
+    indent(out) << "      return TBaseHelper.compareTo((byte[])myValue, (byte[])other.getFieldValue());" << endl;
+    indent(out) << "    } else {" << endl;
+    indent(out) << "      return TBaseHelper.compareTo((Comparable)myValue, (Comparable)other.getFieldValue());" << endl;
+    indent(out) << "    }" << endl;
     indent(out) << "  }" << endl;
     indent(out) << "  return lastComparison;" << endl;
     indent(out) << "}" << endl;

diff --git a/lib/java/src/org/apache/thrift/TBaseHelper.java b/lib/java/src/org/apache/thrift/TBaseHelper.java
index 2cc83e0..2ec71f5 100644 (file)
--- a/lib/java/src/org/apache/thrift/TBaseHelper.java
+++ b/lib/java/src/org/apache/thrift/TBaseHelper.java
@@ -68,8 +68,8 @@ public class TBaseHelper {
       return sizeCompare;
     }
     for (int i = 0; i < a.length; i++) {
-      int byteCompare = compareTo(a, b);
-      if (byteCompare !=0) {
+      int byteCompare = compareTo(a[i], b[i]);
+      if (byteCompare != 0) {
         return byteCompare;
       }
     }

diff --git a/lib/java/test/org/apache/thrift/test/UnionTest.java b/lib/java/test/org/apache/thrift/test/UnionTest.java
index c2c8791..04716c6 100644 (file)
--- a/lib/java/test/org/apache/thrift/test/UnionTest.java
+++ b/lib/java/test/org/apache/thrift/test/UnionTest.java
@@ -7,6 +7,7 @@ import org.apache.thrift.transport.TMemoryBuffer;
 import thrift.test.Empty;
 import thrift.test.StructWithAUnion;
 import thrift.test.TestUnion;
+import thrift.test.ComparableUnion;
 
 public class UnionTest {
 
@@ -15,8 +16,9 @@ public class UnionTest {
    */
   public static void main(String[] args) throws Exception {
     testBasic();
-    testEquality();    
+    testEquality();
     testSerialization();
+    testCompareTo();
   }
 
   public static void testBasic() throws Exception {
@@ -131,6 +133,38 @@ public class UnionTest {
 
     swau.write(proto);
     new Empty().read(proto);
+  }
+  
+  public static void testCompareTo() throws Exception {
+    ComparableUnion cu = ComparableUnion.string_field("a");
+    ComparableUnion cu2 = ComparableUnion.string_field("b");
+
+    if (cu.compareTo(cu2) != -1) {
+      throw new RuntimeException("a was supposed to be < b, but was " + cu.compareTo(cu2));
+    }
+
+    if (cu2.compareTo(cu) != 1) {
+      throw new RuntimeException("b was supposed to be > a, but was " + cu2.compareTo(cu));
+    }
+
+    cu2 = ComparableUnion.binary_field(new byte[]{2});
+
+    if (cu.compareTo(cu2) != -1) {
+      throw new RuntimeException("a was supposed to be < b, but was " + cu.compareTo(cu2));
+    }
+
+    if (cu2.compareTo(cu) != 1) {
+      throw new RuntimeException("b was supposed to be > a, but was " + cu2.compareTo(cu));
+    }
+
+    cu = ComparableUnion.binary_field(new byte[]{1});
 
+    if (cu.compareTo(cu2) != -1) {
+      throw new RuntimeException("a was supposed to be < b, but was " + cu.compareTo(cu2));
+    }
+
+    if (cu2.compareTo(cu) != 1) {
+      throw new RuntimeException("b was supposed to be > a, but was " + cu2.compareTo(cu));
+    }
   }
 }

diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift
index 48df1fc..5b2f672 100644 (file)
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -263,6 +263,11 @@ union TestUnion {
   5: i32 other_i32_field;
 }
 
+union ComparableUnion {
+  1: string string_field;
+  2: binary binary_field;
+}
+
 struct StructWithAUnion {
   1: TestUnion test_union;
 }