From: Bryan Duxbury <bryanduxbury@apache.org>
Date: Thu, 31 Dec 2009 18:59:15 +0000 (+0000)
Subject: THRIFT-670. java: Unions don't skip unrecognizable fields correctly
X-Git-Tag: 0.3.0~146
X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=c2ec7cae04b1a18a0fbb2dc3802fb3d18f0c080c;p=common%2Fthrift.git

THRIFT-670. java: Unions don't skip unrecognizable fields correctly

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

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@894924 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 da1e1c69..28406d97 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -850,7 +850,10 @@ void t_java_generator::generate_read_value(ofstream& out, t_struct* tstruct) {
 
   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,11 +876,18 @@ void t_java_generator::generate_read_value(ofstream& out, t_struct* tstruct) {
     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;
 
diff --git a/lib/java/test/org/apache/thrift/test/UnionTest.java b/lib/java/test/org/apache/thrift/test/UnionTest.java
index 551e4078..2527f4a7 100644
--- a/lib/java/test/org/apache/thrift/test/UnionTest.java
+++ b/lib/java/test/org/apache/thrift/test/UnionTest.java
@@ -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 @@ public class UnionTest {
     testEquality();
     testSerialization();
     testCompareTo();
+    testSkip();
   }
 
   public static void testBasic() throws Exception {
@@ -189,4 +193,17 @@ public class UnionTest {
       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!");
+    }
+  }
 }
diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift
index 6731e5e4..9cdb16d0 100644
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -299,6 +299,14 @@ union TestUnion {
   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;