[jira] Updated: (THRIFT-1039) Refactor methods accessing binary data

["Mathias Herberts (JIRA)" <ji...@apache.org>]

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

Mathias Herberts updated THRIFT-1039:
-------------------------------------

    Attachment: THRIFT-1039.patch

Attached patch that replaces the 'byte[] getXxx()' method with 'ByteBuffer getXxx()' and adds a 'byte[] copyBytes(ByteBuffer buf)' method to the generated Java classes.

The change in getXxx will break existing code using structs with binary fields, but the introduction of copyBytes will clarify the semantics of retrieving byte arrays from ByteBuffers.

I voluntarly added copyBytes to the generated code instead of adding it to TBaseHelper as it made the uses more clear (IMHO):

struct A {
  1: binary bin_field,
}

A a = new A();
a.setBin_field(new byte[2]);
byte[] b = a.copyBytes(a.getBin_field());

> Refactor methods accessing binary data
> --------------------------------------
>
>                 Key: THRIFT-1039
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1039
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Java - Compiler
>         Environment: All
>            Reporter: Mathias Herberts
>            Priority: Minor
>         Attachments: THRIFT-1039.patch
>
>
> Since THRIFT-830, binary fields are implemented using ByteBuffer.
> struct A {
>   1: binary bin_field,
> }
> will generate a method 'byte[] getBin_field()' which will optionally resize the underlying ByteBuffer so it has a capacity of 'remaining()' and return the backing array of this new ByteBuffer.
> This has several implications.
> First the ByteBuffer in 'bin_field' might be modified, thus misleading users into thinking that the call to 'getBin_field()' had no effect on the underlying structure.
> Thus the following will fail:
> byte[] b = new byte[2];
> b[0] = 0x01;
> A a = new A();
> a.setBin_field(ByteBuffer.wrap(b, 0, 1));
> byte[] bb = a.getBin_field();
> b[0] = 0x02;
> Assert.assertEquals(0x02, bb[0]);
> Second it creates a singularity in the getters as all other getters involving containers of binary data will return collections of ByteBuffer.
> I suggest we refactor to something more in the line of what HADOOP-6298 has done, i.e. provide a copyBytes() method which copies the bytes in a ByteBuffer and returns a correctly sized array.
> Such a method could be generated for each structure, with the following signature:
> public byte[] copyBytes(ByteBuffer b);
> This method would basically do what the current 'getBin_field' type methods do (allocate a byte array and fill it with the ByteBuffer's data), except it would not modify the structure's internal ByteBuffers.

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