Skip to content

Commit

Permalink
fix: remove client side validations (#868)
Browse files Browse the repository at this point in the history
* fix: remove client side validation hmac/notifications

* remove client validation on storage class values

* revive tests for allowing unspecified states
  • Loading branch information
cojenco committed Oct 6, 2022
1 parent 0efa464 commit 928ebbc
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 34 deletions.
3 changes: 0 additions & 3 deletions google/cloud/storage/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -3719,9 +3719,6 @@ def update_storage_class(
:param retry:
(Optional) How to retry the RPC. See: :ref:`configuring_retries`
"""
if new_class not in self.STORAGE_CLASSES:
raise ValueError(f"Invalid storage class: {new_class}")

# Update current blob's storage class prior to rewrite
self._patch_property("storageClass", new_class)

Expand Down
2 changes: 0 additions & 2 deletions google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -2654,8 +2654,6 @@ def storage_class(self, value):
or
:attr:`~google.cloud.storage.constants.DURABLE_REDUCED_AVAILABILITY_LEGACY_STORAGE_CLASS`,
"""
if value not in self.STORAGE_CLASSES:
raise ValueError(f"Invalid storage class: {value}")
self._patch_property("storageClass", value)

@property
Expand Down
8 changes: 0 additions & 8 deletions google/cloud/storage/hmac_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,6 @@ def state(self):

@state.setter
def state(self, value):
if value not in self._SETTABLE_STATES:
raise ValueError(
f"State may only be set to one of: {', '.join(self._SETTABLE_STATES)}"
)

self._properties["state"] = value

@property
Expand Down Expand Up @@ -289,9 +284,6 @@ def delete(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
:raises :class:`~google.api_core.exceptions.NotFound`:
if the key does not exist on the back-end.
"""
if self.state != self.INACTIVE_STATE:
raise ValueError("Cannot delete key if not in 'INACTIVE' state.")

qs_params = {}
if self.user_project is not None:
qs_params["userProject"] = self.user_project
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/storage/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
:raises ValueError: if the notification has no ID.
"""
if self.notification_id is None:
raise ValueError("Notification not intialized by server")
raise ValueError("Notification ID not set: set an explicit notification_id")

client = self._require_client(client)

Expand Down Expand Up @@ -352,7 +352,7 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
:raises ValueError: if the notification has no ID.
"""
if self.notification_id is None:
raise ValueError("Notification not intialized by server")
raise ValueError("Notification ID not set: set an explicit notification_id")

client = self._require_client(client)

Expand Down Expand Up @@ -395,7 +395,7 @@ def delete(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
:raises ValueError: if the notification has no ID.
"""
if self.notification_id is None:
raise ValueError("Notification not intialized by server")
raise ValueError("Notification ID not set: set an explicit notification_id")

client = self._require_client(client)

Expand Down
43 changes: 32 additions & 11 deletions tests/unit/test_blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -5002,17 +5002,6 @@ def test_rewrite_same_name_w_kms_key_w_version(self):
_target_object=dest,
)

def test_update_storage_class_invalid(self):
blob_name = "blob-name"
bucket = _Bucket()
blob = self._make_one(blob_name, bucket=bucket)
blob.rewrite = mock.Mock(spec=[])

with self.assertRaises(ValueError):
blob.update_storage_class("BOGUS")

blob.rewrite.assert_not_called()

def _update_storage_class_multi_pass_helper(self, **kw):
blob_name = "blob-name"
storage_class = "NEARLINE"
Expand Down Expand Up @@ -5223,6 +5212,38 @@ def test_update_storage_class_single_pass_w_retry(self):
retry = mock.Mock(spec=[])
self._update_storage_class_single_pass_helper(retry=retry)

def test_update_storage_class_invalid(self):
from google.cloud.exceptions import BadRequest

storage_class = "BOGUS"
blob_name = "blob-name"
client = mock.Mock(spec=[])
bucket = _Bucket(client=client)
blob = self._make_one(blob_name, bucket=bucket)
blob.rewrite = mock.Mock(spec=[])
blob.rewrite.side_effect = BadRequest("Invalid storage class")

with self.assertRaises(BadRequest):
blob.update_storage_class(storage_class)

# Test that invalid classes are allowed without client side validation.
# Fall back to server side validation and errors.
self.assertEqual(blob.storage_class, storage_class)

blob.rewrite.assert_called_once_with(
blob,
if_generation_match=None,
if_generation_not_match=None,
if_metageneration_match=None,
if_metageneration_not_match=None,
if_source_generation_match=None,
if_source_generation_not_match=None,
if_source_metageneration_match=None,
if_source_metageneration_not_match=None,
timeout=self._get_default_timeout(),
retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
)

def test_cache_control_getter(self):
BLOB_NAME = "blob-name"
bucket = _Bucket()
Expand Down
10 changes: 7 additions & 3 deletions tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -2813,11 +2813,15 @@ def test_storage_class_getter(self):
self.assertEqual(bucket.storage_class, NEARLINE_STORAGE_CLASS)

def test_storage_class_setter_invalid(self):
invalid_class = "BOGUS"
NAME = "name"
bucket = self._make_one(name=NAME)
with self.assertRaises(ValueError):
bucket.storage_class = "BOGUS"
self.assertFalse("storageClass" in bucket._changes)
bucket.storage_class = invalid_class

# Test that invalid classes are allowed without client side validation.
# Fall back to server side validation and errors.
self.assertEqual(bucket.storage_class, invalid_class)
self.assertTrue("storageClass" in bucket._changes)

def test_storage_class_setter_STANDARD(self):
from google.cloud.storage.constants import STANDARD_STORAGE_CLASS
Expand Down
9 changes: 5 additions & 4 deletions tests/unit/test_hmac_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,12 @@ def test_state_getter(self):
def test_state_setter_invalid_state(self):
metadata = self._make_one()
expected = "INVALID"
metadata.state = expected

with self.assertRaises(ValueError):
metadata.state = expected

self.assertIsNone(metadata.state)
# Test that invalid states are allowed without client side validation.
# Fall back to server side validation and errors.
self.assertEqual(metadata.state, expected)
self.assertEqual(metadata._properties["state"], expected)

def test_state_setter_inactive(self):
metadata = self._make_one()
Expand Down

0 comments on commit 928ebbc

Please sign in to comment.