THRFIT-804. java: CompareTo is broken for unions set to map, set, or list
This patch fixes TUnion's compareTo, and factors out the standard part of the comparison to TBaseHelper.
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@957350 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 2db3ca3..89c1956 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -1010,12 +1010,7 @@
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) << " 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) << " return TBaseHelper.compareTo(getFieldValue(), other.getFieldValue());" << 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 fccece8..55e9505 100644
--- a/lib/java/src/org/apache/thrift/TBaseHelper.java
+++ b/lib/java/src/org/apache/thrift/TBaseHelper.java
@@ -31,6 +31,22 @@
private static final Comparator comparator = new NestedStructureComparator();
+ public static int compareTo(Object o1, Object o2) {
+ if (o1 instanceof Comparable) {
+ return compareTo((Comparable)o1, (Comparable)o2);
+ } else if (o1 instanceof List) {
+ return compareTo((List)o1, (List)o2);
+ } else if (o1 instanceof Set) {
+ return compareTo((Set)o1, (Set)o2);
+ } else if (o1 instanceof Map) {
+ return compareTo((Map)o1, (Map)o2);
+ } else if (o1 instanceof byte[]) {
+ return compareTo((byte[])o1, (byte[])o2);
+ } else {
+ throw new IllegalArgumentException("Cannot compare objects of type " + o1.getClass());
+ }
+ }
+
public static int compareTo(boolean a, boolean b) {
return Boolean.valueOf(a).compareTo(b);
}
diff --git a/lib/java/test/org/apache/thrift/TestTUnion.java b/lib/java/test/org/apache/thrift/TestTUnion.java
index 2ff80e9..a8b1287 100644
--- a/lib/java/test/org/apache/thrift/TestTUnion.java
+++ b/lib/java/test/org/apache/thrift/TestTUnion.java
@@ -24,10 +24,16 @@
import thrift.test.ComparableUnion;
import thrift.test.Empty;
+import thrift.test.RandomStuff;
import thrift.test.SomeEnum;
import thrift.test.StructWithAUnion;
import thrift.test.TestUnion;
import thrift.test.TestUnionMinusStringField;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
import junit.framework.TestCase;
public class TestTUnion extends TestCase {
@@ -98,6 +104,22 @@
assertTrue(cu.compareTo(cu2) < 0);
assertTrue(cu2.compareTo(cu) > 0);
+
+ TestUnion union1 = new TestUnion(TestUnion._Fields.STRUCT_LIST, new ArrayList<RandomStuff>());
+ TestUnion union2 = new TestUnion(TestUnion._Fields.STRUCT_LIST, new ArrayList<RandomStuff>());
+ assertTrue(union1.compareTo(union2) == 0);
+
+ TestUnion union3 = new TestUnion(TestUnion._Fields.I32_SET, new HashSet<Integer>());
+ Set<Integer> i32_set = new HashSet<Integer>();
+ i32_set.add(1);
+ TestUnion union4 = new TestUnion(TestUnion._Fields.I32_SET, i32_set);
+ assertTrue(union3.compareTo(union4) < 0);
+
+ Map<Integer, Integer> i32_map = new HashMap<Integer, Integer>();
+ i32_map.put(1,1);
+ TestUnion union5 = new TestUnion(TestUnion._Fields.I32_MAP, i32_map);
+ TestUnion union6 = new TestUnion(TestUnion._Fields.I32_MAP, new HashMap<Integer, Integer>());
+ assertTrue(union5.compareTo(union6) > 0);
}
public void testEquality() throws Exception {
diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift
index 5e361d2..c67fbcb 100644
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -299,6 +299,8 @@
4: list<RandomStuff> struct_list;
5: i32 other_i32_field;
6: SomeEnum enum_field;
+ 7: set<i32> i32_set;
+ 8: map<i32, i32> i32_map;
}
union TestUnionMinusStringField {
@@ -307,6 +309,8 @@
4: list<RandomStuff> struct_list;
5: i32 other_i32_field;
6: SomeEnum enum_field;
+ 7: set<i32> i32_set;
+ 8: map<i32, i32> i32_map;
}
union ComparableUnion {
@@ -339,4 +343,5 @@
1: string field1;
2: BigFieldIdStruct field2;
3: i32 field3;
-}
\ No newline at end of file
+}
+