On Thu, Jan 28, 2016 at 11:04 AM, BCG <bgo...@hushmail.com> wrote: > On 01/28/2016 12:06 PM, John Sirois wrote: > >> I've filed an issue that proposes either adding an option to the `java` >> gen >> lang or else adding a new gen kang (ala `javame`) that effects generation >> of immutable thrift structs and unions for java: >> https://issues.apache.org/jira/browse/THRIFT-3583 >> >> I have positive feedback from the Apache Aurora project where I conducted >> a >> full working experiment with the idea ( >> http://markmail.org/message/a6sdqcelgokw6mwz) and some initial positive >> feedback on the thrift ticket. Before proceeding to draw up patches, I'd >> like to vet a few possible paths here and generally gauge the community >> stance. >> >> From what I can tell, there are 3 primary challenges implementing an >> option >> to generate immutable entities - in order from most daunting to least: >> >> 1. The thrift java support library is fundamentally mutable. >> It starts in TBase, where you need an instance before you can `read` it >> from the wire, you can `reset` it and you can `deepCopy`. The latter is >> not problematic, but the former are. It seems like, assuming the >> immutable >> option were to leverage the existing thrift java lib, the TBase interface >> would need to be decomposed into a read interface and a write interface >> and >> then TBase would need to be built back up on top of those to maintain >> backaward compatibility but open a seam to introduce the new object model. >> >> I think this is a sensible approach. > >> 2. The java standard collection library is fundamentally mutable. >> Its true, you can just use Collections.immutable{List,Map,Set} but it >> would >> be nice to communicate immutability in signatures - notably using guava's >> Immutable{List,Map,Set} - as well as leverage the performance >> optimizations >> they've invested so much time in. Using guava though would present a >> jar-hell situtation for thrift's consumers. We could roll our own >> immutable collections - or interfaces - but this presents an impedance >> mismatch problem for those using guava. >> > A possible solution to this would be to have an optional compiler flag for > using Guava collections in the generated code. There are not huge > differences in the APIs (from a code generation standpoint). This same > approach is used for Sorted* collections in the Thrift compiler already. > Users can then be responsible for adding their preferred version of Guava > to their own application themselves (i.e., don't add it as a compile > dependency to libthrift's pom). I'm not familiar with how stable the Guava > API is, but I presume it is fairly easy to generate code that would be > backwards compatible with many older versions of that library.
I think you're right here about guava stability. The APIs my experiment changes linked below use have all been stable as long as I can remember - I think all the way back to before the google-collections -> guava rename. So forcing users to supply the guava dep would probably work just fine. > >> 3. The java standard library for older versions of java (<=1.7) is >> fundamentally nullable. >> It would be wonderful to present the option of representing nullable >> fields >> as Optional. Unfortunately the java stdlib doesn't carry this until >> present - 1.8. Again there is the option of using guava's Optional with >> the associated jar-hell problem. And also the roll our own option with >> the >> same impedance mismatch issues. >> > I have the same feeling as above... I think that this should be up to the > user and controllable via a compiler flag... for instance maybe we could > generate 1.7 compatible code by default, and have flags to select using > Java 8's Optional or Guava's Optional (as long as references to Optional > don't leak into the libthrift JAR, which is current built to run on Java 5: > https://github.com/bgould/thrift/blob/master/lib/java/build.xml#L97 ). > I'd also be in favor of making Java 8 for generated code and having a > "java7" flag similar to the "java5" flag that already exists. Also maybe > I'm missing the point - but is it not possible to have immutable structs > that have nullable fields, or is using Optional required for immutability? > You're correct. We can have immutable structs that have @Nullable fields, I'd just like to modernize the API as a whole is undertaking this endeavor. Although my demonstration series of changes [1][2][3] for Aurora don't use Optional (Aurora is java 1.8 so I could), I think Aurora would likely love to have this - its just a big mechanical change to adapt today. Optional pairs particularly well with java 8 and the with'er UnaryOperator mutator style implemented in the experiment changes below. [1] https://reviews.apache.org/r/42748/ [2] https://reviews.apache.org/r/42749/ [3] https://reviews.apache.org/r/42756/ > > Whatever the solution, I'm happy to help contribute and test this feature > as work progresses. > > $.02 > > Ben > >
