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
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -964,7 +964,12 @@
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
--- a/lib/java/src/org/apache/thrift/TBaseHelper.java
+++ b/lib/java/src/org/apache/thrift/TBaseHelper.java
@@ -68,8 +68,8 @@
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
--- 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 thrift.test.Empty;
import thrift.test.StructWithAUnion;
import thrift.test.TestUnion;
+import thrift.test.ComparableUnion;
public class UnionTest {
@@ -15,8 +16,9 @@
*/
public static void main(String[] args) throws Exception {
testBasic();
- testEquality();
+ testEquality();
testSerialization();
+ testCompareTo();
}
public static void testBasic() throws Exception {
@@ -131,6 +133,38 @@
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
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -263,6 +263,11 @@
5: i32 other_i32_field;
}
+union ComparableUnion {
+ 1: string string_field;
+ 2: binary binary_field;
+}
+
struct StructWithAUnion {
1: TestUnion test_union;
}