[ 
https://issues.apache.org/jira/browse/THRIFT-1035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12980333#action_12980333
 ] 

Mathias Herberts commented on THRIFT-1035:
------------------------------------------

I think developers using Thrift expect a.setBinary_map(mymap) to not alter or 
replace 'mymap' with anything else, at least that's what the set semantics of 
Java beans mean to me, so replacing 'mymap' with another map with the same 
content does not seem to be right as 'mymap' could have special properties 
(could be a ConcurrentHashMap or a fancy collection from 
com.google.common.collect).

And let's consider scenario B:

struct B {
  1: map<string, map<string, binary>> bin_map,
}

B b = new B();

b.setBin_map(new HashMap<String,Map<String,byte[]>>());
b.getBin_map().put("foo", new HashMap<String,byte[]>());

unless you wrap the original Map and catch the call to put there, I don't think 
the approach you mentioned works that easily.

> Container types containing binary data are parameterized with ByteBuffer in 
> the generated Java code
> ---------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-1035
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1035
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Compiler, Java - Library
>    Affects Versions: 0.4, 0.5, 0.6, 0.7
>         Environment: All
>            Reporter: Mathias Herberts
>         Attachments: THRIFT-1035-2.patch, THRIFT-1035.patch
>
>
> Since THRIFT-830, binary fields are internally handled using ByteBuffer.
> Release 0.4.0 was the first to expose the ByteBuffer to the outside world 
> (replacing previous methods returning/accepting byte[]).
> THRIFT-882 lead to the methods accepting/returning byte[] being available 
> again, as it was deemed more reasonable not to expose the ByteBuffer too much 
> as their use could be cumbersome. This lead to 0.5.0 being backward 
> compatible with 0.3.0 on the binary fields front.
> During that time, nobody noticed that container types that contained binary 
> data had their generated Java code changed to collections parameterized with 
> ByteBuffer instead of byte[].
> list<binary> -> List<ByteBuffer>
> set<binary> -> Set<ByteBuffer>
> map<binary,...> -> Map<ByteBuffer,...>
> map<...,binary> -> Map<...,ByteBuffer>
> This introduces confusion in the API and still exposes ByteBuffer when 
> discussion on THRIFT-882 concluded this should be avoided.
> We need to provide a way to offer the original parameterization with byte[] 
> as this will simplify working with that type of collection and thus will 
> increase the odds of Thrift's adoption.

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

Reply via email to