chore(test): expand StorageInstance vetoing to apply to bucket acl operations #1819
Conversation
Rather than using a runtime proxy to do invocation method argument matching we are now defining a concrete class to override the methods it needs to provide enforcement for. This is simpler in that it uses standard java language implementation approach which is helped by compiler and IDEs. It is at the expense of needing to define the new ~500 line AbstractStorageDecorator class.
product-auto-label
bot
added
size: l
| } | ||
|
|
||
| @Override | ||
| public boolean deleteAcl(String bucket, Entity entity) { |
There was a problem hiding this comment.
By having this protection, I'm concerned about future changes not being caught by this mechanism. However, IIUC, whenever new methods are added to the Storage interface, the new methods will need to be implemented in the Vetoing class as well so it covers this case.
There was a problem hiding this comment.
Correct, by explicitly defining it as an implementation of the interface we will have to implement the method if a new one is added, whereas the runtime generated proxy before was using reflection to selectively choose which methods to augment.
| throw e.getCause(); | ||
| } | ||
| })); | ||
| proxy = new VetoingStorageProxy(); |
There was a problem hiding this comment.
Side bar:
I'm wondering if you're planning on exposing AbstractStorageProxy as a builder option that can be passed and potentially used for other efforts (thinking observability)?
There was a problem hiding this comment.
At this point, no. I think observability stuff we're going to want/need to be more explicit and prescriptive about any extension points we provide. That said, for our internal implementation we could so something similar to allow for a single implementation that would wrap both http and grpc Stoage.
This also changes the JDK distribution from zulu to temurin https://github.com/actions/setup-java#eclipse-temurin Source-Link: googleapis/synthtool@ef9fe2e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:31c8276a1bfb43766597d32645721c029cb94571f1b8d996cb2c290744fe52f9
* ci: javadoc job (JDK 17) in ci.yaml (#1819) This also changes the JDK distribution from zulu to temurin https://github.com/actions/setup-java#eclipse-temurin Source-Link: googleapis/synthtool@ef9fe2e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:31c8276a1bfb43766597d32645721c029cb94571f1b8d996cb2c290744fe52f9 * javadocs: cleanup for java 17 doclint errors * remove rel from `a` tags * add caption to table * close dangling tags --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: BenWhitehead <BenWhitehead@users.noreply.github.com> Co-authored-by: Tomo Suzuki <suztomo@google.com>
2 participants
Simplify StorageInstance vetoing implementation. Rather than using a runtime proxy to do invocation method argument matching we are now defining a concrete class to override the methods it needs to provide enforcement for.
This is simpler in that it uses standard java language implementation approach which is helped by compiler and IDEs. It is at the expense of needing to define the new ~500 line AbstractStorageDecorator class.
Delete several obsolete tests, which have been replace by other tests or do not make sense to be written the way they are.
Cleanup of TODO from #1785