You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by br...@apache.org on 2010/06/23 23:17:48 UTC
svn commit: r957350 - in /incubator/thrift/trunk:
compiler/cpp/src/generate/t_java_generator.cc
lib/java/src/org/apache/thrift/TBaseHelper.java
lib/java/test/org/apache/thrift/TestTUnion.java test/DebugProtoTest.thrift
Author: bryanduxbury
Date: Wed Jun 23 21:17:48 2010
New Revision: 957350
URL: http://svn.apache.org/viewvc?rev=957350&view=rev
Log:
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.
Modified:
incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
incubator/thrift/trunk/lib/java/src/org/apache/thrift/TBaseHelper.java
incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTUnion.java
incubator/thrift/trunk/test/DebugProtoTest.thrift
Modified: incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc?rev=957350&r1=957349&r2=957350&view=diff
==============================================================================
--- incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc (original)
+++ incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc Wed Jun 23 21:17:48 2010
@@ -1010,12 +1010,7 @@ void t_java_generator::generate_union_co
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;
Modified: incubator/thrift/trunk/lib/java/src/org/apache/thrift/TBaseHelper.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/src/org/apache/thrift/TBaseHelper.java?rev=957350&r1=957349&r2=957350&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/src/org/apache/thrift/TBaseHelper.java (original)
+++ incubator/thrift/trunk/lib/java/src/org/apache/thrift/TBaseHelper.java Wed Jun 23 21:17:48 2010
@@ -31,6 +31,22 @@ public class TBaseHelper {
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);
}
Modified: incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTUnion.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTUnion.java?rev=957350&r1=957349&r2=957350&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTUnion.java (original)
+++ incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTUnion.java Wed Jun 23 21:17:48 2010
@@ -24,10 +24,16 @@ import org.apache.thrift.transport.TMemo
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 @@ public class TestTUnion extends TestCase
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 {
Modified: incubator/thrift/trunk/test/DebugProtoTest.thrift
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DebugProtoTest.thrift?rev=957350&r1=957349&r2=957350&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DebugProtoTest.thrift (original)
+++ incubator/thrift/trunk/test/DebugProtoTest.thrift Wed Jun 23 21:17:48 2010
@@ -299,6 +299,8 @@ union TestUnion {
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 @@ union TestUnionMinusStringField {
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 @@ struct BreaksRubyCompactProtocol {
1: string field1;
2: BigFieldIdStruct field2;
3: i32 field3;
-}
\ No newline at end of file
+}
+