search

twitter / scrooge

A Thrift parser/generator
http://twitter.github.io/scrooge/
Apache License 2.0
793 stars 247 forks source link

Add configuration choice of getting an immutable type in code generation for `bytes` thrift type #346

Open rogern opened 3 years ago

rogern commented 3 years ago

Thrift type bytes currently gets generated as a java.nio.ByteBuffer. This type is mutable and if not careful defensive copying is used reading it changes it's internal position in the array. It supports asReadonlyBuffer but that can't be used if the bytes are sent via Thrift to clients, I see a java.nio.ReadOnlyBufferException thrown if done like that.

So to make this less painful I'd like Scrooge to support this type to be an io.twitter.util.Buf via configuration. Probably as the shared version.

Our workaround right now is defensive copying on the byte array itself or directly use list<byte> in Thrift but it feels like a workaround to me but i'm pretty new on this...

mosesn commented 3 years ago

@rogern for the slow response. I think this is a reasonable idea. Something we've discussed internally is being able to annotate fields to influence the way they're generated (eg being able to use specialized classes for certain kinds of maps). That might be a reasonable approach for this kind of patch?

Anyway, I think we would be happy to accept a patch that looks something like this, but we should probably discuss in greater detail what we actually want to do, and at what granularity we want to support it.