Repository: thrift
Updated Branches:
  refs/heads/master a105450fd -> 3311a9b23


THRIFT-4177 fix java deep copy
Client: Java

Java compiler produces deep copy constructor that could make shallow copy 
accidentally.

This closes #1254


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/3311a9b2
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/3311a9b2
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/3311a9b2

Branch: refs/heads/master
Commit: 3311a9b2375276441234218f4351c6a8f66a6bc2
Parents: a105450
Author: Deniss Afonin <deniss.afo...@creative-mobile.com>
Authored: Tue Apr 18 19:27:49 2017 +0300
Committer: James E. King, III <jk...@apache.org>
Committed: Wed Apr 19 12:38:14 2017 -0400

----------------------------------------------------------------------
 .../cpp/src/thrift/generate/t_java_generator.cc |  1 +
 lib/java/build.xml                              |  3 ++
 .../test/org/apache/thrift/TestDeepCopy.java    | 34 ++++++++++++++++++++
 test/JavaDeepCopyTest.thrift                    | 19 +++++++++++
 test/Makefile.am                                |  1 +
 5 files changed, 58 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/3311a9b2/compiler/cpp/src/thrift/generate/t_java_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc 
b/compiler/cpp/src/thrift/generate/t_java_generator.cc
index 80b8eef..3408bf6 100644
--- a/compiler/cpp/src/thrift/generate/t_java_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc
@@ -4657,6 +4657,7 @@ void 
t_java_generator::generate_deep_copy_non_container(ofstream& out,
                                                         std::string dest_name,
                                                         t_type* type) {
   (void)dest_name;
+  type = get_true_type(type);
   if (type->is_base_type() || type->is_enum() || type->is_typedef()) {
     if (type->is_binary()) {
       out << "org.apache.thrift.TBaseHelper.copyBinary(" << source_name << ")";

http://git-wip-us.apache.org/repos/asf/thrift/blob/3311a9b2/lib/java/build.xml
----------------------------------------------------------------------
diff --git a/lib/java/build.xml b/lib/java/build.xml
index 6af9f27..e40eafa 100644
--- a/lib/java/build.xml
+++ b/lib/java/build.xml
@@ -287,6 +287,9 @@
     <exec executable="${thrift.compiler}" failonerror="true">
       <arg line="--gen java:reuse-objects -out ${genreuse} 
${test.thrift.home}/ReuseObjects.thrift"/>
     </exec>
+    <exec executable="${thrift.compiler}" failonerror="true">
+      <arg line="--gen java ${test.thrift.home}/JavaDeepCopyTest.thrift"/>
+    </exec>
   </target>
 
   <target name="proxy" if="proxy.enabled">

http://git-wip-us.apache.org/repos/asf/thrift/blob/3311a9b2/lib/java/test/org/apache/thrift/TestDeepCopy.java
----------------------------------------------------------------------
diff --git a/lib/java/test/org/apache/thrift/TestDeepCopy.java 
b/lib/java/test/org/apache/thrift/TestDeepCopy.java
new file mode 100644
index 0000000..acafaef
--- /dev/null
+++ b/lib/java/test/org/apache/thrift/TestDeepCopy.java
@@ -0,0 +1,34 @@
+package org.apache.thrift;
+
+import junit.framework.TestCase;
+import thrift.test.DeepCopyBar;
+import thrift.test.DeepCopyFoo;
+
+public class TestDeepCopy extends TestCase {
+
+  public void testDeepCopy() throws Exception {
+    final DeepCopyFoo foo = new DeepCopyFoo();
+
+    foo.addToL(new DeepCopyBar());
+    foo.addToS(new DeepCopyBar());
+    foo.putToM("test 3", new DeepCopyBar());
+
+    foo.addToLi(new thrift.test.Object());
+    foo.addToSi(new thrift.test.Object());
+    foo.putToMi("test 3", new thrift.test.Object());
+
+    foo.setBar(new DeepCopyBar());
+
+    final DeepCopyFoo deepCopyFoo = foo.deepCopy();
+
+    assertNotSame(foo.getBar(), deepCopyFoo.getBar());
+
+    assertNotSame(foo.getL().get(0),                          
deepCopyFoo.getL().get(0));
+    assertNotSame(foo.getS().toArray(new DeepCopyBar[0])[0],  
deepCopyFoo.getS().toArray(new DeepCopyBar[0])[0]);
+    assertNotSame(foo.getM().get("test 3"),                   
deepCopyFoo.getM().get("test 3"));
+
+    assertNotSame(foo.getLi().get(0),                                 
deepCopyFoo.getLi().get(0));
+    assertNotSame(foo.getSi().toArray(new thrift.test.Object[0])[0],  
deepCopyFoo.getSi().toArray(new thrift.test.Object[0])[0]);
+    assertNotSame(foo.getMi().get("test 3"),                          
deepCopyFoo.getMi().get("test 3"));
+  }
+}

http://git-wip-us.apache.org/repos/asf/thrift/blob/3311a9b2/test/JavaDeepCopyTest.thrift
----------------------------------------------------------------------
diff --git a/test/JavaDeepCopyTest.thrift b/test/JavaDeepCopyTest.thrift
new file mode 100644
index 0000000..fc974ae
--- /dev/null
+++ b/test/JavaDeepCopyTest.thrift
@@ -0,0 +1,19 @@
+include "JavaTypes.thrift"
+
+namespace java thrift.test
+
+struct DeepCopyFoo {
+  1: optional list<DeepCopyBar> l,
+  2: optional set<DeepCopyBar> s,
+  3: optional map<string, DeepCopyBar> m,
+  4: optional list<JavaTypes.Object> li,
+  5: optional set<JavaTypes.Object> si,
+  6: optional map<string, JavaTypes.Object> mi,
+  7: optional DeepCopyBar bar,
+}
+
+struct DeepCopyBar {
+  1: optional string a,
+  2: optional i32 b,
+  3: optional bool c,
+}

http://git-wip-us.apache.org/repos/asf/thrift/blob/3311a9b2/test/Makefile.am
----------------------------------------------------------------------
diff --git a/test/Makefile.am b/test/Makefile.am
index 01fab4f..5e4ebcf 100755
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -134,6 +134,7 @@ EXTRA_DIST = \
        FullCamelTest.thrift \
        Include.thrift \
        JavaBeansTest.thrift \
+       JavaDeepCopyTest.thrift \
        JavaTypes.thrift \
        JsDeepConstructorTest.thrift \
        ManyOptionals.thrift \

Reply via email to