svn commit: r998275 - 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/TestTBaseHelper.java

[bryanduxbury]

Author: bryanduxbury
Date: Fri Sep 17 19:27:36 2010
New Revision: 998275

URL: http://svn.apache.org/viewvc?rev=998275&view=rev
Log:
THRIFT-882. java: deep copy of binary fields does not copy ByteBuffer 
characteristics (arrayOffset, position)

This patch ensures that binary fields are copied correctly.


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/TestTBaseHelper.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=998275&r1=998274&r2=998275&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 Fri 
Sep 17 19:27:36 2010
@@ -1393,7 +1393,7 @@ void t_java_generator::generate_java_str
     indent(out) << "  return lastComparison;" << endl;
     indent(out) << "}" << endl;
 
-    indent(out) << "if (" << generate_isset_check(field) << ") {";
+    indent(out) << "if (" << generate_isset_check(field) << ") {" << endl;
     indent(out) << "  lastComparison = TBaseHelper.compareTo(this." << 
field->get_name() << ", typedOther." << field->get_name() << ");" << endl;
     indent(out) << "  if (lastComparison != 0) {" << endl;
     indent(out) << "    return lastComparison;" << endl;
@@ -3732,14 +3732,12 @@ void t_java_generator::generate_deep_cop
 
 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()) {
-    // binary fields need to be copied with System.arraycopy
-    if (((t_base_type*)type)->is_binary()){
-      out << "ByteBuffer.wrap(new byte[" << source_name << ".limit() - " << 
source_name << ".arrayOffset()]);" << endl;
-      indent(out) << "System.arraycopy(" << source_name << ".array(), " << 
source_name << ".arrayOffset(), " << dest_name << ".array(), 0, " << 
source_name << ".limit() - " << source_name << ".arrayOffset())";
-    }
-    // everything else can be copied directly
-    else
+    if (((t_base_type*)type)->is_binary()) {
+      out << "TBaseHelper.copyBinary(" << source_name << ");" << endl;
+    } else {
+      // everything else can be copied directly
       out << source_name;
+    }
   } else {
     out << "new " << type_name(type, true, true) << "(" << source_name << ")";
   }

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=998275&r1=998274&r2=998275&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 Fri 
Sep 17 19:27:36 2010
@@ -28,7 +28,9 @@ import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
 
-public class TBaseHelper {
+public final class TBaseHelper {
+
+  private TBaseHelper(){}
 
   private static final Comparator comparator = new NestedStructureComparator();
 
@@ -273,4 +275,21 @@ public class TBaseHelper {
     }
     return ByteBuffer.wrap(byteBufferToByteArray(in));
   }
+
+  public static ByteBuffer copyBinary(final ByteBuffer orig) {
+    ByteBuffer copy = ByteBuffer.wrap(new byte[orig.remaining()]);
+    if (orig.hasArray()) {
+      System.arraycopy(orig.array(), orig.arrayOffset() + orig.position(), 
copy.array(), 0, orig.remaining());
+    } else {
+      orig.slice().get(copy.array());
+    }
+
+    return copy;
+  }
+
+  public static byte[] copyBinary(final byte[] orig) {
+    byte[] copy = new byte[orig.length];
+    System.arraycopy(orig, 0, copy, 0, orig.length);
+    return copy;
+  }
 }

Modified: 
incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTBaseHelper.java
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTBaseHelper.java?rev=998275&r1=998274&r2=998275&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTBaseHelper.java 
(original)
+++ incubator/thrift/trunk/lib/java/test/org/apache/thrift/TestTBaseHelper.java 
Fri Sep 17 19:27:36 2010
@@ -150,4 +150,35 @@ public class TestTBaseHelper extends Tes
     assertEquals(3, b3.length);
     assertEquals(ByteBuffer.wrap(b1, 1, 3), ByteBuffer.wrap(b3));
   }
+
+  public void testCopyBinaryWithByteBuffer() throws Exception {
+    byte[] bytes = new byte[]{0, 1, 2, 3, 4, 5};
+    ByteBuffer b = ByteBuffer.wrap(bytes);
+    ByteBuffer bCopy = TBaseHelper.copyBinary(b);
+    assertEquals(b, bCopy);
+    assertEquals(0, b.position());
+
+    b = ByteBuffer.allocateDirect(6);
+    b.put(bytes);
+    b.position(0);
+    bCopy = TBaseHelper.copyBinary(b);
+    assertEquals(6, b.remaining());
+    assertEquals(0, b.position());
+    assertEquals(b, bCopy);
+
+    b.mark();
+    b.get();
+    bCopy = TBaseHelper.copyBinary(b);
+    assertEquals(ByteBuffer.wrap(bytes, 1, 5), bCopy);
+    assertEquals(1, b.position());
+    b.reset();
+    assertEquals(0, b.position());
+  }
+
+  public void testCopyBinaryWithByteArray() throws Exception {
+    byte[] bytes = new byte[]{0, 1, 2, 3, 4, 5};
+    byte[] copy = TBaseHelper.copyBinary(bytes);
+    assertEquals(ByteBuffer.wrap(bytes), ByteBuffer.wrap(copy));
+    assertNotSame(bytes, copy);
+  }
 }