svn commit: r894924 - in /incubator/thrift/trunk: compiler/cpp/src/generate/t_java_generator.cc lib/java/test/org/apache/thrift/test/UnionTest.java test/DebugProtoTest.thrift

[bryanduxbury]

Author: bryanduxbury
Date: Thu Dec 31 18:59:15 2009
New Revision: 894924

URL: http://svn.apache.org/viewvc?rev=894924&view=rev
Log:
THRIFT-670. java: Unions don't skip unrecognizable fields correctly

This patch adds a test and a fix for the bug.

Modified:
    incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
    incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/UnionTest.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=894924&r1=894923&r2=894924&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 Thu 
Dec 31 18:59:15 2009
@@ -850,7 +850,10 @@
 
   indent_up();
 
-  indent(out) << "switch (_Fields.findByThriftId(field.id)) {" << endl;
+  indent(out) << "_Fields setField = _Fields.findByThriftId(field.id);" << 
endl;
+  indent(out) << "if (setField != null) {" << endl;
+  indent_up();
+  indent(out) << "switch (setField) {" << endl;
   indent_up();
 
   const vector<t_field*>& members = tstruct->get_members();
@@ -873,12 +876,19 @@
     indent(out) << "}" << endl;
     indent_down();
   }
-  
+
   indent(out) << "default:" << endl;
-  indent(out) << "  TProtocolUtil.skip(iprot, field.type);" << endl;
-  indent(out) << "  return null;" << endl;
+  indent(out) << "  throw new IllegalStateException(\"setField wasn't null, 
but didn't match any of the case statements!\");" << endl;
+
+  indent_down();
+  indent(out) << "}" << endl;
 
   indent_down();
+  indent(out) << "} else {" << endl;
+  indent_up();
+  indent(out) << "TProtocolUtil.skip(iprot, field.type);" << endl;
+  indent(out) << "return null;" << endl;
+  indent_down();
   indent(out) << "}" << endl;
 
   indent_down();

Modified: 
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/UnionTest.java
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/UnionTest.java?rev=894924&r1=894923&r2=894924&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/UnionTest.java 
(original)
+++ incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/UnionTest.java 
Thu Dec 31 18:59:15 2009
@@ -17,15 +17,18 @@
  */
 package org.apache.thrift.test;
 
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TSerializer;
 import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.transport.TMemoryBuffer;
 
+import thrift.test.ComparableUnion;
 import thrift.test.Empty;
+import thrift.test.SomeEnum;
 import thrift.test.StructWithAUnion;
 import thrift.test.TestUnion;
-import thrift.test.SomeEnum;
-import thrift.test.ComparableUnion;
+import thrift.test.TestUnionMinusStringField;
 
 public class UnionTest {
 
@@ -37,6 +40,7 @@
     testEquality();
     testSerialization();
     testCompareTo();
+    testSkip();
   }
 
   public static void testBasic() throws Exception {
@@ -189,4 +193,17 @@
       throw new RuntimeException("b was supposed to be > a, but was " + 
cu2.compareTo(cu));
     }
   }
+
+  public static void testSkip() throws Exception {
+    TestUnion tu = TestUnion.string_field("string");
+    byte[] tuSerialized = new TSerializer().serialize(tu);
+    TestUnionMinusStringField tums = new TestUnionMinusStringField();
+    new TDeserializer().deserialize(tums, tuSerialized);
+    if (tums.getSetField() != null) {
+      throw new RuntimeException("Expected tums to be unset!");
+    }
+    if (tums.getFieldValue() != null) {
+      throw new RuntimeException("Expected tums to have null value!");
+    }
+  }
 }

Modified: incubator/thrift/trunk/test/DebugProtoTest.thrift
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DebugProtoTest.thrift?rev=894924&r1=894923&r2=894924&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DebugProtoTest.thrift (original)
+++ incubator/thrift/trunk/test/DebugProtoTest.thrift Thu Dec 31 18:59:15 2009
@@ -299,6 +299,14 @@
   6: SomeEnum enum_field;
 }
 
+union TestUnionMinusStringField {
+  2: i32 i32_field;
+  3: OneOfEach struct_field;
+  4: list<RandomStuff> struct_list;
+  5: i32 other_i32_field;
+  6: SomeEnum enum_field;
+}
+
 union ComparableUnion {
   1: string string_field;
   2: binary binary_field;