Add UUID to parsing for matching elements across instances

[ZacSweers]

This is a prototype of trying to tag parsed elements with unique IDs so that one could match them even if you newBuilder'd it, you could match back other elements that reference the now-old reference to them. This would allow for one to normalize references in a Schema after changing a bunch of elements in it, such as with a preprocessor to edit Schemas before code gen.

If the implementation here looks good, I can proceed with figuring out how to update tests to handle this in a deterministic way

[ZacSweers]

@benjamin-bader absolutely no CR expected for at least 3-4 weeks! Focus on your honeymoon :)

[benjamin-bader]

Hm, the tests failed, but it's not immediately clear why. I'm guessing that the tests haven't been updated to account for the new UUID member, probably the AutoValue parser elements that would automatically include it in .equals().

[ZacSweers]

Yeah I haven't updated the tests yet, wanted to get a review of the API first before taking a crack at them

[benjamin-bader]

API looks fine to me; it's what I'd do if I were implementing the feature, so no complaints here.

[ZacSweers]

Cool, I'll work on updating the tests then. My initial thinking is to basically add some way to set a seed to use for uuids for test-only purposes. Alternatively, we could make mark the uuid as transient to autovalue somehow so it's ignored, but not sure how nicely that plays with builders and whatnot. Will try some stuff

[codecov-io]

Codecov Report

Merging #152 into master will decrease coverage by 0.04%.
The diff coverage is 71.69%.

@@             Coverage Diff              @@
##             master     #152      +/-   ##
============================================
- Coverage     65.45%   65.41%   -0.05%     
- Complexity      931      936       +5     
============================================
  Files            88       89       +1     
  Lines          4806     4843      +37     
  Branches        561      562       +1     
============================================
+ Hits           3146     3168      +22     
- Misses         1423     1437      +14     
- Partials        237      238       +1

Impacted Files	Coverage Δ	Complexity Δ
...java/com/microsoft/thrifty/schema/UserElement.java	50% <ø> (ø)	3 <0> (ø)	⬇️
...in/java/com/microsoft/thrifty/schema/UserType.java	83.33% <0%> (-2.88%)	12 <0> (ø)
.../main/java/com/microsoft/thrifty/schema/Field.java	93.18% <0%> (-2.17%)	22 <0> (ø)
...va/com/microsoft/thrifty/schema/ServiceMethod.java	56.96% <0%> (-0.74%)	20 <0> (ø)
.../java/com/microsoft/thrifty/schema/EnumMember.java	40% <0%> (-1.67%)	7 <0> (ø)
...m/microsoft/thrifty/schema/parser/EnumElement.java	100% <100%> (ø)	2 <0> (ø)	⬇️
...osoft/thrifty/schema/parser/EnumMemberElement.java	100% <100%> (ø)	2 <0> (ø)	⬇️
...crosoft/thrifty/schema/parser/FunctionElement.java	88.88% <100%> (+1.38%)	2 <0> (ø)	⬇️
...microsoft/thrifty/schema/parser/StructElement.java	100% <100%> (ø)	2 <0> (ø)	⬇️
.../microsoft/thrifty/schema/parser/FieldElement.java	66.66% <100%> (+4.16%)	2 <0> (ø)	⬇️
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85ff3c7...08f85ce. Read the comment docs.

[benjamin-bader]

Interesting, I must have missed this class the last time I looked at this PR. Why would you need something other than UUID::randomUUID - for testing, it seems?

If so, and there aren't any larger plans for this class, have you considered simply adding a UUIDProvider param to a ThriftParser constructor, and making the parser itself responsible for assigning the UUIDs? It would mean also changing the autovalue factory functions for the parser elements, to add a UUID, but would have the benefit of being less stateful.

I think you might have deliberately decided against doing this, so I'm curious about why.

[ZacSweers]

I added it in this followup. The primary case is testing, but one could conceivably also use this to supply their own no-op version if they don't need it and want the slight perf boost from not calculating a random one.

I opted to make it a plugin setup because I wanted to have a simple static location for both simplicity in the parser's access as well as making it public for anyone creating/modifying their own types to use if they create their own (to keep things consistent). This is borrowed largely from RxJava's style of plugins

[benjamin-bader]

Thanks! For the PR, and for your patience seeing it through.

[ZacSweers]

Thanks!