Re-remove dependency on Guava #88
Conversation
* Use `copyOf` and `unmoidifiableSet` functions to replace `ImmutableSet` * Roll our own memoizing supplier
|
Maybe ask Lucas to review? |
| @@ -34,7 +32,15 @@ public final class EntityTypeName { | |||
| protected EntityTypeName(List<String> namespace, String basename) { | |||
| this.namespace = namespace; | |||
| this.basename= basename; | |||
| this.entityTypeNameRepr = Suppliers.memoize(() -> getEntityTypeNameRepr(this)); | |||
| this.entityTypeNameRepr = new Supplier<String>() { | |||
There was a problem hiding this comment.
If anyone is wondering why this is stalled: The Guava memoizing supplier is thread safe, but this implementation would not be. We don't make an explicit guarantees about thread safety of the Java bindings, but out Rust implementation is thread safe, so I don't want to break that here, and I haven't had the time to do this properly.
There was a problem hiding this comment.
Thanks for noting the current status @john-h-kastner-aws.
Reviewing the content of this class, is the purpose of using a Supplier to defer to the native JNI call until it is actually needed? Since instances of this class are effectively immutable, it seems like calling getEntityTypeNameRepr() multiple times is not necessarily a problem for the Java toString() representation, unless there is a state concern on the Rust side.
|
@john-h-kastner-aws Raising the question, is this change planned for reconsideration at some point? It would be helpful to remove the 3 MB dependency on Google Guava for those integrating with the Cedar Java library. |
Requires the following changes:
Use
copyOfandunmodifiableSetfunctions to replaceImmutableSet. Prefer usingcopyOfbecause it creates an unmodifiable copy of the set or list, so mutation to the input does not reflect in the new object.unmodifiableSetcan still be used to return an immutable view of a set without causing any copying.Roll our own memoizing supplier. This change should be reviewed by someone with more Java knowledge, but, AFAIK, the behavior should be the same.
Issue #, if available:
#86
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.