Expose a TypeMarker factory to wrap an arbitrary Type #2264
Conversation
Generate changelog in
|
| @Override | ||
| public int hashCode() { | ||
| return getType().hashCode(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object other) { | ||
| if (other instanceof WrappingTypeMarker) { | ||
| WrappingTypeMarker otherMarker = (WrappingTypeMarker) other; | ||
| return Objects.equals(getType(), otherMarker.getType()); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Should these be moved to TypeMarker? I don't see any reason why we wouldn't want this implementation for all TypeMarker instances.
I guess maybe it's a little weird to consider different anonymous types equal, but that feel like the correct behavior for TypeMarker.
There was a problem hiding this comment.
I went back and forth on this. We definitely want to allow caches to work correctly for these, where in the usual anonymous class creation path we’d get the same instance and didn’t need to worry about it.
Equals and hashcode are pretty special in that they should take implementation details into account, so I don’t think it would be correct to make them final in the base class.
I think in a follow-up id like to update our caches to create a copy of TypeMarker instances used as keys using TypeMarker.of(marker.getType()) to avoid holding onto encapsulating classes
|
Released 8.13.0 |
This mirrors a change we've just introduced to conjure-undertow here: palantir/conjure-java#2264 Dialogue takes advantage of this new factory method in ConjureBodySerDe to avoid holding class references in serializer caches, which introduced some friction in our dns-polling lifecycle management. I haven't reverted the change to specify `weakKeys` in the caches, however the keys should never be freed unless classes are unloaded (e.g. plugins).
Before this PR
It was impossible to make a reusable serializer/deserializer factory for a generic wrapper type, a new de/serializer would need to be implemented for each component, passing the concrete
TypeMarker. While likely safer, it's not ideal to repeat unnecessary cruft like this.After this PR
==COMMIT_MSG==
Expose a TypeMarker factory to wrap an arbitrary Type
==COMMIT_MSG==
Downsides: This method doesn't provide compile-time safety, as the returned
TypeMarkercannot have generic type info (result isTypeMarker<?>becauseTypeitself doesn't provide type info). However, this is reasonable because it can simplify code used by the annotation processor, and doesn't represent anything more dangerous than cast suppressions folks already use.