Author: dreiss
Date: Mon Jan  5 13:02:48 2009
New Revision: 731719

URL: http://svn.apache.org/viewvc?rev=731719&view=rev
Log:
THRIFT-138. java: Fix copy constructor for binary fields

Modified:
    incubator/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
    incubator/thrift/trunk/test/java/src/DeepCopyTest.java

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=731719&r1=731718&r2=731719&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 Mon 
Jan  5 13:02:48 2009
@@ -148,7 +148,7 @@
                                           t_doc*     tdoc);
 
   void generate_deep_copy_container(std::ofstream& out, std::string 
source_name_p1, std::string source_name_p2, std::string result_name, t_type* 
type);
-  void generate_deep_copy_non_container(std::ofstream& out, std::string 
source_name, t_type* type);
+  void generate_deep_copy_non_container(std::ofstream& out, std::string 
source_name, std::string dest_name, t_type* type);
 
   /**
    * Helper rendering functions
@@ -684,7 +684,7 @@
       indent(out) << "this." << field_name << " = __this__" << field_name << 
";" << endl;
     } else {
       indent(out) << "this." << field_name << " = ";
-      generate_deep_copy_non_container(out, "other." + field_name, type);
+      generate_deep_copy_non_container(out, "other." + field_name, field_name, 
type);
       out << ";" << endl;
     }
 
@@ -2628,7 +2628,7 @@
       generate_deep_copy_container(out, iterator_element_name + "_key", "", 
result_element_name + "_key", key_type);
     } else {
       indent(out) << type_name(key_type, true, false) << " " << 
result_element_name << "_key = ";
-      generate_deep_copy_non_container(out, iterator_element_name + "_key", 
key_type);
+      generate_deep_copy_non_container(out, iterator_element_name + "_key", 
result_element_name + "_key", key_type);
       out << ";" << endl;
     }
 
@@ -2638,7 +2638,7 @@
       generate_deep_copy_container(out, iterator_element_name + "_value", "", 
result_element_name + "_value", val_type);
     } else {
       indent(out) << type_name(val_type, true, false) << " " << 
result_element_name << "_value = ";
-      generate_deep_copy_non_container(out, iterator_element_name + "_value", 
val_type);
+      generate_deep_copy_non_container(out, iterator_element_name + "_value", 
result_element_name + "_value", val_type);
       out << ";" << endl;
     }
 
@@ -2669,9 +2669,17 @@
       indent(out) << result_name << ".add(" << result_element_name << ");" << 
endl;
     } else {
       // iterative copy
-      indent(out) << result_name << ".add(";
-      generate_deep_copy_non_container(out, iterator_element_name, elem_type);
-      out << ");" << endl;
+      if(((t_base_type*)elem_type)->is_binary()){
+        indent(out) << "byte[] temp_binary_element = ";
+        generate_deep_copy_non_container(out, iterator_element_name, 
"temp_binary_element", elem_type);
+        out << ";" << endl;
+        indent(out) << result_name << ".add(temp_binary_element);" << endl;
+      }
+      else{
+        indent(out) << result_name << ".add(";
+        generate_deep_copy_non_container(out, iterator_element_name, 
result_name, elem_type);
+        out << ");" << endl;
+      }
     }
 
     indent_down();
@@ -2681,9 +2689,16 @@
   }
 }
 
-void t_java_generator::generate_deep_copy_non_container(ofstream& out, 
std::string source_name, t_type* type) {
+void t_java_generator::generate_deep_copy_non_container(ofstream& out, 
std::string source_name, std::string dest_name, t_type* type) {
   if (type->is_base_type() || type->is_enum() || type->is_typedef()) {
-    out << source_name;
+    // binary fields need to be copied with System.arraycopy
+    if (((t_base_type*)type)->is_binary()){
+      out << "new byte[" << source_name << ".length];" << endl;
+      indent(out) << "System.arraycopy(" << source_name << ", 0, " << 
dest_name << ", 0, " << source_name << ".length)";
+    }
+    // everything else can be copied directly
+    else
+      out << source_name;
   } else {
     out << "new " << type_name(type, true, true) << "(" << source_name << ")";
   }

Modified: incubator/thrift/trunk/test/java/src/DeepCopyTest.java
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/test/java/src/DeepCopyTest.java?rev=731719&r1=731718&r2=731719&view=diff
==============================================================================
--- incubator/thrift/trunk/test/java/src/DeepCopyTest.java (original)
+++ incubator/thrift/trunk/test/java/src/DeepCopyTest.java Mon Jan  5 13:02:48 
2009
@@ -37,6 +37,7 @@
     ooe.double_precision = Math.PI;
     ooe.some_characters = "JSON THIS! \"\1";
     ooe.zomg_unicode = new String(kUnicodeBytes, "UTF-8");
+    ooe.base64 = "string to bytes".getBytes();
 
     Nesting n = new Nesting(new Bonk(), new OneOfEach());
     n.my_ooe.integer16 = 16;
@@ -104,11 +105,15 @@
     if (!hmCopy.equals(hmCopy2))
       throw new RuntimeException("copy constructor generated incorrect copy");
 
+    hm.big.get(0).base64[0]++; // change binary value in original object
+    if (hm.equals(hmCopy2)) // make sure the change didn't propagate to the 
copied object
+      throw new RuntimeException("Binary field not copied correctly!");
+    hm.big.get(0).base64[0]--; // undo change
+
     hmCopy2.bonks.get("nothing").get(1).message = "What else?";
 
     if (hm.equals(hmCopy2))
       throw new RuntimeException("A deep copy was not done!");
 
-    //System.out.println("DeepCopyTest passed!");
   }
 }


Reply via email to