fix: Remove all client side validation for OLM, allow nonspecific lif…#1160
fix: Remove all client side validation for OLM, allow nonspecific lif…#1160JesseLovelace merged 7 commits intomainfrom
Conversation
| * expressed as subclasses of this class, accessed by static factory methods. | ||
| */ | ||
| public abstract static class LifecycleAction implements Serializable { | ||
| public static class LifecycleAction implements Serializable { |
There was a problem hiding this comment.
Do we need to remove the modifier abstract in this case?
There was a problem hiding this comment.
Abstract classes can't be instantiated so we do if we want to allow for unsupported action types to be created in this way.
The alternative would be to make an implementation of LifecycleAction for that purpose, like "CustomLifecycleAction" or something, but I'm worried that doing that would be confusing, or imply that this is the correct way to use action types that aren't in the library (when in actuality users should upgrade to the latest version of the library)
There was a problem hiding this comment.
Ah, totally missed that. Thanks Jesse, I would confirm what happens when a say setStorageClass doesn't have storageClass in the rule (the case an action type is added that requires additional parameters) and see how GCS handles the request on a PATCH request.
I suspect it will fail; so we may want to clarify if we want the library not to create an instance for that unknown action type.
| default: | ||
| throw new UnsupportedOperationException( | ||
| "The specified lifecycle action " + action.getType() + " is not currently supported"); | ||
| log.warning( |
There was a problem hiding this comment.
Can we add tests to ensure unknown types no longer throw exceptions?
google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
Outdated
Show resolved
Hide resolved
|
@JesseLovelace are you still addressing the clirr issue? |
| public abstract String getActionType(); | ||
| private final String actionType; | ||
|
|
||
| private LifecycleAction(String actionType) { |
There was a problem hiding this comment.
Let's make this constructor public to allow someone (as remote a chance as it is) that is already extending themselves allow them to continue to extend, but now they would need to provide the actionType.
|
/cherry-pick v2.1.9 |
|
/cherry-pick 2.1.x |
#1160) * fix: Remove all client side validation for OLM, allow nonspecific lifecycle actions * Test unsupported actions * address comments * Fix clirr * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * checkstyle Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* fix: Remove all client side validation for OLM, allow nonspecific lifecycle actions * Test unsupported actions * address comments * Fix clirr * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * checkstyle Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: JesseLovelace <43148100+JesseLovelace@users.noreply.github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Removes client side validation by refactoring
LifecycleActioninto a non-abstract class and allowing for nonspecific lifecycle actions.