From 3fa6245c987dc70c0c9adc9004d4614d03c5de07 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 May 2020 12:19:43 +0300 Subject: [PATCH 01/15] feat: add ifMetageneration*Match support, pt1 --- google/cloud/storage/_helpers.py | 143 +++++++++++++++++++++++++++++-- google/cloud/storage/blob.py | 22 +---- google/cloud/storage/bucket.py | 103 +++++++++++++++++++--- google/cloud/storage/client.py | 54 ++++++++++-- tests/unit/test__helpers.py | 94 ++++++++++++++++++++ tests/unit/test_bucket.py | 59 +++++++++++++ tests/unit/test_client.py | 72 ++++++++++++++++ 7 files changed, 505 insertions(+), 42 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index b649384f7..f4aa8643f 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -30,6 +30,14 @@ _DEFAULT_STORAGE_HOST = u"https://storage.googleapis.com" +# generation match parameters in camel and snake cases +_GENERATION_MATCH_PARAMETERS = ( + ("if_generation_match", "ifGenerationMatch"), + ("if_generation_not_match", "ifGenerationNotMatch"), + ("if_metageneration_match", "ifMetagenerationMatch"), + ("if_metageneration_not_match", "ifMetagenerationNotMatch"), +) + def _get_storage_host(): return os.environ.get(STORAGE_EMULATOR_ENV_VAR, _DEFAULT_STORAGE_HOST) @@ -121,27 +129,54 @@ def _query_params(self): params["userProject"] = self.user_project return params - def reload(self, client=None, timeout=_DEFAULT_TIMEOUT): + def reload( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Reload properties from Cloud Storage. If :attr:`user_project` is set, bills the API request to that project. :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: the client to use. If not passed, falls back to the + :param client: the client to use. If not passed, falls back to the ``client`` stored on the current object. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) query_params = self._query_params # Pass only '?projection=noAcl' here because 'acl' and related # are handled via custom endpoints. query_params["projection"] = "noAcl" + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) api_response = client._connection.api_request( method="GET", path=self.path, @@ -180,7 +215,13 @@ def _set_properties(self, value): # If the values are reset, the changes must as well. self._changes = set() - def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): + def patch( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Sends all changed properties in a PATCH request. Updates the ``_properties`` with the response from the backend. @@ -189,20 +230,41 @@ def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: the client to use. If not passed, falls back to the + :param client: the client to use. If not passed, falls back to the ``client`` stored on the current object. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) query_params = self._query_params # Pass '?projection=full' here because 'PATCH' documented not # to work properly w/ 'noAcl'. query_params["projection"] = "full" + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) update_properties = {key: self._properties[key] for key in self._changes} # Make the API call. @@ -216,7 +278,13 @@ def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): ) self._set_properties(api_response) - def update(self, client=None, timeout=_DEFAULT_TIMEOUT): + def update( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Sends all properties in a PUT request. Updates the ``_properties`` with the response from the backend. @@ -225,18 +293,40 @@ def update(self, client=None, timeout=_DEFAULT_TIMEOUT): :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: the client to use. If not passed, falls back to the + :param client: the client to use. If not passed, falls back to the ``client`` stored on the current object. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) + query_params = self._query_params query_params["projection"] = "full" + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) api_response = client._connection.api_request( method="PUT", path=self.path, @@ -312,3 +402,44 @@ def _convert_to_timestamp(value): utc_naive = value.replace(tzinfo=None) - value.utcoffset() mtime = (utc_naive - datetime(1970, 1, 1)).total_seconds() return mtime + + +def _add_generation_match_parameters(name_value_pairs, **parameters): + """Add generation match parameters into the given parameters list. + + :type name_value_pairs: list or dict + :param name_value_pairs: Parameters list or dict. + + :type parameters: dict + :param parameters: if*generation*match parameters to add. + """ + for snakecase_name, camelcase_name in _GENERATION_MATCH_PARAMETERS: + value = parameters.get(snakecase_name) + + if value is not None: + if isinstance(name_value_pairs, list): + name_value_pairs.append((camelcase_name, value)) + + elif isinstance(name_value_pairs, dict): + name_value_pairs[camelcase_name] = value + + +def _raise_for_more_than_one_none(**kwargs): + """Raise ``ValueError`` exception if more than one parameter was set. + + :type error: :exc:`ValueError` + :param error: Description of which fields were set + + :raises: :class:`~ValueError` containing the fields that were set + """ + if sum(arg is not None for arg in kwargs.values()) > 1: + escaped_keys = ["'%s'" % name for name in kwargs.keys()] + + keys_but_last = ", ".join(escaped_keys[:-1]) + last_key = escaped_keys[-1] + + msg = "Pass at most one of {keys_but_last} and {last_key}".format( + keys_but_last=keys_but_last, last_key=last_key + ) + + raise ValueError(msg) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index d3416616b..bce83a2b1 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -57,6 +57,7 @@ from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _convert_to_timestamp +from google.cloud.storage._helpers import _raise_for_more_than_one_none from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import ACL @@ -2639,24 +2640,3 @@ def _add_query_parameters(base_url, name_value_pairs): query = parse_qsl(query) query.extend(name_value_pairs) return urlunsplit((scheme, netloc, path, urlencode(query), frag)) - - -def _raise_for_more_than_one_none(**kwargs): - """Raise ``ValueError`` exception if more than one parameter was set. - - :type error: :exc:`ValueError` - :param error: Description of which fields were set - - :raises: :class:`~ValueError` containing the fields that were set - """ - if sum(arg is not None for arg in kwargs.values()) > 1: - escaped_keys = ["'%s'" % name for name in kwargs.keys()] - - keys_but_last = ", ".join(escaped_keys[:-1]) - last_key = escaped_keys[-1] - - msg = "Pass at most one of {keys_but_last} and {last_key}".format( - keys_but_last=keys_but_last, last_key=last_key - ) - - raise ValueError(msg) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 69fa321bb..9d4c8f24d 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -32,7 +32,9 @@ from google.cloud.exceptions import NotFound from google.api_core.iam import Policy from google.cloud.storage import _signing +from google.cloud.storage._helpers import _add_generation_match_parameters from google.cloud.storage._helpers import _PropertyMixin +from google.cloud.storage._helpers import _raise_for_more_than_one_none from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _validate_name from google.cloud.storage._signing import generate_signed_url_v2 @@ -646,15 +648,22 @@ def notification( notification_id=notification_id, ) - def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): + def exists( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Determines whether or not this bucket exists. If :attr:`user_project` is set, bills the API request to that project. :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. @@ -662,9 +671,24 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :rtype: bool :returns: True if the bucket exists in Cloud Storage. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) # We only need the status code (200 or not) so we seek to # minimize the returned payload. @@ -673,6 +697,11 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): if self.user_project is not None: query_params["userProject"] = self.user_project + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) try: # We intentionally pass `_target_object=None` since fields=name # would limit the local properties. @@ -762,7 +791,13 @@ def create( timeout=timeout, ) - def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): + def patch( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Sends all changed properties in a PATCH request. Updates the ``_properties`` with the response from the backend. @@ -771,15 +806,31 @@ def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: the client to use. If not passed, falls back to the + :param client: the client to use. If not passed, falls back to the ``client`` stored on the current object. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) # Special case: For buckets, it is possible that labels are being # removed; this requires special handling. if self._label_removals: @@ -789,7 +840,12 @@ def patch(self, client=None, timeout=_DEFAULT_TIMEOUT): self._properties["labels"][removed_label] = None # Call the superclass method. - return super(Bucket, self).patch(client=client, timeout=timeout) + return super(Bucket, self).patch( + client=client, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) @property def acl(self): @@ -1065,7 +1121,14 @@ def get_notification(self, notification_id, client=None, timeout=_DEFAULT_TIMEOU notification.reload(client=client, timeout=timeout) return notification - def delete(self, force=False, client=None, timeout=_DEFAULT_TIMEOUT): + def delete( + self, + force=False, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Delete this bucket. The bucket **must** be empty in order to submit a delete request. If @@ -1073,9 +1136,8 @@ def delete(self, force=False, client=None, timeout=_DEFAULT_TIMEOUT): objects / blobs in the bucket (i.e. try to empty the bucket). If the bucket doesn't exist, this will raise - :class:`google.cloud.exceptions.NotFound`. If the bucket is not empty - (and ``force=False``), will raise - :class:`google.cloud.exceptions.Conflict`. + :class:`google.cloud.exceptions.NotFound`. If the bucket is not empty + (and ``force=False``), will raise :class:`google.cloud.exceptions.Conflict`. If ``force=True`` and the bucket contains more than 256 objects / blobs this will cowardly refuse to delete the objects (or the bucket). This @@ -1089,8 +1151,9 @@ def delete(self, force=False, client=None, timeout=_DEFAULT_TIMEOUT): :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response on each request. @@ -1098,15 +1161,35 @@ def delete(self, force=False, client=None, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :raises: :class:`ValueError` if ``force`` is ``True`` and the bucket contains more than 256 objects / blobs. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) query_params = {} if self.user_project is not None: query_params["userProject"] = self.user_project + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) if force: blobs = list( self.list_blobs( diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index 1a7711552..dabaffb1d 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -281,7 +281,13 @@ def batch(self): """ return Batch(client=self) - def get_bucket(self, bucket_or_name, timeout=_DEFAULT_TIMEOUT): + def get_bucket( + self, + bucket_or_name, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """API call: retrieve a bucket via a GET request. See @@ -300,6 +306,14 @@ def get_bucket(self, bucket_or_name, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + if_metageneration_match (Optional[long]): + Make the operation conditional on whether the + blob's current metageneration matches the given value. + + if_metageneration_not_match (Optional[long]): + Make the operation conditional on whether the blob's + current metageneration does not match the given value. + Returns: google.cloud.storage.bucket.Bucket The bucket matching the name provided. @@ -308,6 +322,10 @@ def get_bucket(self, bucket_or_name, timeout=_DEFAULT_TIMEOUT): google.cloud.exceptions.NotFound If the bucket is not found. + ValueError + If `if_metageneration_match` and + `if_metageneration_not_match` are both set. + Examples: Retrieve a bucket using a string. @@ -329,11 +347,21 @@ def get_bucket(self, bucket_or_name, timeout=_DEFAULT_TIMEOUT): """ bucket = self._bucket_arg_to_bucket(bucket_or_name) - - bucket.reload(client=self, timeout=timeout) + bucket.reload( + client=self, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) return bucket - def lookup_bucket(self, bucket_name, timeout=_DEFAULT_TIMEOUT): + def lookup_bucket( + self, + bucket_name, + timeout=_DEFAULT_TIMEOUT, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Get a bucket by name, returning None if not found. You can use this if you would rather check for a None value @@ -353,11 +381,27 @@ def lookup_bucket(self, bucket_name, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :rtype: :class:`google.cloud.storage.bucket.Bucket` :returns: The bucket matching the name provided or None if not found. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match`` are both set. """ try: - return self.get_bucket(bucket_name, timeout=timeout) + return self.get_bucket( + bucket_name, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) except NotFound: return None diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 10b71b7bc..1a9242d8f 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -126,6 +126,37 @@ def test_reload(self): ) self.assertEqual(derived._changes, set()) + def test_reload_with_metageneration_match(self): + METAGENERATION_NUMBER = 6 + + connection = _Connection({"foo": "Foo"}) + client = _Client(connection) + derived = self._derivedClass("/path")() + # Make sure changes is not a set instance before calling reload + # (which will clear / replace it with an empty set), checked below. + derived._changes = object() + derived.reload( + client=client, timeout=42, if_metageneration_match=METAGENERATION_NUMBER + ) + self.assertEqual(derived._properties, {"foo": "Foo"}) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual( + kw[0], + { + "method": "GET", + "path": "/path", + "query_params": { + "projection": "noAcl", + "ifMetagenerationMatch": METAGENERATION_NUMBER, + }, + "headers": {}, + "_target_object": derived, + "timeout": 42, + }, + ) + self.assertEqual(derived._changes, set()) + def test_reload_w_user_project(self): user_project = "user-project-123" connection = _Connection({"foo": "Foo"}) @@ -191,6 +222,41 @@ def test_patch(self): # Make sure changes get reset by patch(). self.assertEqual(derived._changes, set()) + def test_patch_with_metageneration_match(self): + METAGENERATION_NUMBER = 6 + + connection = _Connection({"foo": "Foo"}) + client = _Client(connection) + derived = self._derivedClass("/path")() + # Make sure changes is non-empty, so we can observe a change. + BAR = object() + BAZ = object() + derived._properties = {"bar": BAR, "baz": BAZ} + derived._changes = set(["bar"]) # Ignore baz. + derived.patch( + client=client, timeout=42, if_metageneration_match=METAGENERATION_NUMBER + ) + self.assertEqual(derived._properties, {"foo": "Foo"}) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual( + kw[0], + { + "method": "PATCH", + "path": "/path", + "query_params": { + "projection": "full", + "ifMetagenerationMatch": METAGENERATION_NUMBER, + }, + # Since changes does not include `baz`, we don't see it sent. + "data": {"bar": BAR}, + "_target_object": derived, + "timeout": 42, + }, + ) + # Make sure changes get reset by patch(). + self.assertEqual(derived._changes, set()) + def test_patch_w_user_project(self): user_project = "user-project-123" connection = _Connection({"foo": "Foo"}) @@ -241,6 +307,34 @@ def test_update(self): # Make sure changes get reset by patch(). self.assertEqual(derived._changes, set()) + def test_update_with_metageneration_not_match(self): + GENERATION_NUMBER = 6 + + connection = _Connection({"foo": "Foo"}) + client = _Client(connection) + derived = self._derivedClass("/path")() + # Make sure changes is non-empty, so we can observe a change. + BAR = object() + BAZ = object() + derived._properties = {"bar": BAR, "baz": BAZ} + derived._changes = set(["bar"]) # Update sends 'baz' anyway. + derived.update( + client=client, timeout=42, if_metageneration_not_match=GENERATION_NUMBER + ) + self.assertEqual(derived._properties, {"foo": "Foo"}) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]["method"], "PUT") + self.assertEqual(kw[0]["path"], "/path") + self.assertEqual( + kw[0]["query_params"], + {"projection": "full", "ifMetagenerationNotMatch": GENERATION_NUMBER}, + ) + self.assertEqual(kw[0]["data"], {"bar": BAR, "baz": BAZ}) + self.assertEqual(kw[0]["timeout"], 42) + # Make sure changes get reset by patch(). + self.assertEqual(derived._changes, set()) + def test_update_w_user_project(self): user_project = "user-project-123" connection = _Connection({"foo": "Foo"}) diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 365e1f0e1..416e53d49 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -588,6 +588,40 @@ def api_request(cls, *args, **kwargs): expected_cw = [((), expected_called_kwargs)] self.assertEqual(_FakeConnection._called_with, expected_cw) + def test_exists_with_metageneration_match(self): + class _FakeConnection(object): + + _called_with = [] + + @classmethod + def api_request(cls, *args, **kwargs): + cls._called_with.append((args, kwargs)) + # exists() does not use the return value + return object() + + BUCKET_NAME = "bucket-name" + METAGENERATION_NUMBER = 6 + + bucket = self._make_one(name=BUCKET_NAME) + client = _Client(_FakeConnection) + self.assertTrue( + bucket.exists( + client=client, timeout=42, if_metageneration_match=METAGENERATION_NUMBER + ) + ) + expected_called_kwargs = { + "method": "GET", + "path": bucket.path, + "query_params": { + "fields": "name", + "ifMetagenerationMatch": METAGENERATION_NUMBER, + }, + "_target_object": None, + "timeout": 42, + } + expected_cw = [((), expected_called_kwargs)] + self.assertEqual(_FakeConnection._called_with, expected_cw) + def test_exists_hit_w_user_project(self): USER_PROJECT = "user-project-123" @@ -928,6 +962,31 @@ def test_delete_force_delete_blobs(self): ] self.assertEqual(connection._deleted_buckets, expected_cw) + def test_delete_with_metageneration_match(self): + NAME = "name" + BLOB_NAME1 = "blob-name1" + BLOB_NAME2 = "blob-name2" + GET_BLOBS_RESP = {"items": [{"name": BLOB_NAME1}, {"name": BLOB_NAME2}]} + DELETE_BLOB1_RESP = DELETE_BLOB2_RESP = {} + METAGENERATION_NUMBER = 6 + + connection = _Connection(GET_BLOBS_RESP, DELETE_BLOB1_RESP, DELETE_BLOB2_RESP) + connection._delete_bucket = True + client = _Client(connection) + bucket = self._make_one(client=client, name=NAME) + result = bucket.delete(if_metageneration_match=METAGENERATION_NUMBER) + self.assertIsNone(result) + expected_cw = [ + { + "method": "DELETE", + "path": bucket.path, + "query_params": {"ifMetagenerationMatch": METAGENERATION_NUMBER}, + "_target_object": None, + "timeout": self._get_default_timeout(), + } + ] + self.assertEqual(connection._deleted_buckets, expected_cw) + def test_delete_force_miss_blobs(self): NAME = "name" BLOB_NAME = "blob-name1" diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 7acba35fa..5a443573f 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -436,6 +436,42 @@ def test_get_bucket_with_string_hit(self): timeout=self._get_default_timeout(), ) + def test_get_bucket_with_metageneration_match(self): + from google.cloud.storage.bucket import Bucket + + PROJECT = "PROJECT" + CREDENTIALS = _make_credentials() + METAGENERATION_NUMBER = 6 + client = self._make_one(project=PROJECT, credentials=CREDENTIALS) + + BUCKET_NAME = "bucket-name" + URI = "/".join( + [ + client._connection.API_BASE_URL, + "storage", + client._connection.API_VERSION, + "b", + "%s?projection=noAcl&ifMetagenerationMatch=%s" + % (BUCKET_NAME, METAGENERATION_NUMBER), + ] + ) + data = {"name": BUCKET_NAME} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http + + bucket = client.get_bucket( + BUCKET_NAME, if_metageneration_match=METAGENERATION_NUMBER + ) + self.assertIsInstance(bucket, Bucket) + self.assertEqual(bucket.name, BUCKET_NAME) + http.request.assert_called_once_with( + method="GET", + url=URI, + data=mock.ANY, + headers=mock.ANY, + timeout=self._get_default_timeout(), + ) + def test_get_bucket_with_object_miss(self): from google.cloud.exceptions import NotFound from google.cloud.storage.bucket import Bucket @@ -566,6 +602,42 @@ def test_lookup_bucket_hit(self): timeout=self._get_default_timeout(), ) + def test_lookup_bucket_with_metageneration_match(self): + from google.cloud.storage.bucket import Bucket + + PROJECT = "PROJECT" + CREDENTIALS = _make_credentials() + METAGENERATION_NUMBER = 6 + client = self._make_one(project=PROJECT, credentials=CREDENTIALS) + + BUCKET_NAME = "bucket-name" + URI = "/".join( + [ + client._connection.API_BASE_URL, + "storage", + client._connection.API_VERSION, + "b", + "%s?projection=noAcl&ifMetagenerationMatch=%s" + % (BUCKET_NAME, METAGENERATION_NUMBER), + ] + ) + data = {"name": BUCKET_NAME} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http + + bucket = client.lookup_bucket( + BUCKET_NAME, if_metageneration_match=METAGENERATION_NUMBER + ) + self.assertIsInstance(bucket, Bucket) + self.assertEqual(bucket.name, BUCKET_NAME) + http.request.assert_called_once_with( + method="GET", + url=URI, + data=mock.ANY, + headers=mock.ANY, + timeout=self._get_default_timeout(), + ) + def test_create_bucket_w_missing_client_project(self): credentials = _make_credentials() client = self._make_one(project=None, credentials=credentials) From 4cbad43a685c5c2ef4418752b3a5c4ea781f9e38 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 May 2020 13:05:08 +0300 Subject: [PATCH 02/15] fix unit tests, add test for helper --- google/cloud/storage/_helpers.py | 20 +++++++-------- tests/unit/test__helpers.py | 42 ++++++++++++++++++++++++++++++++ tests/unit/test_client.py | 16 ++++++++++-- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index f4aa8643f..d02872eac 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -404,24 +404,24 @@ def _convert_to_timestamp(value): return mtime -def _add_generation_match_parameters(name_value_pairs, **parameters): +def _add_generation_match_parameters(parameters, **match_parameters): """Add generation match parameters into the given parameters list. - :type name_value_pairs: list or dict - :param name_value_pairs: Parameters list or dict. + :type parameters: list or dict + :param parameters: Parameters list or dict. - :type parameters: dict - :param parameters: if*generation*match parameters to add. + :type match_parameters: dict + :param match_parameters: if*generation*match parameters to add. """ for snakecase_name, camelcase_name in _GENERATION_MATCH_PARAMETERS: - value = parameters.get(snakecase_name) + value = match_parameters.get(snakecase_name) if value is not None: - if isinstance(name_value_pairs, list): - name_value_pairs.append((camelcase_name, value)) + if isinstance(parameters, list): + parameters.append((camelcase_name, value)) - elif isinstance(name_value_pairs, dict): - name_value_pairs[camelcase_name] = value + elif isinstance(parameters, dict): + parameters[camelcase_name] = value def _raise_for_more_than_one_none(**kwargs): diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 1a9242d8f..a61a67f75 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -437,6 +437,48 @@ def read(self, block_size): self.assertEqual(MD5.hash_obj._blocks, [BYTES_TO_SIGN]) +class Test__add_generation_match_parameters(unittest.TestCase): + def _call_fut(self, params, **match_params): + from google.cloud.storage._helpers import _add_generation_match_parameters + + return _add_generation_match_parameters(params, **match_params) + + def test_add_generation_match_parameters_list(self): + GENERATION_NUMBER = 9 + METAGENERATION_NUMBER = 6 + EXPECTED_PARAMS = [ + ("param1", "value1"), + ("param2", "value2"), + ("ifGenerationMatch", GENERATION_NUMBER), + ("ifMetagenerationMatch", METAGENERATION_NUMBER), + ] + params = [("param1", "value1"), ("param2", "value2")] + self._call_fut( + params, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, + ) + self.assertEqual(params, EXPECTED_PARAMS) + + def test_add_generation_match_parameters_dict(self): + GENERATION_NUMBER = 9 + METAGENERATION_NUMBER = 6 + EXPECTED_PARAMS = { + "param1": "value1", + "param2": "value2", + "ifGenerationMatch": GENERATION_NUMBER, + "ifMetagenerationMatch": METAGENERATION_NUMBER, + } + + params = {"param1": "value1", "param2": "value2"} + self._call_fut( + params, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, + ) + self.assertEqual(params, EXPECTED_PARAMS) + + class _Connection(object): def __init__(self, *responses): self._responses = responses diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 5a443573f..45354768e 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -445,7 +445,7 @@ def test_get_bucket_with_metageneration_match(self): client = self._make_one(project=PROJECT, credentials=CREDENTIALS) BUCKET_NAME = "bucket-name" - URI = "/".join( + URI1 = "/".join( [ client._connection.API_BASE_URL, "storage", @@ -455,6 +455,16 @@ def test_get_bucket_with_metageneration_match(self): % (BUCKET_NAME, METAGENERATION_NUMBER), ] ) + URI2 = "/".join( + [ + client._connection.API_BASE_URL, + "storage", + client._connection.API_VERSION, + "b", + "%s?ifMetagenerationMatch=%s&projection=noAcl" + % (BUCKET_NAME, METAGENERATION_NUMBER), + ] + ) data = {"name": BUCKET_NAME} http = _make_requests_session([_make_json_response(data)]) client._http_internal = http @@ -466,11 +476,13 @@ def test_get_bucket_with_metageneration_match(self): self.assertEqual(bucket.name, BUCKET_NAME) http.request.assert_called_once_with( method="GET", - url=URI, + url=mock.ANY, data=mock.ANY, headers=mock.ANY, timeout=self._get_default_timeout(), ) + _, kwargs = http.request.call_args + self.assertIn(kwargs.get("url"), (URI1, URI2)) def test_get_bucket_with_object_miss(self): from google.cloud.exceptions import NotFound From b905487d50c51a3c0907709ca1f3f6c8b31d3ae0 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 May 2020 13:24:20 +0300 Subject: [PATCH 03/15] fix unit tests --- tests/unit/test__helpers.py | 16 ++++++++++++++++ tests/unit/test_client.py | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index a61a67f75..a718d476b 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -478,6 +478,22 @@ def test_add_generation_match_parameters_dict(self): ) self.assertEqual(params, EXPECTED_PARAMS) + def test_add_generation_match_parameters_tuple(self): + GENERATION_NUMBER = 9 + METAGENERATION_NUMBER = 6 + EXPECTED_PARAMS = ( + ("param1", "value1"), + ("param2", "value2"), + ) + + params = (("param1", "value1"), ("param2", "value2")) + self._call_fut( + params, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, + ) + self.assertEqual(params, EXPECTED_PARAMS) + class _Connection(object): def __init__(self, *responses): diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 45354768e..7ccf35716 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -623,7 +623,7 @@ def test_lookup_bucket_with_metageneration_match(self): client = self._make_one(project=PROJECT, credentials=CREDENTIALS) BUCKET_NAME = "bucket-name" - URI = "/".join( + URI1 = "/".join( [ client._connection.API_BASE_URL, "storage", @@ -633,6 +633,16 @@ def test_lookup_bucket_with_metageneration_match(self): % (BUCKET_NAME, METAGENERATION_NUMBER), ] ) + URI2 = "/".join( + [ + client._connection.API_BASE_URL, + "storage", + client._connection.API_VERSION, + "b", + "%s?ifMetagenerationMatch=%s&projection=noAcl" + % (BUCKET_NAME, METAGENERATION_NUMBER), + ] + ) data = {"name": BUCKET_NAME} http = _make_requests_session([_make_json_response(data)]) client._http_internal = http @@ -644,11 +654,13 @@ def test_lookup_bucket_with_metageneration_match(self): self.assertEqual(bucket.name, BUCKET_NAME) http.request.assert_called_once_with( method="GET", - url=URI, + url=mock.ANY, data=mock.ANY, headers=mock.ANY, timeout=self._get_default_timeout(), ) + _, kwargs = http.request.call_args + self.assertIn(kwargs.get("url"), (URI1, URI2)) def test_create_bucket_w_missing_client_project(self): credentials = _make_credentials() From ddecf53265a113b7d6726f89cc9fccc98cee914a Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 May 2020 15:52:29 +0300 Subject: [PATCH 04/15] add generation match args into more methods --- google/cloud/storage/_helpers.py | 72 ++++++++++++++++++++-- google/cloud/storage/blob.py | 100 +++++++++++++++++++++++++++++-- google/cloud/storage/bucket.py | 95 +++++++++++++++++++++++++++-- tests/unit/test_blob.py | 82 +++++++++++++++++++++++-- tests/unit/test_bucket.py | 45 ++++++++++++++ 5 files changed, 378 insertions(+), 16 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index d02872eac..1e13a06b0 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -133,6 +133,8 @@ def reload( self, client=None, timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, if_metageneration_match=None, if_metageneration_not_match=None, ): @@ -152,6 +154,19 @@ def reload( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + :type if_metageneration_match: long :param if_metageneration_match: (Optional) Make the operation conditional on whether the blob's current metageneration matches the given value. @@ -163,6 +178,10 @@ def reload( :raises: :class:`ValueError` if ``if_metageneration_match`` and ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) _raise_for_more_than_one_none( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, @@ -174,6 +193,8 @@ def reload( query_params["projection"] = "noAcl" _add_generation_match_parameters( query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) @@ -219,6 +240,8 @@ def patch( self, client=None, timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, if_metageneration_match=None, if_metageneration_not_match=None, ): @@ -240,6 +263,19 @@ def patch( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + :type if_metageneration_match: long :param if_metageneration_match: (Optional) Make the operation conditional on whether the blob's current metageneration matches the given value. @@ -249,8 +285,14 @@ def patch( blob's current metageneration does not match the given value. :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) _raise_for_more_than_one_none( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, @@ -262,6 +304,8 @@ def patch( query_params["projection"] = "full" _add_generation_match_parameters( query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) @@ -282,6 +326,8 @@ def update( self, client=None, timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, if_metageneration_match=None, if_metageneration_not_match=None, ): @@ -303,17 +349,35 @@ def update( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + :type if_metageneration_match: long :param if_metageneration_match: (Optional) Make the operation conditional on whether the blob's current metageneration matches the given value. - :type if_metageneration_not_match: long - :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the - blob's current metageneration does not match the given value. + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. :raises: :class:`ValueError` if ``if_metageneration_match`` and ``if_metageneration_not_match`` are both set. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) _raise_for_more_than_one_none( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index bce83a2b1..805de5305 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -54,6 +54,7 @@ from google.cloud._helpers import _rfc3339_to_datetime from google.cloud._helpers import _to_bytes from google.cloud.exceptions import NotFound +from google.cloud.storage._helpers import _add_generation_match_parameters from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _convert_to_timestamp @@ -566,7 +567,15 @@ def generate_signed_url( access_token=access_token, ) - def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): + def exists( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Determines whether or not this blob exists. If :attr:`user_project` is set on the bucket, bills the API request @@ -583,15 +592,56 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. + :rtype: bool :returns: True if the blob exists in Cloud Storage. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) # We only need the status code (200 or not) so we seek to # minimize the returned payload. query_params = self._query_params query_params["fields"] = "name" + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) try: # We intentionally pass `_target_object=None` since fields=name # would limit the local properties. @@ -609,7 +659,15 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT): except NotFound: return False - def delete(self, client=None, timeout=_DEFAULT_TIMEOUT): + def delete( + self, + client=None, + timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Deletes a blob from Cloud Storage. If :attr:`user_project` is set on the bucket, bills the API request @@ -617,8 +675,9 @@ def delete(self, client=None, timeout=_DEFAULT_TIMEOUT): :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. + :type timeout: float or tuple :param timeout: (Optional) The amount of time, in seconds, to wait for the server response. @@ -626,12 +685,45 @@ def delete(self, client=None, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :raises: :class:`google.cloud.exceptions.NotFound` (propagated from :meth:`google.cloud.storage.bucket.Bucket.delete_blob`). + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ self.bucket.delete_blob( - self.name, client=client, generation=self.generation, timeout=timeout + self.name, + client=client, + generation=self.generation, + timeout=timeout, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, ) def _get_transport(self, client): diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 9d4c8f24d..b492b0a26 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -884,6 +884,10 @@ def get_blob( encryption_key=None, generation=None, timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, **kwargs ): """Get a blob object by name. @@ -921,11 +925,37 @@ def get_blob( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :param kwargs: Keyword arguments to pass to the :class:`~google.cloud.storage.blob.Blob` constructor. :rtype: :class:`google.cloud.storage.blob.Blob` or None :returns: The blob object if it exists, otherwise None. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ blob = Blob( bucket=self, @@ -938,7 +968,14 @@ def get_blob( # NOTE: This will not fail immediately in a batch. However, when # Batch.finish() is called, the resulting `NotFound` will be # raised. - blob.reload(client=client, timeout=timeout) + blob.reload( + client=client, + timeout=timeout, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) except NotFound: return None else: @@ -1224,7 +1261,15 @@ def delete( ) def delete_blob( - self, blob_name, client=None, generation=None, timeout=_DEFAULT_TIMEOUT + self, + blob_name, + client=None, + generation=None, + timeout=_DEFAULT_TIMEOUT, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, ): """Deletes a blob from the current bucket. @@ -1244,7 +1289,7 @@ def delete_blob( :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. :type generation: long @@ -1258,25 +1303,67 @@ def delete_blob( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :raises: :class:`google.cloud.exceptions.NotFound` (to suppress the exception, call ``delete_blobs``, passing a no-op ``on_error`` callback, e.g.: + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. + .. literalinclude:: snippets.py :start-after: [START delete_blobs] :end-before: [END delete_blobs] """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) client = self._require_client(client) blob = Blob(blob_name, bucket=self, generation=generation) + query_params = copy.deepcopy(blob._query_params) + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) # We intentionally pass `_target_object=None` since a DELETE # request has no response value (whether in a standard request or # in a batch request). client._connection.api_request( method="DELETE", path=blob.path, - query_params=blob._query_params, + query_params=query_params, _target_object=None, timeout=timeout, ) diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index bb1aa11e1..1b199cc47 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -707,6 +707,29 @@ def test_exists_hit_w_generation(self): }, ) + def test_exists_w_generation_match(self): + BLOB_NAME = "blob-name" + GENERATION = 123456 + + found_response = ({"status": http_client.OK}, b"") + connection = _Connection(found_response) + client = _Client(connection) + bucket = _Bucket(client) + blob = self._make_one(BLOB_NAME, bucket=bucket) + bucket._blobs[BLOB_NAME] = 1 + self.assertTrue(blob.exists(if_generation_match=GENERATION)) + self.assertEqual(len(connection._requested), 1) + self.assertEqual( + connection._requested[0], + { + "method": "GET", + "path": "/b/name/o/{}".format(BLOB_NAME), + "query_params": {"fields": "name", "ifGenerationMatch": GENERATION}, + "_target_object": None, + "timeout": self._get_default_timeout(), + }, + ) + def test_delete_wo_generation(self): BLOB_NAME = "blob-name" not_found_response = ({"status": http_client.NOT_FOUND}, b"") @@ -718,7 +741,19 @@ def test_delete_wo_generation(self): blob.delete() self.assertFalse(blob.exists()) self.assertEqual( - bucket._deleted, [(BLOB_NAME, None, None, self._get_default_timeout())] + bucket._deleted, + [ + ( + BLOB_NAME, + None, + None, + self._get_default_timeout(), + None, + None, + None, + None, + ) + ], ) def test_delete_w_generation(self): @@ -732,7 +767,25 @@ def test_delete_w_generation(self): bucket._blobs[BLOB_NAME] = 1 blob.delete(timeout=42) self.assertFalse(blob.exists()) - self.assertEqual(bucket._deleted, [(BLOB_NAME, None, GENERATION, 42)]) + self.assertEqual( + bucket._deleted, [(BLOB_NAME, None, GENERATION, 42, None, None, None, None)] + ) + + def test_delete_w_generation_match(self): + BLOB_NAME = "blob-name" + GENERATION = 123456 + not_found_response = ({"status": http_client.NOT_FOUND}, b"") + connection = _Connection(not_found_response) + client = _Client(connection) + bucket = _Bucket(client) + blob = self._make_one(BLOB_NAME, bucket=bucket, generation=GENERATION) + bucket._blobs[BLOB_NAME] = 1 + blob.delete(timeout=42, if_generation_match=GENERATION) + self.assertFalse(blob.exists()) + self.assertEqual( + bucket._deleted, + [(BLOB_NAME, None, GENERATION, 42, GENERATION, None, None, None)], + ) def test__get_transport(self): client = mock.Mock(spec=[u"_credentials", "_http"]) @@ -3616,9 +3669,30 @@ def __init__(self, client=None, name="name", user_project=None): self.path = "/b/" + name self.user_project = user_project - def delete_blob(self, blob_name, client=None, generation=None, timeout=None): + def delete_blob( + self, + blob_name, + client=None, + generation=None, + timeout=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): del self._blobs[blob_name] - self._deleted.append((blob_name, client, generation, timeout)) + self._deleted.append( + ( + blob_name, + client, + generation, + timeout, + if_generation_match, + if_generation_not_match, + if_metageneration_match, + if_metageneration_not_match, + ) + ) class _Client(object): diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 416e53d49..b01edf03d 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -722,6 +722,26 @@ def test_get_blob_hit_w_generation(self): self.assertEqual(kw["query_params"], expected_qp) self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_get_blob_w_generation_match(self): + NAME = "name" + BLOB_NAME = "blob-name" + GENERATION = 1512565576797178 + + connection = _Connection({"name": BLOB_NAME, "generation": GENERATION}) + client = _Client(connection) + bucket = self._make_one(name=NAME) + blob = bucket.get_blob(BLOB_NAME, client=client, if_generation_match=GENERATION) + + self.assertIs(blob.bucket, bucket) + self.assertEqual(blob.name, BLOB_NAME) + self.assertEqual(blob.generation, GENERATION) + (kw,) = connection._requested + expected_qp = {"ifGenerationMatch": GENERATION, "projection": "noAcl"} + self.assertEqual(kw["method"], "GET") + self.assertEqual(kw["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) + self.assertEqual(kw["query_params"], expected_qp) + self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_get_blob_hit_with_kwargs(self): from google.cloud.storage.blob import _get_encryption_headers @@ -1069,6 +1089,31 @@ def test_delete_blob_hit_with_generation(self): self.assertEqual(kw["query_params"], {"generation": GENERATION}) self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_delete_blob_with_generation_match(self): + NAME = "name" + BLOB_NAME = "blob-name" + GENERATION = 6 + METAGENERATION = 9 + + connection = _Connection({}) + client = _Client(connection) + bucket = self._make_one(client=client, name=NAME) + result = bucket.delete_blob( + BLOB_NAME, + if_generation_match=GENERATION, + if_metageneration_match=METAGENERATION, + ) + + self.assertIsNone(result) + (kw,) = connection._requested + self.assertEqual(kw["method"], "DELETE") + self.assertEqual(kw["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) + self.assertEqual( + kw["query_params"], + {"ifGenerationMatch": GENERATION, "ifMetagenerationMatch": METAGENERATION}, + ) + self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_delete_blobs_empty(self): NAME = "name" connection = _Connection() From 74961fa111ebadfd3702bed8ef6c62d28cdd08a1 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 4 May 2020 13:33:22 +0300 Subject: [PATCH 05/15] feat: add if*generation*Match support, pt2 --- google/cloud/storage/_helpers.py | 4 + google/cloud/storage/blob.py | 395 ++++++++++++++++++++++++++++++- google/cloud/storage/bucket.py | 104 +++++++- tests/system/test_system.py | 31 +++ tests/unit/test_blob.py | 175 ++++++++++++++ tests/unit/test_bucket.py | 37 +++ 6 files changed, 734 insertions(+), 12 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index 1e13a06b0..727640192 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -36,6 +36,10 @@ ("if_generation_not_match", "ifGenerationNotMatch"), ("if_metageneration_match", "ifMetagenerationMatch"), ("if_metageneration_not_match", "ifMetagenerationNotMatch"), + ("if_source_generation_match", "ifSourceGenerationMatch"), + ("if_source_generation_not_match", "ifSourceGenerationNotMatch"), + ("if_source_metageneration_match", "ifSourceMetagenerationMatch"), + ("if_source_metageneration_not_match", "ifSourceMetagenerationNotMatch"), ) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 805de5305..149a17d24 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -741,7 +741,14 @@ def _get_transport(self, client): client = self._require_client(client) return client._http - def _get_download_url(self, client): + def _get_download_url( + self, + client, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Get the download URL for the current blob. If the ``media_link`` has been loaded, it will be used, otherwise @@ -751,9 +758,42 @@ def _get_download_url(self, client): :type client: :class:`~google.cloud.storage.client.Client` :param client: The client to use. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :rtype: str :returns: The download URL for the current blob. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) name_value_pairs = [] if self.media_link is None: base_url = _DOWNLOAD_URL_TEMPLATE.format( @@ -767,6 +807,13 @@ def _get_download_url(self, client): if self.user_project is not None: name_value_pairs.append(("userProject", self.user_project)) + _add_generation_match_parameters( + name_value_pairs, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) return _add_query_parameters(base_url, name_value_pairs) def _do_download( @@ -839,7 +886,16 @@ def _do_download( download.consume_next_chunk(transport) def download_to_file( - self, file_obj, client=None, start=None, end=None, raw_download=False + self, + file_obj, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, ): """Download the contents of this blob into a file-like object. @@ -884,10 +940,42 @@ def download_to_file( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :raises: :class:`google.cloud.exceptions.NotFound` + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ client = self._require_client(client) - download_url = self._get_download_url(client) + + download_url = self._get_download_url( + client, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) headers = _get_encryption_headers(self._encryption_key) headers["accept-encoding"] = "gzip" @@ -900,7 +988,16 @@ def download_to_file( _raise_from_invalid_response(exc) def download_to_filename( - self, filename, client=None, start=None, end=None, raw_download=False + self, + filename, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, ): """Download the contents of this blob into a named file. @@ -912,7 +1009,7 @@ def download_to_filename( :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. :type start: int @@ -925,7 +1022,32 @@ def download_to_filename( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :raises: :class:`google.cloud.exceptions.NotFound` + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ try: with open(filename, "wb") as file_obj: @@ -935,6 +1057,10 @@ def download_to_filename( start=start, end=end, raw_download=raw_download, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, ) except resumable_media.DataCorruption: # Delete the corrupt downloaded file. @@ -949,7 +1075,17 @@ def download_to_filename( mtime = updated.timestamp() os.utime(file_obj.name, (mtime, mtime)) - def download_as_string(self, client=None, start=None, end=None, raw_download=False): + def download_as_string( + self, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): """Download the contents of this blob as a bytes object. If :attr:`user_project` is set on the bucket, bills the API request @@ -957,7 +1093,7 @@ def download_as_string(self, client=None, start=None, end=None, raw_download=Fal :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. :type start: int @@ -970,9 +1106,35 @@ def download_as_string(self, client=None, start=None, end=None, raw_download=Fal :param raw_download: (Optional) If true, download the object without any expansion. + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + :rtype: bytes :returns: The data stored in this blob. + :raises: :class:`google.cloud.exceptions.NotFound` + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match`` + are both set. """ string_buffer = BytesIO() self.download_to_file( @@ -981,6 +1143,10 @@ def download_as_string(self, client=None, start=None, end=None, raw_download=Fal start=start, end=end, raw_download=raw_download, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, ) return string_buffer.getvalue() @@ -2139,7 +2305,21 @@ def compose(self, sources, client=None, timeout=_DEFAULT_TIMEOUT): ) self._set_properties(api_response) - def rewrite(self, source, token=None, client=None, timeout=_DEFAULT_TIMEOUT): + def rewrite( + self, + source, + token=None, + client=None, + timeout=_DEFAULT_TIMEOUT, + 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, + ): """Rewrite source blob into this one. If :attr:`user_project` is set on the bucket, bills the API request @@ -2165,13 +2345,95 @@ def rewrite(self, source, token=None, client=None, timeout=_DEFAULT_TIMEOUT): Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Makes the operation + conditional on whether the destination + object's current generation matches the + given value. Setting to 0 makes the + operation succeed only if there are no + live versions of the object. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + generation does not match the given + value. If no live object exists, + the precondition fails. Setting to + 0 makes the operation succeed only + if there is a live version + of the object. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration matches the given + value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration does not match + the given value. + + :type if_source_generation_match: long + :param if_source_generation_match: (Optional) Makes the operation + conditional on whether the source + object's generation matches the + given value. + + :type if_source_generation_not_match: long + :param if_source_generation_not_match: (Optional) Makes the operation + conditional on whether the source + object's generation does not match + the given value. + + :type if_source_metageneration_match: long + :param if_source_metageneration_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + matches the given value. + + :type if_source_metageneration_not_match: long + :param if_source_metageneration_not_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + does not match the given value. + :rtype: tuple :returns: ``(token, bytes_rewritten, total_bytes)``, where ``token`` is a rewrite token (``None`` if the rewrite is complete), ``bytes_rewritten`` is the number of bytes rewritten so far, and ``total_bytes`` is the total number of bytes to be rewritten. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match``, + or ``if_source_generation_match`` and + ``if_source_generation_not_match``, or + ``if_source_metageneration_match`` and + ``if_source_metageneration_not_match`` + are both set. """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + _raise_for_more_than_one_none( + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + ) + _raise_for_more_than_one_none( + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) client = self._require_client(client) headers = _get_encryption_headers(self._encryption_key) headers.update(_get_encryption_headers(source._encryption_key, source=True)) @@ -2189,6 +2451,18 @@ def rewrite(self, source, token=None, client=None, timeout=_DEFAULT_TIMEOUT): if self.kms_key_name is not None: query_params["destinationKmsKeyName"] = self.kms_key_name + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) + api_response = client._connection.api_request( method="POST", path=source.path + "/rewriteTo" + self.path, @@ -2210,7 +2484,19 @@ def rewrite(self, source, token=None, client=None, timeout=_DEFAULT_TIMEOUT): return api_response["rewriteToken"], rewritten, size - def update_storage_class(self, new_class, client=None): + def update_storage_class( + self, + new_class, + client=None, + 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, + ): """Update blob's storage class via a rewrite-in-place. This helper will wait for the rewrite to complete before returning, so it may take some time for large files. @@ -2235,6 +2521,72 @@ def update_storage_class(self, new_class, client=None): :type client: :class:`~google.cloud.storage.client.Client` :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. + + :type if_generation_match: long + :param if_generation_match: (Optional) Makes the operation + conditional on whether the destination + object's current generation matches the + given value. Setting to 0 makes the + operation succeed only if there are no + live versions of the object. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + generation does not match the given + value. If no live object exists, + the precondition fails. Setting to + 0 makes the operation succeed only + if there is a live version + of the object. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration matches the given + value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration does not match + the given value. + + :type if_source_generation_match: long + :param if_source_generation_match: (Optional) Makes the operation + conditional on whether the source + object's generation matches the + given value. + + :type if_source_generation_not_match: long + :param if_source_generation_not_match: (Optional) Makes the operation + conditional on whether the source + object's generation does not match + the given value. + + :type if_source_metageneration_match: long + :param if_source_metageneration_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + matches the given value. + + :type if_source_metageneration_not_match: long + :param if_source_metageneration_not_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + does not match the given value. + + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match``, + or ``if_source_generation_match`` and + ``if_source_generation_not_match``, or + ``if_source_metageneration_match`` and + ``if_source_metageneration_not_match`` + are both set. """ if new_class not in self.STORAGE_CLASSES: raise ValueError("Invalid storage class: %s" % (new_class,)) @@ -2243,9 +2595,30 @@ def update_storage_class(self, new_class, client=None): self._patch_property("storageClass", new_class) # Execute consecutive rewrite operations until operation is done - token, _, _ = self.rewrite(self) + token, _, _ = self.rewrite( + self, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) while token is not None: - token, _, _ = self.rewrite(self, token=token) + token, _, _ = self.rewrite( + self, + token=token, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) cache_control = _scalar_property("cacheControl") """HTTP 'Cache-Control' header for this object. diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index b492b0a26..14c40b7b7 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1421,6 +1421,14 @@ def copy_blob( preserve_acl=True, source_generation=None, timeout=_DEFAULT_TIMEOUT, + 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, ): """Copy the given blob to the given bucket, optionally with a new name. @@ -1438,7 +1446,7 @@ def copy_blob( :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` - :param client: (Optional) The client to use. If not passed, falls back + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. :type preserve_acl: bool @@ -1457,9 +1465,75 @@ def copy_blob( Can also be passed as a tuple (connect_timeout, read_timeout). See :meth:`requests.Session.request` documentation for details. + :type if_generation_match: long + :param if_generation_match: (Optional) Makes the operation + conditional on whether the destination + object's current generation matches the + given value. Setting to 0 makes the + operation succeed only if there are no + live versions of the object. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + generation does not match the given + value. If no live object exists, + the precondition fails. Setting to + 0 makes the operation succeed only + if there is a live version + of the object. + + :type if_metageneration_match: long + :param if_metageneration_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration matches the given + value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Makes the operation + conditional on whether the + destination object's current + metageneration does not match + the given value. + + :type if_source_generation_match: long + :param if_source_generation_match: (Optional) Makes the operation + conditional on whether the source + object's generation matches the + given value. + + :type if_source_generation_not_match: long + :param if_source_generation_not_match: (Optional) Makes the operation + conditional on whether the source + object's generation does not match + the given value. + + :type if_source_metageneration_match: long + :param if_source_metageneration_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + matches the given value. + + :type if_source_metageneration_not_match: long + :param if_source_metageneration_not_match: (Optional) Makes the operation + conditional on whether the source + object's current metageneration + does not match the given value. + :rtype: :class:`google.cloud.storage.blob.Blob` :returns: The new Blob. + :raises: :class:`ValueError` if ``if_metageneration_match`` and + ``if_metageneration_not_match``, or + ``if_generation_match`` and ``if_generation_not_match``, + or ``if_source_generation_match`` and + ``if_source_generation_not_match``, or + ``if_source_metageneration_match`` and + ``if_source_metageneration_not_match`` + are both set. + Example: Copy a blob including ACL. @@ -1474,6 +1548,22 @@ def copy_blob( >>> new_blob = bucket.copy_blob(blob, dst_bucket) >>> new_blob.acl.save(blob.acl) """ + _raise_for_more_than_one_none( + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + ) + _raise_for_more_than_one_none( + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + _raise_for_more_than_one_none( + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + ) + _raise_for_more_than_one_none( + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) client = self._require_client(client) query_params = {} @@ -1483,6 +1573,18 @@ def copy_blob( if source_generation is not None: query_params["sourceGeneration"] = source_generation + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) + if new_name is None: new_name = blob.name diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 675758794..bded33df7 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -427,6 +427,37 @@ def test_copy_existing_file_with_user_project(self): for blob in to_delete: retry_429_harder(blob.delete)() + def test_copy_existing_file_with_generation_match(self): + new_bucket_name = "copy-w-requester-pays" + unique_resource_id("-") + created = retry_429_503(Config.CLIENT.create_bucket)( + new_bucket_name, requester_pays=True + ) + self.case_buckets_to_delete.append(new_bucket_name) + self.assertEqual(created.name, new_bucket_name) + + to_delete = [] + blob = storage.Blob("simple", bucket=created) + blob.upload_from_string(b"DEADBEEF") + to_delete.append(blob) + try: + dest_bucket = Config.CLIENT.bucket(new_bucket_name) + + new_blob = dest_bucket.copy_blob( + blob, + dest_bucket, + "simple-copy", + if_source_generation_match=blob.generation, + if_source_metageneration_match=blob.metageneration, + ) + to_delete.append(new_blob) + + base_contents = blob.download_as_string() + copied_contents = new_blob.download_as_string() + self.assertEqual(base_contents, copied_contents) + finally: + for blob in to_delete: + retry_429_harder(blob.delete)() + @unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.") def test_bucket_get_blob_with_user_project(self): new_bucket_name = "w-requester-pays" + unique_resource_id("-") diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index 1b199cc47..d79c26daf 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -810,6 +810,24 @@ def test__get_download_url_with_media_link(self): self.assertEqual(download_url, media_link) + def test__get_download_url_with_generation_match(self): + GENERATION_NUMBER = 6 + MEDIA_LINK = "http://test.invalid" + + blob = self._make_one("something.txt", bucket=_Bucket(name="IRRELEVANT")) + # Set the media link on the blob + blob._properties["mediaLink"] = MEDIA_LINK + + client = mock.Mock(_connection=_Connection) + client._connection.API_BASE_URL = "https://storage.googleapis.com" + download_url = blob._get_download_url( + client, if_generation_match=GENERATION_NUMBER + ) + self.assertEqual( + download_url, + "{}?ifGenerationMatch={}".format(MEDIA_LINK, GENERATION_NUMBER), + ) + def test__get_download_url_with_media_link_w_user_project(self): blob_name = "something.txt" user_project = "user-project-123" @@ -1091,6 +1109,28 @@ def test_download_to_file_wo_media_link(self): client._http, file_obj, expected_url, headers, None, None, False ) + def test_download_to_file_w_generation_match(self): + GENERATION_NUMBER = 6 + HEADERS = {"accept-encoding": "gzip"} + EXPECTED_URL = ( + "https://storage.googleapis.com/download/storage/v1/b/" + "name/o/blob-name?alt=media&ifGenerationNotMatch={}".format( + GENERATION_NUMBER + ) + ) + + client = mock.Mock(_connection=_Connection, spec=[u"_http"]) + client._connection.API_BASE_URL = "https://storage.googleapis.com" + blob = self._make_one("blob-name", bucket=_Bucket(client)) + blob._do_download = mock.Mock() + file_obj = io.BytesIO() + + blob.download_to_file(file_obj, if_generation_not_match=GENERATION_NUMBER) + + blob._do_download.assert_called_once_with( + client._http, file_obj, EXPECTED_URL, HEADERS, None, None, False + ) + def _download_to_file_helper(self, use_chunks, raw_download): blob_name = "blob-name" client = mock.Mock(spec=[u"_http"]) @@ -1161,6 +1201,26 @@ def _download_to_filename_helper(self, updated, raw_download): stream = blob._do_download.mock_calls[0].args[1] self.assertEqual(stream.name, temp.name) + def test_download_to_filename_w_generation_match(self): + from google.cloud._testing import _NamedTemporaryFile + + GENERATION_NUMBER = 6 + MEDIA_LINK = "http://example.com/media/" + EXPECTED_LINK = MEDIA_LINK + "?ifGenerationMatch={}".format(GENERATION_NUMBER) + HEADERS = {"accept-encoding": "gzip"} + + client = mock.Mock(spec=["_http"]) + + blob = self._make_one("blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK}) + blob._do_download = mock.Mock() + + with _NamedTemporaryFile() as temp: + blob.download_to_filename(temp.name, if_generation_match=GENERATION_NUMBER) + + blob._do_download.assert_called_once_with( + client._http, mock.ANY, EXPECTED_LINK, HEADERS, None, None, False + ) + def test_download_to_filename_w_updated_wo_raw(self): updated = "2014-12-06T13:13:50.690Z" self._download_to_filename_helper(updated=updated, raw_download=False) @@ -1254,6 +1314,33 @@ def _download_as_string_helper(self, raw_download): stream = blob._do_download.mock_calls[0].args[1] self.assertIsInstance(stream, io.BytesIO) + def test_download_as_string_w_generation_match(self): + GENERATION_NUMBER = 6 + MEDIA_LINK = "http://example.com/media/" + + client = mock.Mock(spec=["_http"]) + blob = self._make_one( + "blob-name", + bucket=_Bucket(client), + properties={"mediaLink": MEDIA_LINK} + ) + blob.download_to_file = mock.Mock() + + fetched = blob.download_as_string(if_generation_match=GENERATION_NUMBER) + self.assertEqual(fetched, b"") + + blob.download_to_file.assert_called_once_with( + mock.ANY, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=GENERATION_NUMBER, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ) + def test_download_as_string_wo_raw(self): self._download_as_string_helper(raw_download=False) @@ -2737,6 +2824,55 @@ def test_rewrite_w_generations(self): self.assertEqual(kw["query_params"], {"sourceGeneration": SOURCE_GENERATION}) self.assertEqual(kw["timeout"], 42) + def test_rewrite_w_generation_match(self): + SOURCE_BLOB = "source" + SOURCE_GENERATION_NUMBER = 42 + DEST_BLOB = "dest" + DEST_BUCKET = "other-bucket" + DEST_GENERATION_NUMBER = 16 + TOKEN = "TOKEN" + RESPONSE = { + "totalBytesRewritten": 33, + "objectSize": 42, + "done": False, + "rewriteToken": TOKEN, + } + response = ({"status": http_client.OK}, RESPONSE) + connection = _Connection(response) + client = _Client(connection) + source_bucket = _Bucket(client=client) + source_blob = self._make_one( + SOURCE_BLOB, bucket=source_bucket, generation=SOURCE_GENERATION_NUMBER + ) + dest_bucket = _Bucket(client=client, name=DEST_BUCKET) + dest_blob = self._make_one( + DEST_BLOB, bucket=dest_bucket, generation=DEST_GENERATION_NUMBER + ) + token, rewritten, size = dest_blob.rewrite( + source_blob, + timeout=42, + if_generation_match=dest_blob.generation, + if_source_generation_match=source_blob.generation, + ) + (kw,) = connection._requested + self.assertEqual(kw["method"], "POST") + self.assertEqual( + kw["path"], + "/b/%s/o/%s/rewriteTo/b/%s/o/%s" + % ( + (source_bucket.name, source_blob.name, dest_bucket.name, dest_blob.name) + ), + ) + self.assertEqual( + kw["query_params"], + { + "ifSourceGenerationMatch": SOURCE_GENERATION_NUMBER, + "ifGenerationMatch": DEST_GENERATION_NUMBER, + "sourceGeneration": SOURCE_GENERATION_NUMBER, + }, + ) + self.assertEqual(kw["timeout"], 42) + def test_rewrite_other_bucket_other_name_no_encryption_partial(self): SOURCE_BLOB = "source" DEST_BLOB = "dest" @@ -3045,6 +3181,45 @@ def test_update_storage_class_w_encryption_key_w_user_project(self): self.assertEqual(headers["X-Goog-Encryption-Key"], BLOB_KEY_B64) self.assertEqual(headers["X-Goog-Encryption-Key-Sha256"], BLOB_KEY_HASH_B64) + def test_update_storage_class_w_generation_match(self): + BLOB_NAME = "blob-name" + STORAGE_CLASS = u"NEARLINE" + GENERATION_NUMBER = 6 + SOURCE_GENERATION_NUMBER = 9 + RESPONSE = { + "totalBytesRewritten": 42, + "objectSize": 42, + "done": True, + "resource": {"storageClass": STORAGE_CLASS}, + } + response = ({"status": http_client.OK}, RESPONSE) + connection = _Connection(response) + client = _Client(connection) + bucket = _Bucket(client=client) + blob = self._make_one(BLOB_NAME, bucket=bucket) + + blob.update_storage_class( + "NEARLINE", + if_generation_match=GENERATION_NUMBER, + if_source_generation_match=SOURCE_GENERATION_NUMBER, + ) + self.assertEqual(blob.storage_class, "NEARLINE") + + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]["method"], "POST") + PATH = "/b/name/o/%s/rewriteTo/b/name/o/%s" % (BLOB_NAME, BLOB_NAME) + self.assertEqual(kw[0]["path"], PATH) + self.assertEqual( + kw[0]["query_params"], + { + "ifGenerationMatch": GENERATION_NUMBER, + "ifSourceGenerationMatch": SOURCE_GENERATION_NUMBER, + }, + ) + SENT = {"storageClass": STORAGE_CLASS} + self.assertEqual(kw[0]["data"], SENT) + def test_cache_control_getter(self): BLOB_NAME = "blob-name" bucket = _Bucket() diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index b01edf03d..ecdde46aa 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1234,6 +1234,43 @@ def test_copy_blobs_source_generation(self): self.assertEqual(kw["query_params"], {"sourceGeneration": GENERATION}) self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_copy_blobs_w_generation_match(self): + SOURCE = "source" + DEST = "dest" + BLOB_NAME = "blob-name" + GENERATION_NUMBER = 6 + SOURCE_GENERATION_NUMBER = 9 + + connection = _Connection({}) + client = _Client(connection) + source = self._make_one(client=client, name=SOURCE) + dest = self._make_one(client=client, name=DEST) + blob = self._make_blob(SOURCE, BLOB_NAME) + + new_blob = source.copy_blob( + blob, + dest, + if_generation_match=GENERATION_NUMBER, + if_source_generation_match=SOURCE_GENERATION_NUMBER, + ) + self.assertIs(new_blob.bucket, dest) + self.assertEqual(new_blob.name, BLOB_NAME) + + (kw,) = connection._requested + COPY_PATH = "/b/{}/o/{}/copyTo/b/{}/o/{}".format( + SOURCE, BLOB_NAME, DEST, BLOB_NAME + ) + self.assertEqual(kw["method"], "POST") + self.assertEqual(kw["path"], COPY_PATH) + self.assertEqual( + kw["query_params"], + { + "ifGenerationMatch": GENERATION_NUMBER, + "ifSourceGenerationMatch": SOURCE_GENERATION_NUMBER, + }, + ) + self.assertEqual(kw["timeout"], self._get_default_timeout()) + def test_copy_blobs_preserve_acl(self): from google.cloud.storage.acl import ObjectACL From 7e083610ba41e7cb296eea52add4088dfd08fd41 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 4 May 2020 13:52:08 +0300 Subject: [PATCH 06/15] Lint fix. --- tests/unit/test_blob.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index d79c26daf..f9ea4f21c 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -1211,7 +1211,9 @@ def test_download_to_filename_w_generation_match(self): client = mock.Mock(spec=["_http"]) - blob = self._make_one("blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK}) + blob = self._make_one( + "blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK} + ) blob._do_download = mock.Mock() with _NamedTemporaryFile() as temp: @@ -1320,9 +1322,7 @@ def test_download_as_string_w_generation_match(self): client = mock.Mock(spec=["_http"]) blob = self._make_one( - "blob-name", - bucket=_Bucket(client), - properties={"mediaLink": MEDIA_LINK} + "blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK} ) blob.download_to_file = mock.Mock() From 6f9f910ecfccc2f06022563b0396f91d5d83c9ba Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 5 May 2020 10:34:42 +0300 Subject: [PATCH 07/15] delete "more than one set "checks --- google/cloud/storage/_helpers.py | 40 -------------------------------- google/cloud/storage/blob.py | 18 -------------- google/cloud/storage/bucket.py | 39 ------------------------------- google/cloud/storage/client.py | 7 ------ 4 files changed, 104 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index 1e13a06b0..d450d73bd 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -174,18 +174,7 @@ def reload( :type if_metageneration_not_match: long :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the blob's current metageneration does not match the given value. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) query_params = self._query_params # Pass only '?projection=noAcl' here because 'acl' and related @@ -283,20 +272,7 @@ def patch( :type if_metageneration_not_match: long :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the blob's current metageneration does not match the given value. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) query_params = self._query_params # Pass '?projection=full' here because 'PATCH' documented not @@ -365,23 +341,7 @@ def update( :type if_metageneration_match: long :param if_metageneration_match: (Optional) Make the operation conditional on whether the blob's current metageneration matches the given value. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) query_params = self._query_params diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 805de5305..54ccbae35 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -613,22 +613,9 @@ def exists( :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the blob's current metageneration does not match the given value. - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. - :rtype: bool :returns: True if the blob exists in Cloud Storage. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) # We only need the status code (200 or not) so we seek to # minimize the returned payload. @@ -709,11 +696,6 @@ def delete( :raises: :class:`google.cloud.exceptions.NotFound` (propagated from :meth:`google.cloud.storage.bucket.Bucket.delete_blob`). - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ self.bucket.delete_blob( self.name, diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index b492b0a26..ae5933465 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -681,14 +681,7 @@ def exists( :rtype: bool :returns: True if the bucket exists in Cloud Storage. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) # We only need the status code (200 or not) so we seek to # minimize the returned payload. @@ -823,14 +816,7 @@ def patch( :type if_metageneration_not_match: long :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the blob's current metageneration does not match the given value. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) # Special case: For buckets, it is possible that labels are being # removed; this requires special handling. if self._label_removals: @@ -951,11 +937,6 @@ def get_blob( :rtype: :class:`google.cloud.storage.blob.Blob` or None :returns: The blob object if it exists, otherwise None. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ blob = Blob( bucket=self, @@ -1208,14 +1189,7 @@ def delete( :raises: :class:`ValueError` if ``force`` is ``True`` and the bucket contains more than 256 objects / blobs. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) query_params = {} @@ -1328,24 +1302,11 @@ def delete_blob( the exception, call ``delete_blobs``, passing a no-op ``on_error`` callback, e.g.: - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. - .. literalinclude:: snippets.py :start-after: [START delete_blobs] :end-before: [END delete_blobs] """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) client = self._require_client(client) blob = Blob(blob_name, bucket=self, generation=generation) diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index dabaffb1d..1124828d5 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -322,10 +322,6 @@ def get_bucket( google.cloud.exceptions.NotFound If the bucket is not found. - ValueError - If `if_metageneration_match` and - `if_metageneration_not_match` are both set. - Examples: Retrieve a bucket using a string. @@ -391,9 +387,6 @@ def lookup_bucket( :rtype: :class:`google.cloud.storage.bucket.Bucket` :returns: The bucket matching the name provided or None if not found. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match`` are both set. """ try: return self.get_bucket( From ff26d2f2b743dbbaf4091d158b598a30043a494d Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 5 May 2020 10:47:51 +0300 Subject: [PATCH 08/15] del excess import --- google/cloud/storage/bucket.py | 1 - 1 file changed, 1 deletion(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index ae5933465..131cb086e 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -34,7 +34,6 @@ from google.cloud.storage import _signing from google.cloud.storage._helpers import _add_generation_match_parameters from google.cloud.storage._helpers import _PropertyMixin -from google.cloud.storage._helpers import _raise_for_more_than_one_none from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _validate_name from google.cloud.storage._signing import generate_signed_url_v2 From 1f4d9c14a03599020d514e7a88f0dff4a9dd2254 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 5 May 2020 10:56:12 +0300 Subject: [PATCH 09/15] delete "more than one set" checks --- google/cloud/storage/blob.py | 62 ---------------------------------- google/cloud/storage/bucket.py | 25 -------------- 2 files changed, 87 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 36aef8093..ba6f34407 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -762,20 +762,7 @@ def _get_download_url( :rtype: str :returns: The download URL for the current blob. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) name_value_pairs = [] if self.media_link is None: base_url = _DOWNLOAD_URL_TEMPLATE.format( @@ -943,11 +930,6 @@ def download_to_file( blob's current metageneration does not match the given value. :raises: :class:`google.cloud.exceptions.NotFound` - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ client = self._require_client(client) @@ -1025,11 +1007,6 @@ def download_to_filename( blob's current metageneration does not match the given value. :raises: :class:`google.cloud.exceptions.NotFound` - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ try: with open(filename, "wb") as file_obj: @@ -1112,11 +1089,6 @@ def download_as_string( :returns: The data stored in this blob. :raises: :class:`google.cloud.exceptions.NotFound` - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match`` - are both set. """ string_buffer = BytesIO() self.download_to_file( @@ -2390,32 +2362,7 @@ def rewrite( ``bytes_rewritten`` is the number of bytes rewritten so far, and ``total_bytes`` is the total number of bytes to be rewritten. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match``, - or ``if_source_generation_match`` and - ``if_source_generation_not_match``, or - ``if_source_metageneration_match`` and - ``if_source_metageneration_not_match`` - are both set. """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - _raise_for_more_than_one_none( - if_source_generation_match=if_source_generation_match, - if_source_generation_not_match=if_source_generation_not_match, - ) - _raise_for_more_than_one_none( - if_source_metageneration_match=if_source_metageneration_match, - if_source_metageneration_not_match=if_source_metageneration_not_match, - ) client = self._require_client(client) headers = _get_encryption_headers(self._encryption_key) headers.update(_get_encryption_headers(source._encryption_key, source=True)) @@ -2560,15 +2507,6 @@ def update_storage_class( conditional on whether the source object's current metageneration does not match the given value. - - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match``, - or ``if_source_generation_match`` and - ``if_source_generation_not_match``, or - ``if_source_metageneration_match`` and - ``if_source_metageneration_not_match`` - are both set. """ if new_class not in self.STORAGE_CLASSES: raise ValueError("Invalid storage class: %s" % (new_class,)) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 703e5cb80..cce7f3405 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1485,15 +1485,6 @@ def copy_blob( :rtype: :class:`google.cloud.storage.blob.Blob` :returns: The new Blob. - :raises: :class:`ValueError` if ``if_metageneration_match`` and - ``if_metageneration_not_match``, or - ``if_generation_match`` and ``if_generation_not_match``, - or ``if_source_generation_match`` and - ``if_source_generation_not_match``, or - ``if_source_metageneration_match`` and - ``if_source_metageneration_not_match`` - are both set. - Example: Copy a blob including ACL. @@ -1508,22 +1499,6 @@ def copy_blob( >>> new_blob = bucket.copy_blob(blob, dst_bucket) >>> new_blob.acl.save(blob.acl) """ - _raise_for_more_than_one_none( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - _raise_for_more_than_one_none( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - _raise_for_more_than_one_none( - if_source_generation_match=if_source_generation_match, - if_source_generation_not_match=if_source_generation_not_match, - ) - _raise_for_more_than_one_none( - if_source_metageneration_match=if_source_metageneration_match, - if_source_metageneration_not_match=if_source_metageneration_not_match, - ) client = self._require_client(client) query_params = {} From bd5b54f516c2386c7e3730cc0c1d720539f8e2b7 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 12 May 2020 10:57:32 +0300 Subject: [PATCH 10/15] rename the helper; add error raising in case of wront parameters type --- google/cloud/storage/_helpers.py | 10 +++++++++- google/cloud/storage/blob.py | 8 ++++---- tests/unit/test__helpers.py | 16 ++++++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index d450d73bd..6b8d5988b 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -436,6 +436,9 @@ def _add_generation_match_parameters(parameters, **match_parameters): :type match_parameters: dict :param match_parameters: if*generation*match parameters to add. + + :raises: :exc:`ValueError` if ``parameters`` is not a ``list()`` + or a ``dict()``. """ for snakecase_name, camelcase_name in _GENERATION_MATCH_PARAMETERS: value = match_parameters.get(snakecase_name) @@ -447,8 +450,13 @@ def _add_generation_match_parameters(parameters, **match_parameters): elif isinstance(parameters, dict): parameters[camelcase_name] = value + else: + raise ValueError( + "`parameters` argument should be a dict() or a list()." + ) + -def _raise_for_more_than_one_none(**kwargs): +def _raise_if_more_than_one_set(**kwargs): """Raise ``ValueError`` exception if more than one parameter was set. :type error: :exc:`ValueError` diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 54ccbae35..520c2bf54 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -58,7 +58,7 @@ from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _convert_to_timestamp -from google.cloud.storage._helpers import _raise_for_more_than_one_none +from google.cloud.storage._helpers import _raise_if_more_than_one_set from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import ACL @@ -185,7 +185,7 @@ def __init__( self.chunk_size = chunk_size # Check that setter accepts value. self._bucket = bucket self._acl = ObjectACL(self) - _raise_for_more_than_one_none( + _raise_if_more_than_one_set( encryption_key=encryption_key, kms_key_name=kms_key_name, ) @@ -1615,12 +1615,12 @@ def upload_from_file( if num_retries is not None: warnings.warn(_NUM_RETRIES_MESSAGE, DeprecationWarning, stacklevel=2) - _raise_for_more_than_one_none( + _raise_if_more_than_one_set( if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, ) - _raise_for_more_than_one_none( + _raise_if_more_than_one_set( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index a718d476b..9663f787f 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -481,18 +481,14 @@ def test_add_generation_match_parameters_dict(self): def test_add_generation_match_parameters_tuple(self): GENERATION_NUMBER = 9 METAGENERATION_NUMBER = 6 - EXPECTED_PARAMS = ( - ("param1", "value1"), - ("param2", "value2"), - ) params = (("param1", "value1"), ("param2", "value2")) - self._call_fut( - params, - if_generation_match=GENERATION_NUMBER, - if_metageneration_match=METAGENERATION_NUMBER, - ) - self.assertEqual(params, EXPECTED_PARAMS) + with self.assertRaises(ValueError): + self._call_fut( + params, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, + ) class _Connection(object): From bbebfc1f359cfd56812fb7d7f5e8c594db79fdbd Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 14 May 2020 14:21:55 +0300 Subject: [PATCH 11/15] add more system tests --- tests/system/test_system.py | 163 +++++++++++++++++++++++++++++++++++- tests/unit/test__helpers.py | 16 +++- tests/unit/test_blob.py | 16 +++- 3 files changed, 186 insertions(+), 9 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 4354e835b..30973cd0f 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -427,8 +427,38 @@ def test_copy_existing_file_with_user_project(self): for blob in to_delete: retry_429_harder(blob.delete)() - def test_copy_existing_file_with_generation_match(self): - new_bucket_name = "copy-w-requester-pays" + unique_resource_id("-") + def test_copy_file_with_generation_match(self): + new_bucket_name = "generation-match" + unique_resource_id("-") + created = retry_429_503(Config.CLIENT.create_bucket)( + new_bucket_name, requester_pays=True + ) + self.case_buckets_to_delete.append(new_bucket_name) + self.assertEqual(created.name, new_bucket_name) + + to_delete = [] + blob = storage.Blob("simple", bucket=created) + blob.upload_from_string(b"DEADBEEF") + to_delete.append(blob) + try: + dest_bucket = Config.CLIENT.bucket(new_bucket_name) + + new_blob = dest_bucket.copy_blob( + blob, + dest_bucket, + "simple-copy", + if_source_generation_match=blob.metageneration, + ) + to_delete.append(new_blob) + + base_contents = blob.download_as_string() + copied_contents = new_blob.download_as_string() + self.assertEqual(base_contents, copied_contents) + finally: + for blob in to_delete: + retry_429_harder(blob.delete)() + + def test_copy_file_with_metageneration_match(self): + new_bucket_name = "generation-match" + unique_resource_id("-") created = retry_429_503(Config.CLIENT.create_bucket)( new_bucket_name, requester_pays=True ) @@ -446,7 +476,6 @@ def test_copy_existing_file_with_generation_match(self): blob, dest_bucket, "simple-copy", - if_source_generation_match=blob.generation, if_source_metageneration_match=blob.metageneration, ) to_delete.append(new_blob) @@ -619,6 +648,69 @@ def test_crud_blob_w_user_project(self): blob1.delete() + def test_crud_blob_w_generation_match(self): + WRONG_GENERATION_NUMBER = 6 + WRONG_METAGENERATION_NUMBER = 9 + + bucket = Config.CLIENT.bucket(self.bucket.name) + blob = bucket.blob("SmallFile") + + file_data = self.FILES["simple"] + with open(file_data["path"], mode="rb") as to_read: + file_contents = to_read.read() + + blob.upload_from_filename(file_data["path"]) + gen0 = blob.generation + + # Upload a second generation of the blob + blob.upload_from_string(b"gen1") + gen1 = blob.generation + + blob0 = bucket.blob("SmallFile", generation=gen0) + blob1 = bucket.blob("SmallFile", generation=gen1) + + try: + # Exercise 'objects.get' (metadata) w/ generation_match. + with self.assertRaises(google.api_core.exceptions.PreconditionFailed): + blob.exists(if_generation_match=WRONG_GENERATION_NUMBER) + + self.assertTrue(blob.exists(if_generation_match=gen1)) + + with self.assertRaises(google.api_core.exceptions.PreconditionFailed): + blob.reload(if_metageneration_match=WRONG_METAGENERATION_NUMBER) + + blob.reload(if_generation_match=gen1) + + # # Exercise 'objects.get' (media) w/ generation match. + self.assertEqual( + blob0.download_as_string(if_generation_match=gen0), file_contents + ) + self.assertEqual( + blob1.download_as_string(if_generation_not_match=gen0), b"gen1" + ) + + # # Exercise 'objects.patch' w/ generation match. + blob0.content_language = "en" + blob0.patch(if_generation_match=gen0) + + self.assertEqual(blob0.content_language, "en") + self.assertIsNone(blob1.content_language) + + # # Exercise 'objects.update' w/ generation match. + metadata = {"foo": "Foo", "bar": "Bar"} + blob0.metadata = metadata + blob0.update(if_generation_match=gen0) + + self.assertEqual(blob0.metadata, metadata) + self.assertIsNone(blob1.metadata) + finally: + # Exercise 'objects.delete' (metadata) w/ generation match. + with self.assertRaises(google.api_core.exceptions.PreconditionFailed): + blob0.delete(if_metageneration_match=WRONG_METAGENERATION_NUMBER) + + blob0.delete(if_generation_match=gen0) + blob1.delete(if_metageneration_not_match=WRONG_METAGENERATION_NUMBER) + @unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.") def test_blob_acl_w_user_project(self): with_user_project = Config.CLIENT.bucket( @@ -693,6 +785,34 @@ def test_direct_write_and_read_into_file(self): self.assertEqual(file_contents, stored_contents) + def test_download_w_generation_match(self): + WRONG_GENERATION_NUMBER = 6 + + blob = self.bucket.blob("MyBuffer") + file_contents = b"Hello World" + blob.upload_from_string(file_contents) + self.case_blobs_to_delete.append(blob) + + same_blob = self.bucket.blob("MyBuffer") + same_blob.reload() # Initialize properties. + temp_filename = tempfile.mktemp() + with open(temp_filename, "wb") as file_obj: + with self.assertRaises(google.api_core.exceptions.PreconditionFailed): + same_blob.download_to_file( + file_obj, if_generation_match=WRONG_GENERATION_NUMBER + ) + + same_blob.download_to_file( + file_obj, + if_generation_match=blob.generation, + if_metageneration_match=blob.metageneration, + ) + + with open(temp_filename, "rb") as file_obj: + stored_contents = file_obj.read() + + self.assertEqual(file_contents, stored_contents) + def test_copy_existing_file(self): filename = self.FILES["logo"]["path"] blob = storage.Blob("CloudLogo", bucket=self.bucket) @@ -1441,6 +1561,43 @@ def test_rewrite_rotate_with_user_project(self): finally: retry_429_harder(created.delete)(force=True) + def test_rewrite_with_generation_match(self): + BLOB_NAME = "rotating-keys" + file_data = self.FILES["simple"] + new_bucket_name = "rewrite-generation-match" # + unique_resource_id("-") + created = Config.CLIENT.create_bucket(new_bucket_name, requester_pays=True) + try: + with_user_project = Config.CLIENT.bucket( + new_bucket_name, user_project=USER_PROJECT + ) + + SOURCE_KEY = os.urandom(32) + source = with_user_project.blob(BLOB_NAME, encryption_key=SOURCE_KEY) + source.upload_from_filename(file_data["path"]) + source_data = source.download_as_string() + + DEST_KEY = os.urandom(32) + dest = with_user_project.blob(BLOB_NAME, encryption_key=DEST_KEY) + + with self.assertRaises(google.api_core.exceptions.PreconditionFailed): + token, rewritten, total = dest.rewrite( + source, if_generation_not_match=dest.generation + ) + + token, rewritten, total = dest.rewrite( + source, + if_generation_match=dest.generation, + if_source_generation_match=source.generation, + if_source_metageneration_match=source.metageneration, + ) + self.assertEqual(token, None) + self.assertEqual(rewritten, len(source_data)) + self.assertEqual(total, len(source_data)) + + self.assertEqual(dest.download_as_string(), source_data) + finally: + created.delete(force=True) + class TestStorageUpdateStorageClass(TestStorageFiles): def test_update_storage_class_small_file(self): diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 9663f787f..7f1f8998e 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -126,7 +126,8 @@ def test_reload(self): ) self.assertEqual(derived._changes, set()) - def test_reload_with_metageneration_match(self): + def test_reload_with_generation_match(self): + GENERATION_NUMBER = 9 METAGENERATION_NUMBER = 6 connection = _Connection({"foo": "Foo"}) @@ -136,7 +137,10 @@ def test_reload_with_metageneration_match(self): # (which will clear / replace it with an empty set), checked below. derived._changes = object() derived.reload( - client=client, timeout=42, if_metageneration_match=METAGENERATION_NUMBER + client=client, + timeout=42, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, ) self.assertEqual(derived._properties, {"foo": "Foo"}) kw = connection._requested @@ -148,6 +152,7 @@ def test_reload_with_metageneration_match(self): "path": "/path", "query_params": { "projection": "noAcl", + "ifGenerationMatch": GENERATION_NUMBER, "ifMetagenerationMatch": METAGENERATION_NUMBER, }, "headers": {}, @@ -223,6 +228,7 @@ def test_patch(self): self.assertEqual(derived._changes, set()) def test_patch_with_metageneration_match(self): + GENERATION_NUMBER = 9 METAGENERATION_NUMBER = 6 connection = _Connection({"foo": "Foo"}) @@ -234,7 +240,10 @@ def test_patch_with_metageneration_match(self): derived._properties = {"bar": BAR, "baz": BAZ} derived._changes = set(["bar"]) # Ignore baz. derived.patch( - client=client, timeout=42, if_metageneration_match=METAGENERATION_NUMBER + client=client, + timeout=42, + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, ) self.assertEqual(derived._properties, {"foo": "Foo"}) kw = connection._requested @@ -246,6 +255,7 @@ def test_patch_with_metageneration_match(self): "path": "/path", "query_params": { "projection": "full", + "ifGenerationMatch": GENERATION_NUMBER, "ifMetagenerationMatch": METAGENERATION_NUMBER, }, # Since changes does not include `baz`, we don't see it sent. diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index f9ea4f21c..94822b93a 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -709,7 +709,8 @@ def test_exists_hit_w_generation(self): def test_exists_w_generation_match(self): BLOB_NAME = "blob-name" - GENERATION = 123456 + GENERATION_NUMBER = 123456 + METAGENERATION_NUMBER = 6 found_response = ({"status": http_client.OK}, b"") connection = _Connection(found_response) @@ -717,14 +718,23 @@ def test_exists_w_generation_match(self): bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) bucket._blobs[BLOB_NAME] = 1 - self.assertTrue(blob.exists(if_generation_match=GENERATION)) + self.assertTrue( + blob.exists( + if_generation_match=GENERATION_NUMBER, + if_metageneration_match=METAGENERATION_NUMBER, + ) + ) self.assertEqual(len(connection._requested), 1) self.assertEqual( connection._requested[0], { "method": "GET", "path": "/b/name/o/{}".format(BLOB_NAME), - "query_params": {"fields": "name", "ifGenerationMatch": GENERATION}, + "query_params": { + "fields": "name", + "ifGenerationMatch": GENERATION_NUMBER, + "ifMetagenerationMatch": METAGENERATION_NUMBER, + }, "_target_object": None, "timeout": self._get_default_timeout(), }, From e9748bb0620c2bae36f403092333d614f4328a17 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 14 May 2020 14:46:12 +0300 Subject: [PATCH 12/15] system tests fixes --- tests/system/test_system.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 30973cd0f..a836603a5 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -446,7 +446,7 @@ def test_copy_file_with_generation_match(self): blob, dest_bucket, "simple-copy", - if_source_generation_match=blob.metageneration, + if_source_generation_match=blob.generation, ) to_delete.append(new_blob) @@ -1564,7 +1564,7 @@ def test_rewrite_rotate_with_user_project(self): def test_rewrite_with_generation_match(self): BLOB_NAME = "rotating-keys" file_data = self.FILES["simple"] - new_bucket_name = "rewrite-generation-match" # + unique_resource_id("-") + new_bucket_name = "rewrite-generation-match" + unique_resource_id("-") created = Config.CLIENT.create_bucket(new_bucket_name, requester_pays=True) try: with_user_project = Config.CLIENT.bucket( From 3e830913035542474334cc7c22c3c9263e7ad474 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 14 May 2020 15:15:11 +0300 Subject: [PATCH 13/15] cleanup system test --- tests/system/test_system.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index a836603a5..d73f1c7da 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -1562,26 +1562,26 @@ def test_rewrite_rotate_with_user_project(self): retry_429_harder(created.delete)(force=True) def test_rewrite_with_generation_match(self): - BLOB_NAME = "rotating-keys" + WRONG_GENERATION_NUMBER = 6 + BLOB_NAME = "generation-match" + file_data = self.FILES["simple"] new_bucket_name = "rewrite-generation-match" + unique_resource_id("-") - created = Config.CLIENT.create_bucket(new_bucket_name, requester_pays=True) + created = Config.CLIENT.create_bucket(new_bucket_name) try: with_user_project = Config.CLIENT.bucket( new_bucket_name, user_project=USER_PROJECT ) - SOURCE_KEY = os.urandom(32) - source = with_user_project.blob(BLOB_NAME, encryption_key=SOURCE_KEY) + source = with_user_project.blob(BLOB_NAME) source.upload_from_filename(file_data["path"]) source_data = source.download_as_string() - DEST_KEY = os.urandom(32) - dest = with_user_project.blob(BLOB_NAME, encryption_key=DEST_KEY) + dest = with_user_project.blob(BLOB_NAME) with self.assertRaises(google.api_core.exceptions.PreconditionFailed): token, rewritten, total = dest.rewrite( - source, if_generation_not_match=dest.generation + source, if_generation_match=WRONG_GENERATION_NUMBER ) token, rewritten, total = dest.rewrite( From 0dbd9a9593db616e5b85f9c993556272a5da4940 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 14 May 2020 15:36:01 +0300 Subject: [PATCH 14/15] fix comments --- google/cloud/storage/_helpers.py | 4 ++++ tests/system/test_system.py | 21 +++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index a70528fe7..dee5cbc42 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -345,6 +345,10 @@ def update( :type if_metageneration_match: long :param if_metageneration_match: (Optional) Make the operation conditional on whether the blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. """ client = self._require_client(client) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index d73f1c7da..6ca87edb1 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -670,7 +670,7 @@ def test_crud_blob_w_generation_match(self): blob1 = bucket.blob("SmallFile", generation=gen1) try: - # Exercise 'objects.get' (metadata) w/ generation_match. + # Exercise 'objects.get' (metadata) w/ generation match. with self.assertRaises(google.api_core.exceptions.PreconditionFailed): blob.exists(if_generation_match=WRONG_GENERATION_NUMBER) @@ -681,7 +681,7 @@ def test_crud_blob_w_generation_match(self): blob.reload(if_generation_match=gen1) - # # Exercise 'objects.get' (media) w/ generation match. + # Exercise 'objects.get' (media) w/ generation match. self.assertEqual( blob0.download_as_string(if_generation_match=gen0), file_contents ) @@ -689,14 +689,14 @@ def test_crud_blob_w_generation_match(self): blob1.download_as_string(if_generation_not_match=gen0), b"gen1" ) - # # Exercise 'objects.patch' w/ generation match. + # Exercise 'objects.patch' w/ generation match. blob0.content_language = "en" blob0.patch(if_generation_match=gen0) self.assertEqual(blob0.content_language, "en") self.assertIsNone(blob1.content_language) - # # Exercise 'objects.update' w/ generation match. + # Exercise 'objects.update' w/ generation match. metadata = {"foo": "Foo", "bar": "Bar"} blob0.metadata = metadata blob0.update(if_generation_match=gen0) @@ -1567,17 +1567,15 @@ def test_rewrite_with_generation_match(self): file_data = self.FILES["simple"] new_bucket_name = "rewrite-generation-match" + unique_resource_id("-") - created = Config.CLIENT.create_bucket(new_bucket_name) + created = retry_429_503(Config.CLIENT.create_bucket)(new_bucket_name) try: - with_user_project = Config.CLIENT.bucket( - new_bucket_name, user_project=USER_PROJECT - ) + bucket = Config.CLIENT.bucket(new_bucket_name) - source = with_user_project.blob(BLOB_NAME) + source = bucket.blob(BLOB_NAME) source.upload_from_filename(file_data["path"]) source_data = source.download_as_string() - dest = with_user_project.blob(BLOB_NAME) + dest = bucket.blob(BLOB_NAME) with self.assertRaises(google.api_core.exceptions.PreconditionFailed): token, rewritten, total = dest.rewrite( @@ -1593,10 +1591,9 @@ def test_rewrite_with_generation_match(self): self.assertEqual(token, None) self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(dest.download_as_string(), source_data) finally: - created.delete(force=True) + retry_429_harder(created.delete)(force=True) class TestStorageUpdateStorageClass(TestStorageFiles): From dc0bb4f525f3dfff465b200499d57aaa7bcc9f15 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 15 May 2020 01:17:52 +0300 Subject: [PATCH 15/15] delete excess checks --- google/cloud/storage/blob.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 817c026a6..9567c1096 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -1753,16 +1753,6 @@ def upload_from_file( if num_retries is not None: warnings.warn(_NUM_RETRIES_MESSAGE, DeprecationWarning, stacklevel=2) - _raise_if_more_than_one_set( - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - ) - - _raise_if_more_than_one_set( - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - _maybe_rewind(file_obj, rewind=rewind) predefined_acl = ACL.validate_predefined(predefined_acl)