From 741d3fda4e4280022cede29ebeb7c2ea09e73b6f Mon Sep 17 00:00:00 2001 From: Danielle Hanks <41087581+daniellehanks@users.noreply.github.com> Date: Thu, 8 Jul 2021 10:12:45 -0600 Subject: [PATCH] feat: add support for Etag headers on reads (#489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Support conditional requests based on ETag for read operations (`reload`, `exists`, `download_*`). My own testing seems to indicate that the JSON API does not support ETag If-Match/If-None-Match headers on modify requests (`patch`, `delete`, etc.), please correct me if I am mistaken. This part two of #451. Part one in https://github.com/googleapis/python-storage/pull/488. Fixes #451 🦕 --- docs/generation_metageneration.rst | 48 ++++++- google/cloud/storage/_helpers.py | 42 +++++- google/cloud/storage/blob.py | 78 +++++++++++ google/cloud/storage/bucket.py | 50 ++++++- google/cloud/storage/client.py | 20 ++- tests/system/test_blob.py | 46 +++++++ tests/system/test_client.py | 41 ++++++ tests/unit/test__helpers.py | 60 +++++++++ tests/unit/test_blob.py | 202 ++++++++++++++++++++++++++++- tests/unit/test_bucket.py | 92 +++++++++++++ tests/unit/test_client.py | 38 ++++++ 11 files changed, 701 insertions(+), 16 deletions(-) diff --git a/docs/generation_metageneration.rst b/docs/generation_metageneration.rst index 287e6573a..4a92e534a 100644 --- a/docs/generation_metageneration.rst +++ b/docs/generation_metageneration.rst @@ -1,10 +1,10 @@ -Conditional Requests Via Generation / Metageneration Preconditions -================================================================== +Conditional Requests Via ETag / Generation / Metageneration Preconditions +========================================================================= Preconditions tell Cloud Storage to only perform a request if the -:ref:`generation ` or +:ref:`ETag `, :ref:`generation `, or :ref:`metageneration ` number of the affected object -meets your precondition criteria. These checks of the generation and +meets your precondition criteria. These checks of the ETag, generation, and metageneration numbers ensure that the object is in the expected state, allowing you to perform safe read-modify-write updates and conditional operations on objects @@ -12,6 +12,20 @@ operations on objects Concepts -------- +.. _concept-etag: + +ETag +:::::::::::::: + +An ETag is returned as part of the response header whenever a resource is +returned, as well as included in the resource itself. Users should make no +assumptions about the value used in an ETag except that it changes whenever the +underlying data changes, per the +`specification `_ + +The ``ETag`` attribute is set by the GCS back-end, and is read-only in the +client library. + .. _concept-metageneration: Metageneration @@ -59,6 +73,32 @@ See also Conditional Parameters ---------------------- +.. _using-if-etag-match: + +Using ``if_etag_match`` +::::::::::::::::::::::::::::: + +Passing the ``if_etag_match`` parameter to a method which retrieves a +blob resource (e.g., +:meth:`Blob.reload `) +makes the operation conditional on whether the blob's current ``ETag`` matches +the given value. This parameter is not supported for modification (e.g., +:meth:`Blob.update `). + + +.. _using-if-etag-not-match: + +Using ``if_etag_not_match`` +::::::::::::::::::::::::::::: + +Passing the ``if_etag_not_match`` parameter to a method which retrieves a +blob resource (e.g., +:meth:`Blob.reload `) +makes the operation conditional on whether the blob's current ``ETag`` matches +the given value. This parameter is not supported for modification (e.g., +:meth:`Blob.update `). + + .. _using-if-generation-match: Using ``if_generation_match`` diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index ff5767de7..68aee0a0c 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -22,6 +22,7 @@ from datetime import datetime import os +from six import string_types from six.moves.urllib.parse import urlsplit from google import resumable_media from google.cloud.storage.constants import _DEFAULT_TIMEOUT @@ -34,6 +35,12 @@ _DEFAULT_STORAGE_HOST = u"https://storage.googleapis.com" +# etag match parameters in snake case and equivalent header +_ETAG_MATCH_PARAMETERS = ( + ("if_etag_match", "If-Match"), + ("if_etag_not_match", "If-None-Match"), +) + # generation match parameters in camel and snake cases _GENERATION_MATCH_PARAMETERS = ( ("if_generation_match", "ifGenerationMatch"), @@ -147,6 +154,8 @@ def reload( self, client=None, projection="noAcl", + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -168,6 +177,12 @@ def reload( Defaults to ``'noAcl'``. Specifies the set of properties to return. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]]) + :param if_etag_not_match: (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -205,10 +220,14 @@ def reload( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) + headers = self._encryption_headers() + _add_etag_match_headers( + headers, if_etag_match=if_etag_match, if_etag_not_match=if_etag_not_match + ) api_response = client._get_resource( self.path, query_params=query_params, - headers=self._encryption_headers(), + headers=headers, timeout=timeout, retry=retry, _target_object=self, @@ -384,8 +403,7 @@ def update( def _scalar_property(fieldname): - """Create a property descriptor around the :class:`_PropertyMixin` helpers. - """ + """Create a property descriptor around the :class:`_PropertyMixin` helpers.""" def _getter(self): """Scalar property getter.""" @@ -449,6 +467,24 @@ def _convert_to_timestamp(value): return mtime +def _add_etag_match_headers(headers, **match_parameters): + """Add generation match parameters into the given parameters list. + + :type headers: dict + :param headers: Headers dict. + + :type match_parameters: dict + :param match_parameters: if*etag*match parameters to add. + """ + for snakecase_name, header_name in _ETAG_MATCH_PARAMETERS: + value = match_parameters.get(snakecase_name) + + if value is not None: + if isinstance(value, string_types): + value = [value] + headers[header_name] = ", ".join(value) + + def _add_generation_match_parameters(parameters, **match_parameters): """Add generation match parameters into the given parameters list. diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index cd5c381d8..1e8829aea 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -59,6 +59,7 @@ from google.cloud._helpers import _rfc3339_nanos_to_datetime from google.cloud._helpers import _to_bytes from google.cloud.exceptions import NotFound +from google.cloud.storage._helpers import _add_etag_match_headers 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 @@ -634,6 +635,8 @@ def generate_signed_url( def exists( self, client=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -651,6 +654,14 @@ def exists( (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -692,12 +703,19 @@ def exists( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) + + headers = {} + _add_etag_match_headers( + headers, if_etag_match=if_etag_match, if_etag_not_match=if_etag_not_match + ) + try: # We intentionally pass `_target_object=None` since fields=name # would limit the local properties. client._get_resource( self.path, query_params=query_params, + headers=headers, timeout=timeout, retry=retry, _target_object=None, @@ -1002,6 +1020,8 @@ def download_to_file( start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1057,6 +1077,14 @@ def download_to_file( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1121,6 +1149,8 @@ def download_to_file( start=start, end=end, raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, @@ -1137,6 +1167,8 @@ def download_to_filename( start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1168,6 +1200,14 @@ def download_to_filename( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1233,6 +1273,8 @@ def download_to_filename( start=start, end=end, raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, @@ -1260,6 +1302,8 @@ def download_as_bytes( start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1288,6 +1332,14 @@ def download_as_bytes( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1355,6 +1407,8 @@ def download_as_bytes( start=start, end=end, raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, @@ -1371,6 +1425,8 @@ def download_as_string( start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1401,6 +1457,14 @@ def download_as_string( :param raw_download: (Optional) If true, download the object without any expansion. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1460,6 +1524,8 @@ def download_as_string( start=start, end=end, raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, @@ -1475,6 +1541,8 @@ def download_as_text( end=None, raw_download=False, encoding=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1507,6 +1575,14 @@ def download_as_text( downloaded bytes. Defaults to the ``charset`` param of attr:`content_type`, or else to "utf-8". + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1558,6 +1634,8 @@ def download_as_text( start=start, end=end, raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 48531fdf3..63b2e9a7b 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -30,6 +30,7 @@ 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_etag_match_headers 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 @@ -752,6 +753,8 @@ def exists( self, client=None, timeout=_DEFAULT_TIMEOUT, + if_etag_match=None, + if_etag_not_match=None, if_metageneration_match=None, if_metageneration_not_match=None, retry=DEFAULT_RETRY, @@ -770,13 +773,21 @@ def exists( (Optional) The amount of time, in seconds, to wait for the server response. See: :ref:`configuring_timeouts` + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: (Optional) Make the operation conditional on whether the + bucket's current ETag matches the given value. + + :type if_etag_not_match: Union[str, Set[str]]) + :param if_etag_not_match: (Optional) Make the operation conditional on whether the + bucket's current ETag does not match the given value. + :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. + bucket'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. + bucket's current metageneration does not match the given value. :type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy :param retry: @@ -798,12 +809,19 @@ def exists( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) + + headers = {} + _add_etag_match_headers( + headers, if_etag_match=if_etag_match, if_etag_not_match=if_etag_not_match + ) + try: # We intentionally pass `_target_object=None` since fields=name # would limit the local properties. client._get_resource( self.path, query_params=query_params, + headers=headers, timeout=timeout, retry=retry, _target_object=None, @@ -941,6 +959,8 @@ def reload( client=None, projection="noAcl", timeout=_DEFAULT_TIMEOUT, + if_etag_match=None, + if_etag_not_match=None, if_metageneration_match=None, if_metageneration_not_match=None, retry=DEFAULT_RETRY, @@ -964,13 +984,21 @@ def reload( (Optional) The amount of time, in seconds, to wait for the server response. See: :ref:`configuring_timeouts` + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: (Optional) Make the operation conditional on whether the + bucket's current ETag matches the given value. + + :type if_etag_not_match: Union[str, Set[str]]) + :param if_etag_not_match: (Optional) Make the operation conditional on whether the + bucket's current ETag does not match the given value. + :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. + bucket'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. + bucket's current metageneration does not match the given value. :type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy :param retry: @@ -980,6 +1008,8 @@ def reload( client=client, projection=projection, timeout=timeout, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, retry=retry, @@ -1074,6 +1104,8 @@ def get_blob( client=None, encryption_key=None, generation=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1111,6 +1143,14 @@ def get_blob( :param generation: (Optional) If present, selects a specific revision of this object. + :type if_etag_match: Union[str, Set[str]] + :param if_etag_match: + (Optional) See :ref:`using-if-etag-match` + + :type if_etag_not_match: Union[str, Set[str]] + :param if_etag_not_match: + (Optional) See :ref:`using-if-etag-not-match` + :type if_generation_match: long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1156,6 +1196,8 @@ def get_blob( blob.reload( client=client, timeout=timeout, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index d6f688d92..18bd7c9cb 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -34,6 +34,7 @@ from google.cloud.storage._helpers import _get_storage_host from google.cloud.storage._helpers import _DEFAULT_STORAGE_HOST from google.cloud.storage._helpers import _bucket_bound_hostname_url +from google.cloud.storage._helpers import _add_etag_match_headers from google.cloud.storage._http import Connection from google.cloud.storage._signing import ( get_expiration_seconds_v4, @@ -967,6 +968,8 @@ def download_blob_to_file( start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -996,16 +999,22 @@ def download_blob_to_file( raw_download (bool): (Optional) If true, download the object without any expansion. - if_generation_match: long + if_etag_match (Union[str, Set[str]]): + (Optional) See :ref:`using-if-etag-match` + + if_etag_not_match (Union[str, Set[str]]): + (Optional) See :ref:`using-if-etag-not-match` + + if_generation_match (long): (Optional) See :ref:`using-if-generation-match` - if_generation_not_match: long + if_generation_not_match (long): (Optional) See :ref:`using-if-generation-not-match` - if_metageneration_match: long + if_metageneration_match (long): (Optional) See :ref:`using-if-metageneration-match` - if_metageneration_not_match: long + if_metageneration_not_match (long): (Optional) See :ref:`using-if-metageneration-not-match` timeout ([Union[float, Tuple[float, float]]]): @@ -1091,6 +1100,9 @@ def download_blob_to_file( ) headers = _get_encryption_headers(blob_or_uri._encryption_key) headers["accept-encoding"] = "gzip" + _add_etag_match_headers( + headers, if_etag_match=if_etag_match, if_etag_not_match=if_etag_not_match, + ) transport = self._http try: diff --git a/tests/system/test_blob.py b/tests/system/test_blob.py index 84c7040d5..5d654f648 100644 --- a/tests/system/test_blob.py +++ b/tests/system/test_blob.py @@ -235,6 +235,52 @@ def test_blob_crud_w_user_project( blob1.delete() +def test_blob_crud_w_etag_match( + shared_bucket, blobs_to_delete, file_data, service_account, +): + wrong_etag = "kittens" + + blob = shared_bucket.blob("SmallFile") + + info = file_data["simple"] + with open(info["path"], mode="rb") as to_read: + payload = to_read.read() + + blob.upload_from_filename(info["path"]) + blobs_to_delete.append(blob) + etag = blob.etag + + fresh_blob = shared_bucket.blob("SmallFile") + + # Exercise 'objects.get' (metadata) w/ etag match. + with pytest.raises(exceptions.PreconditionFailed): + fresh_blob.exists(if_etag_match=wrong_etag) + + with pytest.raises(exceptions.NotModified): + fresh_blob.exists(if_etag_not_match=etag) + + assert fresh_blob.exists(if_etag_match=etag) + assert fresh_blob.exists(if_etag_not_match=wrong_etag) + + with pytest.raises(exceptions.PreconditionFailed): + fresh_blob.reload(if_etag_match=wrong_etag) + + with pytest.raises(exceptions.NotModified): + fresh_blob.reload(if_etag_not_match=etag) + + fresh_blob.reload(if_etag_match=etag) # no raise + fresh_blob.reload(if_etag_not_match=wrong_etag) # no raise + + # Exercise 'objects.get' (media) w/ etag match. + assert fresh_blob.download_as_bytes(if_etag_match=etag) == payload + + with pytest.raises(exceptions.PreconditionFailed): + fresh_blob.download_as_bytes(if_etag_match=wrong_etag) + + with pytest.raises(exceptions.NotModified): + fresh_blob.download_as_bytes(if_etag_not_match=etag) + + def test_blob_crud_w_generation_match( shared_bucket, blobs_to_delete, file_data, service_account, ): diff --git a/tests/system/test_client.py b/tests/system/test_client.py index d33450eb7..f531f4bb4 100644 --- a/tests/system/test_client.py +++ b/tests/system/test_client.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import io import re import tempfile @@ -102,3 +103,43 @@ def test_download_blob_to_file_w_uri( stored_contents = file_obj.read() assert stored_contents == payload + + +def test_download_blob_to_file_w_etag( + storage_client, shared_bucket, blobs_to_delete, service_account, +): + filename = "kittens" + blob = shared_bucket.blob(filename) + payload = b"fluffy" + blob.upload_from_string(payload) + blobs_to_delete.append(blob) + + buffer = io.BytesIO() + with pytest.raises(exceptions.NotModified): + storage_client.download_blob_to_file( + "gs://" + shared_bucket.name + "/" + filename, + buffer, + if_etag_not_match=blob.etag, + ) + + buffer = io.BytesIO() + with pytest.raises(exceptions.PreconditionFailed): + storage_client.download_blob_to_file( + "gs://" + shared_bucket.name + "/" + filename, + buffer, + if_etag_match="kittens", + ) + + buffer = io.BytesIO() + storage_client.download_blob_to_file( + "gs://" + shared_bucket.name + "/" + filename, + buffer, + if_etag_not_match="kittens", + ) + assert buffer.getvalue() == payload + + buffer = io.BytesIO() + storage_client.download_blob_to_file( + "gs://" + shared_bucket.name + "/" + filename, buffer, if_etag_match=blob.etag, + ) + assert buffer.getvalue() == payload diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 75a439cf1..b99b78cfd 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -136,6 +136,39 @@ def test_reload_w_defaults(self): _target_object=derived, ) + def test_reload_w_etag_match(self): + etag = "kittens" + path = "/path" + response = {"foo": "Foo"} + client = mock.Mock(spec=["_get_resource"]) + client._get_resource.return_value = response + 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.client = client + + derived.reload(if_etag_match=etag,) + + self.assertEqual(derived._properties, response) + self.assertEqual(derived._changes, set()) + + expected_query_params = { + "projection": "noAcl", + } + # no encryption headers by default + expected_headers = { + "If-Match": etag, + } + client._get_resource.assert_called_once_with( + path, + query_params=expected_query_params, + headers=expected_headers, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=derived, + ) + def test_reload_w_generation_match_w_timeout(self): generation_number = 9 metageneration_number = 6 @@ -521,6 +554,33 @@ def read(self, block_size): self.assertEqual(MD5.hash_obj._blocks, [BYTES_TO_SIGN]) +class Test__add_etag_match_headers(unittest.TestCase): + def _call_fut(self, headers, **match_params): + from google.cloud.storage._helpers import _add_etag_match_headers + + return _add_etag_match_headers(headers, **match_params) + + def test_add_etag_match_parameters_str(self): + ETAG = "kittens" + headers = {"foo": "bar"} + EXPECTED_HEADERS = { + "foo": "bar", + "If-Match": ETAG, + } + self._call_fut(headers, if_etag_match=ETAG) + self.assertEqual(headers, EXPECTED_HEADERS) + + def test_add_generation_match_parameters_list(self): + ETAGS = ["kittens", "fluffy"] + EXPECTED_HEADERS = { + "foo": "bar", + "If-Match": ", ".join(ETAGS), + } + headers = {"foo": "bar"} + self._call_fut(headers, if_etag_match=ETAGS) + self.assertEqual(headers, EXPECTED_HEADERS) + + 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 diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index edfe4efdf..fe318a696 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -702,9 +702,11 @@ def test_exists_miss_w_defaults(self): self.assertFalse(blob.exists()) expected_query_params = {"fields": "name"} + expected_headers = {} client._get_resource.assert_called_once_with( blob.path, query_params=expected_query_params, + headers=expected_headers, timeout=self._get_default_timeout(), retry=DEFAULT_RETRY, _target_object=None, @@ -723,9 +725,11 @@ def test_exists_hit_w_user_project_w_timeout(self): self.assertTrue(blob.exists(timeout=timeout)) expected_query_params = {"fields": "name", "userProject": user_project} + expected_headers = {} client._get_resource.assert_called_once_with( blob.path, query_params=expected_query_params, + headers=expected_headers, timeout=timeout, retry=DEFAULT_RETRY, _target_object=None, @@ -744,14 +748,42 @@ def test_exists_hit_w_generation_w_retry(self): self.assertTrue(blob.exists(retry=retry)) expected_query_params = {"fields": "name", "generation": generation} + expected_headers = {} client._get_resource.assert_called_once_with( blob.path, query_params=expected_query_params, + headers=expected_headers, timeout=self._get_default_timeout(), retry=retry, _target_object=None, ) + def test_exists_w_etag_match(self): + blob_name = "blob-name" + etag = "kittens" + api_response = {"name": blob_name} + client = mock.Mock(spec=["_get_resource"]) + client._get_resource.return_value = api_response + bucket = _Bucket(client) + blob = self._make_one(blob_name, bucket=bucket) + + self.assertTrue(blob.exists(if_etag_match=etag, retry=None,)) + + expected_query_params = { + "fields": "name", + } + expected_headers = { + "If-Match": etag, + } + client._get_resource.assert_called_once_with( + blob.path, + query_params=expected_query_params, + headers=expected_headers, + timeout=self._get_default_timeout(), + retry=None, + _target_object=None, + ) + def test_exists_w_generation_match(self): blob_name = "blob-name" generation_number = 123456 @@ -775,9 +807,11 @@ def test_exists_w_generation_match(self): "ifGenerationMatch": generation_number, "ifMetagenerationMatch": metageneration_number, } + expected_headers = {} client._get_resource.assert_called_once_with( blob.path, query_params=expected_query_params, + headers=expected_headers, timeout=self._get_default_timeout(), retry=None, _target_object=None, @@ -1139,7 +1173,7 @@ def _do_download_helper_wo_chunks( transport = object() file_obj = io.BytesIO() download_url = "http://test.invalid" - headers = {} + headers = extra_kwargs.pop("headers", {}) if raw_download: patch = mock.patch("google.cloud.storage.blob.RawDownload") @@ -1210,20 +1244,48 @@ def _do_download_helper_wo_chunks( def test__do_download_wo_chunks_wo_range_wo_raw(self): self._do_download_helper_wo_chunks(w_range=False, raw_download=False) + def test__do_download_wo_chunks_wo_range_wo_raw_w_headers(self): + self._do_download_helper_wo_chunks( + w_range=False, raw_download=False, headers={"If-Match": "kittens"} + ) + def test__do_download_wo_chunks_wo_range_wo_raw_w_retry(self): self._do_download_helper_wo_chunks( w_range=False, raw_download=False, retry=DEFAULT_RETRY ) + def test__do_download_wo_chunks_wo_range_wo_raw_w_retry_w_headers(self): + self._do_download_helper_wo_chunks( + w_range=False, + raw_download=False, + retry=DEFAULT_RETRY, + headers={"If-Match": "kittens"}, + ) + def test__do_download_wo_chunks_w_range_wo_raw(self): self._do_download_helper_wo_chunks(w_range=True, raw_download=False) + def test__do_download_wo_chunks_w_range_wo_raw_w_headers(self): + self._do_download_helper_wo_chunks( + w_range=True, raw_download=False, headers={"If-Match": "kittens"} + ) + def test__do_download_wo_chunks_wo_range_w_raw(self): self._do_download_helper_wo_chunks(w_range=False, raw_download=True) + def test__do_download_wo_chunks_wo_range_w_raw_w_headers(self): + self._do_download_helper_wo_chunks( + w_range=False, raw_download=True, headers={"If-Match": "kittens"} + ) + def test__do_download_wo_chunks_w_range_w_raw(self): self._do_download_helper_wo_chunks(w_range=True, raw_download=True) + def test__do_download_wo_chunks_w_range_w_raw_w_headers(self): + self._do_download_helper_wo_chunks( + w_range=True, raw_download=True, headers={"If-Match": "kittens"} + ) + def test__do_download_wo_chunks_w_custom_timeout(self): self._do_download_helper_wo_chunks( w_range=False, raw_download=False, timeout=9.58 @@ -1356,6 +1418,8 @@ def test_download_to_file_with_failure(self): file_obj, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1384,6 +1448,34 @@ def test_download_to_file_wo_media_link(self): file_obj, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + raw_download=False, + timeout=expected_timeout, + checksum="md5", + retry=DEFAULT_RETRY, + ) + + def test_download_to_file_w_etag_match(self): + etag = "kittens" + client = self._make_client() + blob = self._make_one("blob-name", bucket=_Bucket(client)) + file_obj = io.BytesIO() + + blob.download_to_file(file_obj, if_etag_not_match=etag) + + expected_timeout = self._get_default_timeout() + client.download_blob_to_file.assert_called_once_with( + blob, + file_obj, + start=None, + end=None, + if_etag_match=None, + if_etag_not_match=etag, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1408,6 +1500,8 @@ def test_download_to_file_w_generation_match(self): file_obj, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=generation_number, if_metageneration_match=None, @@ -1452,6 +1546,8 @@ def _download_to_file_helper( file_obj, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1530,6 +1626,8 @@ def _download_to_filename_helper( mock.ANY, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1567,6 +1665,36 @@ def test_download_to_filename_w_custom_timeout(self): updated=None, raw_download=False, timeout=9.58 ) + def test_download_to_filename_w_etag_match(self): + from google.cloud._testing import _NamedTemporaryFile + + etag = "kittens" + client = self._make_client() + blob = self._make_one("blob-name", bucket=_Bucket(client)) + + with _NamedTemporaryFile() as temp: + blob.download_to_filename(temp.name, if_etag_match=etag) + + expected_timeout = self._get_default_timeout() + client.download_blob_to_file.assert_called_once_with( + blob, + mock.ANY, + start=None, + end=None, + if_etag_match=etag, + if_etag_not_match=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + raw_download=False, + timeout=expected_timeout, + checksum="md5", + retry=DEFAULT_RETRY, + ) + stream = client.download_blob_to_file.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 @@ -1583,6 +1711,8 @@ def test_download_to_filename_w_generation_match(self): mock.ANY, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=generation_number, if_generation_not_match=None, if_metageneration_match=None, @@ -1623,6 +1753,8 @@ def test_download_to_filename_corrupted(self): mock.ANY, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1658,6 +1790,8 @@ def _download_as_bytes_helper(self, raw_download, timeout=None, **extra_kwargs): mock.ANY, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1673,6 +1807,36 @@ def _download_as_bytes_helper(self, raw_download, timeout=None, **extra_kwargs): def test_download_as_bytes_w_custom_timeout(self): self._download_as_bytes_helper(raw_download=False, timeout=9.58) + def test_download_as_bytes_w_etag_match(self): + ETAG = "kittens" + MEDIA_LINK = "http://example.com/media/" + + client = self._make_client() + blob = self._make_one( + "blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK} + ) + client.download_blob_to_file = mock.Mock() + + fetched = blob.download_as_bytes(if_etag_match=ETAG) + self.assertEqual(fetched, b"") + + client.download_blob_to_file.assert_called_once_with( + blob, + mock.ANY, + start=None, + end=None, + raw_download=False, + if_etag_match=ETAG, + if_etag_not_match=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + checksum="md5", + retry=DEFAULT_RETRY, + ) + def test_download_as_bytes_w_generation_match(self): GENERATION_NUMBER = 6 MEDIA_LINK = "http://example.com/media/" @@ -1692,6 +1856,8 @@ def test_download_as_bytes_w_generation_match(self): start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=GENERATION_NUMBER, if_generation_not_match=None, if_metageneration_match=None, @@ -1719,6 +1885,8 @@ def _download_as_text_helper( client=None, start=None, end=None, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1764,6 +1932,12 @@ def _download_as_text_helper( if encoding is not None: kwargs["encoding"] = encoding + if if_etag_match is not None: + kwargs["if_etag_match"] = if_etag_match + + if if_etag_not_match is not None: + kwargs["if_etag_not_match"] = if_etag_not_match + if if_generation_match is not None: kwargs["if_generation_match"] = if_generation_match @@ -1795,6 +1969,8 @@ def _download_as_text_helper( end=end, raw_download=raw_download, timeout=expected_timeout, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, if_metageneration_match=if_metageneration_match, @@ -1823,6 +1999,26 @@ def test_download_as_text_w_end(self): def test_download_as_text_w_custom_timeout(self): self._download_as_text_helper(raw_download=False, timeout=9.58) + def test_download_as_text_w_if_etag_match_str(self): + self._download_as_text_helper( + raw_download=False, if_etag_match="kittens", + ) + + def test_download_as_text_w_if_etag_match_list(self): + self._download_as_text_helper( + raw_download=False, if_etag_match=["kittens", "fluffy"], + ) + + def test_download_as_text_w_if_etag_not_match_str(self): + self._download_as_text_helper( + raw_download=False, if_etag_not_match="kittens", + ) + + def test_download_as_text_w_if_etag_not_match_list(self): + self._download_as_text_helper( + raw_download=False, if_etag_not_match=["kittens", "fluffy"], + ) + def test_download_as_text_w_if_generation_match(self): self._download_as_text_helper(raw_download=False, if_generation_match=6) @@ -1889,6 +2085,8 @@ def test_download_as_string(self, mock_warn): start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1923,6 +2121,8 @@ def test_download_as_string_no_retry(self): start=None, end=None, raw_download=False, + if_etag_match=None, + if_etag_not_match=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 4f2932865..e3b770763 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -747,9 +747,36 @@ def test_exists_miss_w_defaults(self): self.assertFalse(bucket.exists()) expected_query_params = {"fields": "name"} + expected_headers = {} + client._get_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + headers=expected_headers, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test_exists_w_etag_match(self): + bucket_name = "bucket-name" + etag = "kittens" + api_response = {"name": bucket_name} + client = mock.Mock(spec=["_get_resource"]) + client._get_resource.return_value = api_response + bucket = self._make_one(client, name=bucket_name) + + self.assertTrue(bucket.exists(if_etag_match=etag)) + + expected_query_params = { + "fields": "name", + } + expected_headers = { + "If-Match": etag, + } client._get_resource.assert_called_once_with( bucket.path, query_params=expected_query_params, + headers=expected_headers, timeout=self._get_default_timeout(), retry=DEFAULT_RETRY, _target_object=None, @@ -772,9 +799,11 @@ def test_exists_w_metageneration_match_w_timeout(self): "fields": "name", "ifMetagenerationMatch": metageneration_number, } + expected_headers = {} client._get_resource.assert_called_once_with( bucket.path, query_params=expected_query_params, + headers=expected_headers, timeout=timeout, retry=DEFAULT_RETRY, _target_object=None, @@ -795,9 +824,11 @@ def test_exists_hit_w_user_project_w_retry_w_explicit_client(self): "fields": "name", "userProject": user_project, } + expected_headers = {} client._get_resource.assert_called_once_with( bucket.path, query_params=expected_query_params, + headers=expected_headers, timeout=self._get_default_timeout(), retry=retry, _target_object=None, @@ -925,6 +956,41 @@ def test_get_blob_hit_w_generation_w_timeout(self): _target_object=blob, ) + def test_get_blob_w_etag_match_w_retry(self): + from google.cloud.storage.blob import Blob + + name = "name" + blob_name = "blob-name" + etag = "kittens" + retry = mock.Mock(spec=[]) + api_response = {"name": blob_name, "etag": etag} + client = mock.Mock(spec=["_get_resource"]) + client._get_resource.return_value = api_response + bucket = self._make_one(client, name=name) + + blob = bucket.get_blob(blob_name, if_etag_match=etag, retry=retry) + + self.assertIsInstance(blob, Blob) + self.assertIs(blob.bucket, bucket) + self.assertEqual(blob.name, blob_name) + self.assertEqual(blob.etag, etag) + + expected_path = "/b/%s/o/%s" % (name, blob_name) + expected_query_params = { + "projection": "noAcl", + } + expected_headers = { + "If-Match": etag, + } + client._get_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + headers=expected_headers, + timeout=self._get_default_timeout(), + retry=retry, + _target_object=blob, + ) + def test_get_blob_w_generation_match_w_retry(self): from google.cloud.storage.blob import Blob @@ -1677,6 +1743,32 @@ def test_delete_blobs_miss_w_on_error(self): ) bucket.delete_blob.assert_has_calls([call_1, call_2]) + def test_reload_w_etag_match(self): + name = "name" + etag = "kittens" + api_response = {"name": name} + client = mock.Mock(spec=["_get_resource"]) + client._get_resource.return_value = api_response + bucket = self._make_one(client, name=name) + + bucket.reload(if_etag_match=etag) + + expected_path = "/b/%s" % (name,) + expected_query_params = { + "projection": "noAcl", + } + expected_headers = { + "If-Match": etag, + } + client._get_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + headers=expected_headers, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=bucket, + ) + def test_reload_w_metageneration_match(self): name = "name" metageneration_number = 9 diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 33ec331d6..cc5c96bbc 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -20,6 +20,7 @@ import re import requests import unittest +from six import string_types from six.moves import http_client from six.moves.urllib import parse as urlparse @@ -1451,6 +1452,32 @@ def test_download_blob_to_file_w_no_retry(self): use_chunks=True, raw_download=True, retry=None ) + def test_download_blob_to_file_w_conditional_etag_match_string(self): + self._download_blob_to_file_helper( + use_chunks=True, raw_download=True, retry=None, if_etag_match="kittens", + ) + + def test_download_blob_to_file_w_conditional_etag_match_list(self): + self._download_blob_to_file_helper( + use_chunks=True, + raw_download=True, + retry=None, + if_etag_match=["kittens", "fluffy"], + ) + + def test_download_blob_to_file_w_conditional_etag_not_match_string(self): + self._download_blob_to_file_helper( + use_chunks=True, raw_download=True, retry=None, if_etag_not_match="kittens", + ) + + def test_download_blob_to_file_w_conditional_etag_not_match_list(self): + self._download_blob_to_file_helper( + use_chunks=True, + raw_download=True, + retry=None, + if_etag_not_match=["kittens", "fluffy"], + ) + def test_download_blob_to_file_w_conditional_retry_pass(self): self._download_blob_to_file_helper( use_chunks=True, @@ -1502,6 +1529,17 @@ def _download_blob_to_file_helper( expected_retry = None headers = {"accept-encoding": "gzip"} + if_etag_match = extra_kwargs.get("if_etag_match") + if if_etag_match is not None: + if isinstance(if_etag_match, string_types): + if_etag_match = [if_etag_match] + headers["If-Match"] = ", ".join(if_etag_match) + if_etag_not_match = extra_kwargs.get("if_etag_not_match") + if if_etag_not_match is not None: + if isinstance(if_etag_not_match, string_types): + if_etag_not_match = [if_etag_not_match] + headers["If-None-Match"] = ", ".join(if_etag_not_match) + blob._do_download.assert_called_once_with( client._http, file_obj,