[jira] Updated: (THRIFT-138) Generated Thrift structs should have a deep-copy constructor

["Piotr Kozikowski (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/THRIFT-138?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Piotr Kozikowski updated THRIFT-138:
------------------------------------

    Attachment: thrift-138-v9.patch

To fix this seemed very straightforward, but actually it was quite tricky. Here is a patch. It's a bit of a hack but I tested it very thoroughly. I added a simple test case to DeepCopyTest.java. You can check that without the changes it doesn't pass. 

I also did a more specific test locally as follows:

thrift file:

struct SomeStruct {
  1: i32 num,
  2: string name,
  3: binary bin,
  4: map<binary,binary> mapvar
  5: list<binary> listvar
}

java file:

SomeStruct s = new SomeStruct();
s.setBin("hello".getBytes());
s.setMapvar(new HashMap<byte[], byte[]>());
s.getMapvar().put("key".getBytes(), "value".getBytes());
s.setListvar(new LinkedList<byte[]>());
s.getListvar().add("element".getBytes());
SomeStruct s2 = new SomeStruct(s);      
      
// Basic case
s.getBin()[0]++;
if (s.equals(s2))
  System.out.println("Error!");
s.getBin()[0]--;
      
// Map key
s.getMapvar().keySet().iterator().next()[0]++;
if (s.equals(s2))
  System.out.println("Error!");
s.getMapvar().keySet().iterator().next()[0]--;
      
// Map value
s.getMapvar().values().iterator().next()[0]++;
if (s.equals(s2))
  System.out.println("Error!");
s.getMapvar().values().iterator().next()[0]--;
     
// List
s.getListvar().get(0)[0]++;
if (s.equals(s2))
  System.out.println("Error!");


Without the patch, I got all four errors, and with it none of them. I think this pretty much covers all possible cases.

> Generated Thrift structs should have a deep-copy constructor
> ------------------------------------------------------------
>
>                 Key: THRIFT-138
>                 URL: https://issues.apache.org/jira/browse/THRIFT-138
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Piotr Kozikowski
>            Priority: Minor
>         Attachments: thrift-138-v2.patch, thrift-138-v3.patch, thrift-138-v4.patch, thrift-138-v5.patch, thrift-138-v6.patch, thrift-138-v7-test.patch, thrift-138-v7.patch, thrift-138-v8.patch, thrift-138-v9.patch, thrift-138.patch
>
>
> It'd be nice to be able to make a copy of a Thrift object by passing it as an argument to the constructor of the same object. Right now, we have to use a serialize/deserialize approach to making a deep object copy, and that seems really inefficient.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.