[THRIFT-1039] Java codegen makes byte[] get/set for binary field, List<ByteBuffer> get/set for list<binary> field

• Type: Improvement
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Component/s: Java - Compiler
• Labels: Breaking-Change

Environment

All

• Assignee: Unassigned
• Reporter: Mathias Herberts
• Created: 17/Jan/11 21:11
• Updated: 03/Jan/19 12:57

Description

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.