[BEAM-134] Example of AutoValue integration

[swegner]

This is a sample for [BEAM-134] Investigate use of AutoValue

In the case of IsmFormat, there are four inner-classes used as immutable value types which can be simplified with AutoValue. Each of them demonstrate some interesting functionality:

• KeyPrefix is the simplest of the bunch, and it's implementation basically disappears after conversion.
• IsmRecord can act as two different types (value or metadata), and has validation logic in its getters they verify correct usage. In this case, we can keep the existing getter functionality and let AutoValue hook into separate package-private abstract properties. IsmShard is similar in this respect.
• Footer provides has custom logic in its .toString() to include a version string. For other value classes, AutoValue will generate a toString() implementation compatible with the equivalent Objects.toStringHelper version. In this case, we can keep the existing implementation and AutoValue won't override it.

As noted in the JIRA issue, I've identified 39 distinct classes which could be similarly converted. The benefits to converting are:

• Less boilerplate code to maintain.
• Eliminate a common source of bugs.
• Lower the barrier to writing immutable value types, which carry their own benefits.

I recommend we take this work.

[davorbonaci]

This looks nice to me. Some magic, but reduces the amount of code.

Let's leave this pull request open for a week or so. This will give more people a chance to comment on this code pattern.

[dhalperi]

I like that this is not retained at compile time. (No action needed)

[swegner]

Also note: AutoValue is a build-time dependency only. The @AutoValue annotation is marked with @RetentionPolicy.SOURCE, meaning the annotations discarded after compilation. The annotation processor generates package-private classes that should only be used within the implementation of the abstract class. In the pom.xml, the dependency is marked <scope>provided</scope>, meaning the dependency isn't transitive and won't be present at runtime.

[kennknowles]

Love it. I did some brief checking into comparable libraries.

• lombok: mutates the current class to add getters and setters. This is actually forbidden for annotation processors as I understand them, so I guess Lombok uses more fragile hooks to accomplish it. I agree with criticism that this is slightly more "magical", in a bad way.
• immutables: very similar. They emphasize builders, which autovalue can also provide. Immutables works with interfaces, which seems nice.
• FreeBuilder (also originating from Google) is a subset of AutoValue and seems a lower vitality project.

I am torn between my bias towards Google technology and the (admittedly modest) bonus of interface support in Immutables. Immutables also has some built-in support for Java standard lib classes - can you dig into the differences there?

[swegner]

Thanks for surfacing a few more options, @kennknowles. After looking at a few, my preference is for AutoValue. Some notes on each:

AutoValue:

• Most popular (based on GitHub stars; I couldn't find a source of Maven stats)
• Standard annotation processor; fully-supported in Maven, IntelliJ & Eclipse
• Generated code is hidden; no user-visible API
• Very flexible usage patterns

lombok

• Also very popular
• Supports many use-cases other than immutable type-generation. Feels more like adding language features on top of Java, similar to Typescript does for Javascript
• Mutates source classes; as you note, this contradicts the annotation processor spec and may not play nicely with IDEs.
• Generates visible APIs; harder to follow code without access to generated code or knowledge of lombok.

immutables

• Much less popular than AutoValue and lombok
• Most similar to AutoValue, but supports generating interface implementations.
• Much more configuration-driven; lots of knobs to customize generated implementations since they are typically API-visible. As a result, there seems to be much more library-specific code.

FreeBuilder

• Much less popular than AutoValue and lombok
• Strives only to support builder pattern; factory methods not supported.

[kennknowles]

Awesome. Thanks for the details. I'm sold on it. Since @lukecwik is the expert on this code, he should be the one to LGTM this particular use. There are some other places that we may immediately use it, such as a follow-up to #120.

[lukecwik]

We could make version an actual field, its not a big deal to store the extra byte for each footer.

Then this would remove even more code from here.

[swegner]

I've addressed all feedback so far. Please take another look. @lukecwik

[lukecwik]

Based upon http://mvnrepository.com/, this is the popularity of those projects as dependencies of other projects:
autovalue: 107 usages
lombok: 924 usages
immutables: 29 usages
FreeBuilder: 4 usages

[lukecwik]

LGTM

Please fix conflict and then I can merge.

[swegner]

Rebased. Should be good-to-go. @lukecwik

[swegner]

@davorbonaci Should AutoValue get backported to Dataflow SDK?

Reasons NOT TO backport: It's not new functionality.
Reasons TO backport: Other functionality we backport may depend on it. This is already the case with DisplayData.

I'm leaning towards not backporting. It should be easy to re-implement the parts of DisplayData which depend on it. /cc @bjchambers

[davorbonaci]

I think the best choice is to back-port on a need basis, i.e., if and when another PR needs it.