diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 651652619..c019b2f12 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1466,9 +1466,9 @@ def delete( self, force=False, client=None, - timeout=_DEFAULT_TIMEOUT, if_metageneration_match=None, if_metageneration_not_match=None, + timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY, ): """Delete this bucket. @@ -1496,13 +1496,6 @@ def delete( :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. - - 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. @@ -1511,6 +1504,13 @@ def delete( :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the blob's current metageneration does not match the given value. + :type timeout: float or tuple + :param timeout: (Optional) The amount of time, in seconds, to wait + for the server response on each request. + + Can also be passed as a tuple (connect_timeout, read_timeout). + See :meth:`requests.Session.request` documentation for details. + :type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy :param retry: (Optional) How to retry the RPC. A None value will disable retries. A google.api_core.retry.Retry value will enable retries, and the object will @@ -1545,6 +1545,7 @@ def delete( max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, client=client, timeout=timeout, + retry=retry, ) ) if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: @@ -1558,19 +1559,22 @@ def delete( # Ignore 404 errors on delete. self.delete_blobs( - blobs, on_error=lambda blob: None, client=client, timeout=timeout + blobs, + on_error=lambda blob: None, + client=client, + timeout=timeout, + retry=retry, ) # 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=self.path, + client._delete_resource( + self.path, query_params=query_params, - _target_object=None, timeout=timeout, retry=retry, + _target_object=None, ) def delete_blob( @@ -1677,13 +1681,12 @@ def delete_blob( # 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, + client._delete_resource( + blob.path, query_params=query_params, - _target_object=None, timeout=timeout, retry=retry, + _target_object=None, ) def delete_blobs( @@ -1802,11 +1805,11 @@ def delete_blobs( self.delete_blob( blob_name, client=client, - timeout=timeout, if_generation_match=next(if_generation_match, None), if_generation_not_match=next(if_generation_not_match, None), if_metageneration_match=next(if_metageneration_match, None), if_metageneration_not_match=next(if_metageneration_not_match, None), + timeout=timeout, retry=retry, ) except NotFound: diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index 51527c1ef..effcddddd 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -528,6 +528,72 @@ def _put_resource( _target_object=_target_object, ) + def _delete_resource( + self, + path, + query_params=None, + headers=None, + timeout=_DEFAULT_TIMEOUT, + retry=DEFAULT_RETRY, + _target_object=None, + ): + """Helper for bucket / blob methods making API 'DELETE' calls. + + Args: + path str: + The path of the resource to delete. + + query_params Optional[dict]: + HTTP query parameters to be passed + + headers Optional[dict]: + HTTP headers to be passed + + timeout (Optional[Union[float, Tuple[float, float]]]): + 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. + + retry (Optional[Union[google.api_core.retry.Retry, google.cloud.storage.retry.ConditionalRetryPolicy]]): + How to retry the RPC. A None value will disable retries. + A google.api_core.retry.Retry value will enable retries, and the object will + define retriable response codes and errors and configure backoff and timeout options. + + A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and + activates it only if certain conditions are met. This class exists to provide safe defaults + for RPC calls that are not technically safe to retry normally (due to potential data + duplication or other side-effects) but become safe to retry if a condition such as + if_metageneration_match is set. + + See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for + information on retry types and how to configure them. + + _target_object (Union[ \ + :class:`~google.cloud.storage.bucket.Bucket`, \ + :class:`~google.cloud.storage.bucket.blob`, \ + ]): + Object to which future data is to be applied -- only relevant + in the context of a batch. + + Returns: + dict + The JSON resource fetched + + Raises: + google.cloud.exceptions.NotFound + If the bucket is not found. + """ + return self._connection.api_request( + method="DELETE", + path=path, + query_params=query_params, + headers=headers, + timeout=timeout, + retry=retry, + _target_object=_target_object, + ) + def get_bucket( self, bucket_or_name, diff --git a/google/cloud/storage/hmac_key.py b/google/cloud/storage/hmac_key.py index ad1e50562..e59960a1c 100644 --- a/google/cloud/storage/hmac_key.py +++ b/google/cloud/storage/hmac_key.py @@ -336,10 +336,6 @@ def delete(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): if self.user_project is not None: qs_params["userProject"] = self.user_project - self._client._connection.api_request( - method="DELETE", - path=self.path, - query_params=qs_params, - timeout=timeout, - retry=retry, + self._client._delete_resource( + self.path, query_params=qs_params, timeout=timeout, retry=retry, ) diff --git a/google/cloud/storage/notification.py b/google/cloud/storage/notification.py index 5389ab51e..2f5661fce 100644 --- a/google/cloud/storage/notification.py +++ b/google/cloud/storage/notification.py @@ -429,12 +429,8 @@ def delete(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): if self.bucket.user_project is not None: query_params["userProject"] = self.bucket.user_project - client._connection.api_request( - method="DELETE", - path=self.path, - query_params=query_params, - timeout=timeout, - retry=retry, + client._delete_resource( + self.path, query_params=query_params, timeout=timeout, retry=retry, ) diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index c0ac843cf..410c9d9b6 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1107,358 +1107,472 @@ def test_get_notification_hit_w_explicit_w_user_project(self): retry=retry, ) - def test_delete_miss(self): + def test_delete_miss_w_defaults(self): from google.cloud.exceptions import NotFound - NAME = "name" - connection = _Connection() - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - self.assertRaises(NotFound, bucket.delete) - expected_cw = [ - { - "method": "DELETE", - "path": bucket.path, - "query_params": {}, - "_target_object": None, - "timeout": self._get_default_timeout(), - "retry": DEFAULT_RETRY, - } - ] - self.assertEqual(connection._deleted_buckets, expected_cw) + name = "name" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.side_effect = NotFound("testing") + bucket = self._make_one(client=client, name=name) + + with self.assertRaises(NotFound): + bucket.delete() + + expected_query_params = {} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test_delete_hit_w_metageneration_match_w_explicit_client(self): + name = "name" + metageneration_number = 6 + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=None, name=name) + + result = bucket.delete( + client=client, if_metageneration_match=metageneration_number, + ) - def test_delete_hit_with_user_project(self): - NAME = "name" - USER_PROJECT = "user-project-123" - GET_BLOBS_RESP = {"items": []} - connection = _Connection(GET_BLOBS_RESP) - connection._delete_bucket = True - client = self._make_client() - client._base_connection = connection - bucket = self._make_one(client=client, name=NAME, user_project=USER_PROJECT) - result = bucket.delete(force=True, timeout=42) self.assertIsNone(result) - expected_cw = [ - { - "method": "DELETE", - "path": bucket.path, - "_target_object": None, - "query_params": {"userProject": USER_PROJECT}, - "timeout": 42, - "retry": DEFAULT_RETRY, - } - ] - self.assertEqual(connection._deleted_buckets, expected_cw) - def test_delete_force_delete_blobs(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 = {} - connection = _Connection(GET_BLOBS_RESP, DELETE_BLOB1_RESP, DELETE_BLOB2_RESP) - connection._delete_bucket = True - client = self._make_client() - client._base_connection = connection - bucket = self._make_one(client=client, name=NAME) - result = bucket.delete(force=True) + expected_query_params = {"ifMetagenerationMatch": metageneration_number} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test_delete_hit_w_force_w_user_project_w_explicit_timeout_retry(self): + name = "name" + user_project = "user-project-123" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name, user_project=user_project) + bucket.list_blobs = mock.Mock(return_value=iter([])) + bucket.delete_blobs = mock.Mock(return_value=None) + timeout = 42 + retry = mock.Mock(spec=[]) + + result = bucket.delete(force=True, timeout=timeout, retry=retry) + self.assertIsNone(result) - expected_cw = [ - { - "method": "DELETE", - "path": bucket.path, - "query_params": {}, - "_target_object": None, - "timeout": self._get_default_timeout(), - "retry": DEFAULT_RETRY, - } - ] - 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) + bucket.list_blobs.assert_called_once_with( + max_results=bucket._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=timeout, + retry=retry, + ) + + bucket.delete_blobs.assert_called_once_with( + [], on_error=mock.ANY, client=client, timeout=timeout, retry=retry, + ) + + expected_query_params = {"userProject": user_project} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=timeout, + retry=retry, + _target_object=None, + ) + + def test_delete_hit_w_force_delete_blobs(self): + name = "name" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) + blobs = [mock.Mock(spec=[]), mock.Mock(spec=[])] + bucket.list_blobs = mock.Mock(return_value=iter(blobs)) + bucket.delete_blobs = mock.Mock(return_value=None) + + result = bucket.delete(force=True) + self.assertIsNone(result) - expected_cw = [ - { - "method": "DELETE", - "path": bucket.path, - "query_params": {"ifMetagenerationMatch": METAGENERATION_NUMBER}, - "_target_object": None, - "timeout": self._get_default_timeout(), - "retry": DEFAULT_RETRY, - } - ] - self.assertEqual(connection._deleted_buckets, expected_cw) - def test_delete_force_miss_blobs(self): - NAME = "name" - BLOB_NAME = "blob-name1" - GET_BLOBS_RESP = {"items": [{"name": BLOB_NAME}]} - # Note the connection does not have a response for the blob. - connection = _Connection(GET_BLOBS_RESP) - connection._delete_bucket = True - client = self._make_client() - client._base_connection = connection - bucket = self._make_one(client=client, name=NAME) + bucket.list_blobs.assert_called_once_with( + max_results=bucket._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + bucket.delete_blobs.assert_called_once_with( + blobs, + on_error=mock.ANY, + client=client, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + expected_query_params = {} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test_delete_w_force_w_user_project_w_miss_on_blob(self): + from google.cloud.exceptions import NotFound + + name = "name" + blob_name = "blob-name" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) + blob = mock.Mock(spec=["name"]) + blob.name = blob_name + blobs = [blob] + bucket.list_blobs = mock.Mock(return_value=iter(blobs)) + bucket.delete_blob = mock.Mock(side_effect=NotFound("testing")) + result = bucket.delete(force=True) + self.assertIsNone(result) - expected_cw = [ - { - "method": "DELETE", - "path": bucket.path, - "query_params": {}, - "_target_object": None, - "timeout": self._get_default_timeout(), - "retry": DEFAULT_RETRY, - } - ] - self.assertEqual(connection._deleted_buckets, expected_cw) - def test_delete_too_many(self): - NAME = "name" - BLOB_NAME1 = "blob-name1" - BLOB_NAME2 = "blob-name2" - GET_BLOBS_RESP = {"items": [{"name": BLOB_NAME1}, {"name": BLOB_NAME2}]} - connection = _Connection(GET_BLOBS_RESP) - connection._delete_bucket = True - client = self._make_client() - client._base_connection = connection - bucket = self._make_one(client=client, name=NAME) + bucket.delete_blob.assert_called_once_with( + blob_name, + client=client, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + expected_query_params = {} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + def test_delete_w_too_many(self): + name = "name" + blob_name1 = "blob-name1" + blob_name2 = "blob-name2" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) + blob1 = mock.Mock(spec=["name"]) + blob1.name = blob_name1 + blob2 = mock.Mock(spec=["name"]) + blob2.name = blob_name2 + blobs = [blob1, blob2] + bucket.list_blobs = mock.Mock(return_value=iter(blobs)) + bucket.delete_blobs = mock.Mock() # Make the Bucket refuse to delete with 2 objects. bucket._MAX_OBJECTS_FOR_ITERATION = 1 - self.assertRaises(ValueError, bucket.delete, force=True) - self.assertEqual(connection._deleted_buckets, []) - def test_delete_blob_miss(self): + with self.assertRaises(ValueError): + bucket.delete(force=True) + + bucket.delete_blobs.assert_not_called() + + def test_delete_blob_miss_w_defaults(self): from google.cloud.exceptions import NotFound - NAME = "name" - NONESUCH = "nonesuch" - connection = _Connection() - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - self.assertRaises(NotFound, bucket.delete_blob, NONESUCH) - (kw,) = connection._requested - self.assertEqual(kw["method"], "DELETE") - self.assertEqual(kw["path"], "/b/%s/o/%s" % (NAME, NONESUCH)) - self.assertEqual(kw["query_params"], {}) - self.assertEqual(kw["timeout"], self._get_default_timeout()) + name = "name" + blob_name = "nonesuch" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.side_effect = NotFound("testing") + bucket = self._make_one(client=client, name=name) + + with self.assertRaises(NotFound): + bucket.delete_blob(blob_name) + + expected_path = "/b/%s/o/%s" % (name, blob_name) + expected_query_params = {} + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + _target_object=None, + ) + + def test_delete_blob_hit_w_user_project_w_timeout(self): + name = "name" + blob_name = "blob-name" + user_project = "user-project-123" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name, user_project=user_project) + timeout = 42 + + result = bucket.delete_blob(blob_name, timeout=timeout) - def test_delete_blob_hit_with_user_project(self): - NAME = "name" - BLOB_NAME = "blob-name" - USER_PROJECT = "user-project-123" - connection = _Connection({}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME, user_project=USER_PROJECT) - result = bucket.delete_blob(BLOB_NAME, timeout=42) 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"], {"userProject": USER_PROJECT}) - self.assertEqual(kw["timeout"], 42) - self.assertEqual(kw["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - def test_delete_blob_hit_with_generation(self): - NAME = "name" - BLOB_NAME = "blob-name" - GENERATION = 1512565576797178 - connection = _Connection({}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - result = bucket.delete_blob(BLOB_NAME, generation=GENERATION) + expected_path = "/b/%s/o/%s" % (name, blob_name) + expected_query_params = {"userProject": user_project} + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=timeout, + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + _target_object=None, + ) + + def test_delete_blob_hit_w_generation_w_retry(self): + name = "name" + blob_name = "blob-name" + generation = 1512565576797178 + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) + retry = mock.Mock(spec=[]) + + result = bucket.delete_blob(blob_name, generation=generation, retry=retry) + 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"], {"generation": GENERATION}) - self.assertEqual(kw["timeout"], self._get_default_timeout()) - self.assertEqual(kw["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - def test_delete_blob_with_generation_match(self): - NAME = "name" - BLOB_NAME = "blob-name" - GENERATION = 6 - METAGENERATION = 9 + expected_path = "/b/%s/o/%s" % (name, blob_name) + expected_query_params = {"generation": generation} + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=retry, + _target_object=None, + ) + + def test_delete_blob_hit_w_generation_match(self): + name = "name" + blob_name = "blob-name" + generation = 6 + metageneration = 9 + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) - 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, + 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}, + + expected_path = "/b/%s/o/%s" % (name, blob_name) + expected_query_params = { + "ifGenerationMatch": generation, + "ifMetagenerationMatch": metageneration, + } + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + _target_object=None, ) - self.assertEqual(kw["timeout"], self._get_default_timeout()) - self.assertEqual(kw["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) def test_delete_blobs_empty(self): - NAME = "name" - connection = _Connection() - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) + name = "name" + bucket = self._make_one(client=None, name=name) + bucket.delete_blob = mock.Mock() + bucket.delete_blobs([]) - self.assertEqual(connection._requested, []) - def test_delete_blobs_hit_w_user_project(self): - NAME = "name" - BLOB_NAME = "blob-name" - USER_PROJECT = "user-project-123" - connection = _Connection({}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME, user_project=USER_PROJECT) - bucket.delete_blobs([BLOB_NAME], timeout=42) - kw = connection._requested - self.assertEqual(len(kw), 1) - self.assertEqual(kw[0]["method"], "DELETE") - self.assertEqual(kw[0]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) - self.assertEqual(kw[0]["query_params"], {"userProject": USER_PROJECT}) - self.assertEqual(kw[0]["timeout"], 42) - self.assertEqual(kw[0]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - - def test_delete_blobs_w_generation_match(self): - NAME = "name" - BLOB_NAME = "blob-name" - BLOB_NAME2 = "blob-name2" - GENERATION_NUMBER = 6 - GENERATION_NUMBER2 = 9 + bucket.delete_blob.assert_not_called() - connection = _Connection({}, {}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - bucket.delete_blobs( - [BLOB_NAME, BLOB_NAME2], - timeout=42, - if_generation_match=[GENERATION_NUMBER, GENERATION_NUMBER2], - ) - kw = connection._requested - self.assertEqual(len(kw), 2) + def test_delete_blobs_hit_w_explicit_client_w_timeout(self): + name = "name" + blob_name = "blob-name" + client = mock.Mock(spec=[]) + bucket = self._make_one(client=None, name=name) + bucket.delete_blob = mock.Mock() + timeout = 42 - self.assertEqual(kw[0]["method"], "DELETE") - self.assertEqual(kw[0]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) - self.assertEqual(kw[0]["timeout"], 42) - self.assertEqual( - kw[0]["query_params"], {"ifGenerationMatch": GENERATION_NUMBER} - ) - self.assertEqual(kw[0]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - self.assertEqual(kw[1]["method"], "DELETE") - self.assertEqual(kw[1]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME2)) - self.assertEqual(kw[1]["timeout"], 42) - self.assertEqual( - kw[1]["query_params"], {"ifGenerationMatch": GENERATION_NUMBER2} + bucket.delete_blobs([blob_name], client=client, timeout=timeout) + + bucket.delete_blob.assert_called_once_with( + blob_name, + client=client, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=timeout, + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, ) - self.assertEqual(kw[1]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) def test_delete_blobs_w_generation_match_wrong_len(self): - NAME = "name" - BLOB_NAME = "blob-name" - BLOB_NAME2 = "blob-name2" - GENERATION_NUMBER = 6 + name = "name" + blob_name = "blob-name" + blob_name2 = "blob-name2" + generation_number = 6 + bucket = self._make_one(client=None, name=name) + bucket.delete_blob = mock.Mock() - connection = _Connection() - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) with self.assertRaises(ValueError): bucket.delete_blobs( - [BLOB_NAME, BLOB_NAME2], - timeout=42, - if_generation_not_match=[GENERATION_NUMBER], + [blob_name, blob_name2], if_generation_not_match=[generation_number], ) + bucket.delete_blob.assert_not_called() + + def test_delete_blobs_w_generation_match_w_retry(self): + name = "name" + blob_name = "blob-name" + blob_name2 = "blob-name2" + generation_number = 6 + generation_number2 = 9 + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + bucket.delete_blob = mock.Mock() + retry = mock.Mock(spec=[]) + + bucket.delete_blobs( + [blob_name, blob_name2], + if_generation_match=[generation_number, generation_number2], + retry=retry, + ) + + call_1 = mock.call( + blob_name, + client=None, + if_generation_match=generation_number, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=retry, + ) + call_2 = mock.call( + blob_name2, + client=None, + if_generation_match=generation_number2, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=retry, + ) + bucket.delete_blob.assert_has_calls([call_1, call_2]) + def test_delete_blobs_w_generation_match_none(self): - NAME = "name" - BLOB_NAME = "blob-name" - BLOB_NAME2 = "blob-name2" - GENERATION_NUMBER = 6 - GENERATION_NUMBER2 = None + name = "name" + blob_name = "blob-name" + blob_name2 = "blob-name2" + generation_number = 6 + generation_number2 = None + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + bucket.delete_blob = mock.Mock() - connection = _Connection({}, {}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) bucket.delete_blobs( - [BLOB_NAME, BLOB_NAME2], - timeout=42, - if_generation_match=[GENERATION_NUMBER, GENERATION_NUMBER2], + [blob_name, blob_name2], + if_generation_match=[generation_number, generation_number2], ) - kw = connection._requested - self.assertEqual(len(kw), 2) - self.assertEqual(kw[0]["method"], "DELETE") - self.assertEqual(kw[0]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) - self.assertEqual(kw[0]["timeout"], 42) - self.assertEqual( - kw[0]["query_params"], {"ifGenerationMatch": GENERATION_NUMBER} + call_1 = mock.call( + blob_name, + client=None, + if_generation_match=generation_number, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + call_2 = mock.call( + blob_name2, + client=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, ) - self.assertEqual(kw[0]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - self.assertEqual(kw[1]["method"], "DELETE") - self.assertEqual(kw[1]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME2)) - self.assertEqual(kw[1]["timeout"], 42) - self.assertEqual(kw[1]["query_params"], {}) - self.assertEqual(kw[1]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) + bucket.delete_blob.assert_has_calls([call_1, call_2]) - def test_delete_blobs_miss_no_on_error(self): + def test_delete_blobs_miss_wo_on_error(self): from google.cloud.exceptions import NotFound - NAME = "name" - BLOB_NAME = "blob-name" - NONESUCH = "nonesuch" - connection = _Connection({}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - self.assertRaises(NotFound, bucket.delete_blobs, [BLOB_NAME, NONESUCH]) - kw = connection._requested - self.assertEqual(len(kw), 2) - self.assertEqual(kw[0]["method"], "DELETE") - self.assertEqual(kw[0]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) - self.assertEqual(kw[0]["timeout"], self._get_default_timeout()) - self.assertEqual(kw[0]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - self.assertEqual(kw[1]["method"], "DELETE") - self.assertEqual(kw[1]["path"], "/b/%s/o/%s" % (NAME, NONESUCH)) - self.assertEqual(kw[1]["timeout"], self._get_default_timeout()) - self.assertEqual(kw[1]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) + name = "name" + blob_name = "blob-name" + blob_name2 = "nonesuch" + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + bucket.delete_blob = mock.Mock() + bucket.delete_blob.side_effect = [None, NotFound("testing")] + + with self.assertRaises(NotFound): + bucket.delete_blobs([blob_name, blob_name2]) + + call_1 = mock.call( + blob_name, + client=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + call_2 = mock.call( + blob_name2, + client=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + bucket.delete_blob.assert_has_calls([call_1, call_2]) def test_delete_blobs_miss_w_on_error(self): - NAME = "name" - BLOB_NAME = "blob-name" - NONESUCH = "nonesuch" - connection = _Connection({}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) + from google.cloud.exceptions import NotFound + + name = "name" + blob_name = "blob-name" + blob_name2 = "nonesuch" + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + bucket.delete_blob = mock.Mock() + bucket.delete_blob.side_effect = [None, NotFound("testing")] + errors = [] - bucket.delete_blobs([BLOB_NAME, NONESUCH], errors.append) - self.assertEqual(errors, [NONESUCH]) - kw = connection._requested - self.assertEqual(len(kw), 2) - self.assertEqual(kw[0]["method"], "DELETE") - self.assertEqual(kw[0]["path"], "/b/%s/o/%s" % (NAME, BLOB_NAME)) - self.assertEqual(kw[0]["timeout"], self._get_default_timeout()) - self.assertEqual(kw[0]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) - self.assertEqual(kw[1]["method"], "DELETE") - self.assertEqual(kw[1]["path"], "/b/%s/o/%s" % (NAME, NONESUCH)) - self.assertEqual(kw[1]["timeout"], self._get_default_timeout()) - self.assertEqual(kw[1]["retry"], DEFAULT_RETRY_IF_GENERATION_SPECIFIED) + bucket.delete_blobs([blob_name, blob_name2], on_error=errors.append) + + self.assertEqual(errors, [blob_name2]) + + call_1 = mock.call( + blob_name, + client=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + call_2 = mock.call( + blob_name2, + client=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + bucket.delete_blob.assert_has_calls([call_1, call_2]) def test_reload_w_metageneration_match(self): name = "name" @@ -3716,31 +3830,10 @@ def __init__(self, *responses): self._deleted_buckets = [] self.credentials = None - @staticmethod - def _is_bucket_path(path): - # Now just ensure the path only has /b/ and one more segment. - return path.startswith("/b/") and path.count("/") == 2 - def api_request(self, **kw): - from google.cloud.exceptions import NotFound - self._requested.append(kw) - - method = kw.get("method") - path = kw.get("path", "") - if method == "DELETE" and self._is_bucket_path(path): - self._deleted_buckets.append(kw) - if self._delete_bucket: - return - else: - raise NotFound("miss") - - try: - response, self._responses = self._responses[0], self._responses[1:] - except IndexError: - raise NotFound("miss") - else: - return response + response, self._responses = self._responses[0], self._responses[1:] + return response class _Client(object): diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 77e3c7e1b..6d34d935a 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -593,6 +593,64 @@ def test__put_resource_hit_w_explicit(self): _target_object=target, ) + def test__delete_resource_miss_w_defaults(self): + from google.cloud.exceptions import NotFound + + project = "PROJECT" + path = "/path/to/something" + credentials = _make_credentials() + + client = self._make_one(project=project, credentials=credentials) + connection = client._base_connection = _make_connection() + + with self.assertRaises(NotFound): + client._delete_resource(path) + + connection.api_request.assert_called_once_with( + method="DELETE", + path=path, + query_params=None, + headers=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test__delete_resource_hit_w_explicit(self): + project = "PROJECT" + path = "/path/to/something" + query_params = {"foo": "Foo"} + headers = {"bar": "Bar"} + timeout = 100 + retry = mock.Mock(spec=[]) + credentials = _make_credentials() + + client = self._make_one(project=project, credentials=credentials) + expected = mock.Mock(spec={}) + connection = client._base_connection = _make_connection(expected) + target = mock.Mock(spec={}) + + found = client._delete_resource( + path, + query_params=query_params, + headers=headers, + timeout=timeout, + retry=retry, + _target_object=target, + ) + + self.assertIs(found, expected) + + connection.api_request.assert_called_once_with( + method="DELETE", + path=path, + query_params=query_params, + headers=headers, + timeout=timeout, + retry=retry, + _target_object=target, + ) + def test_get_bucket_miss_w_string_w_defaults(self): from google.cloud.exceptions import NotFound from google.cloud.storage.bucket import Bucket diff --git a/tests/unit/test_hmac_key.py b/tests/unit/test_hmac_key.py index 45fbf3769..60d0c135b 100644 --- a/tests/unit/test_hmac_key.py +++ b/tests/unit/test_hmac_key.py @@ -385,62 +385,65 @@ def test_update_hit_w_project_set_w_timeout_w_retry(self): ) def test_delete_not_inactive(self): - metadata = self._make_one() + client = mock.Mock(spec=["_delete_resource", "project"]) + client.project = "PROJECT" + metadata = self._make_one(client) + for state in ("ACTIVE", "DELETED"): metadata._properties["state"] = state with self.assertRaises(ValueError): metadata.delete() - def test_delete_miss_no_project_set(self): + client._delete_resource.assert_not_called() + + def test_delete_miss_no_project_set_w_defaults(self): from google.cloud.exceptions import NotFound access_id = "ACCESS-ID" - connection = mock.Mock(spec=["api_request"]) - connection.api_request.side_effect = NotFound("testing") - client = _Client(connection) + client = mock.Mock(spec=["_delete_resource", "project"]) + client._delete_resource.side_effect = NotFound("testing") + client.project = "PROJECT" metadata = self._make_one(client) metadata._properties["accessId"] = access_id metadata.state = "INACTIVE" with self.assertRaises(NotFound): - metadata.delete(timeout=42) + metadata.delete() - expected_path = "/projects/{}/hmacKeys/{}".format( - client.DEFAULT_PROJECT, access_id + expected_path = "/projects/{}/hmacKeys/{}".format(client.project, access_id) + expected_query_params = {} + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, ) - expected_kwargs = { - "method": "DELETE", - "path": expected_path, - "query_params": {}, - "timeout": 42, - "retry": DEFAULT_RETRY, - } - connection.api_request.assert_called_once_with(**expected_kwargs) - def test_delete_hit_w_project_set(self): + def test_delete_hit_w_project_set_w_explicit_timeout_retry(self): project = "PROJECT-ID" access_id = "ACCESS-ID" user_project = "billed-project" - connection = mock.Mock(spec=["api_request"]) - connection.api_request.return_value = {} - client = _Client(connection) + client = mock.Mock(spec=["_delete_resource", "project"]) + client.project = "CLIENT-PROJECT" + client._delete_resource.return_value = {} metadata = self._make_one(client, user_project=user_project) metadata._properties["accessId"] = access_id metadata._properties["projectId"] = project metadata.state = "INACTIVE" + timeout = 42 + retry = mock.Mock(spec=[]) - metadata.delete() + metadata.delete(timeout=timeout, retry=retry) expected_path = "/projects/{}/hmacKeys/{}".format(project, access_id) - expected_kwargs = { - "method": "DELETE", - "path": expected_path, - "query_params": {"userProject": user_project}, - "timeout": self._get_default_timeout(), - "retry": DEFAULT_RETRY, - } - connection.api_request.assert_called_once_with(**expected_kwargs) + expected_query_params = {"userProject": user_project} + client._delete_resource.assert_called_once_with( + expected_path, + query_params=expected_query_params, + timeout=timeout, + retry=retry, + ) class _Client(object): diff --git a/tests/unit/test_notification.py b/tests/unit/test_notification.py index e8cee0478..ae8924b08 100644 --- a/tests/unit/test_notification.py +++ b/tests/unit/test_notification.py @@ -456,51 +456,55 @@ def test_reload_hit_w_explicit_w_user_project(self): ) def test_delete_wo_notification_id(self): - client = self._make_client() + client = mock.Mock(spec=["_delete_resource", "project"]) + client.project = self.BUCKET_PROJECT bucket = self._make_bucket(client) notification = self._make_one(bucket, self.TOPIC_NAME) with self.assertRaises(ValueError): notification.delete() - def test_delete_miss(self): + client._delete_resource.assert_not_called() + + def test_delete_miss_w_defaults(self): from google.cloud.exceptions import NotFound - client = self._make_client() + client = mock.Mock(spec=["_delete_resource", "project"]) + client._delete_resource.side_effect = NotFound("testing") + client.project = self.BUCKET_PROJECT bucket = self._make_bucket(client) notification = self._make_one(bucket, self.TOPIC_NAME) notification._properties["id"] = self.NOTIFICATION_ID - api_request = client._connection.api_request - api_request.side_effect = NotFound("testing") with self.assertRaises(NotFound): - notification.delete(timeout=42) + notification.delete() - api_request.assert_called_once_with( - method="DELETE", - path=self.NOTIFICATION_PATH, + client._delete_resource.assert_called_once_with( + self.NOTIFICATION_PATH, query_params={}, - timeout=42, + timeout=self._get_default_timeout(), retry=DEFAULT_RETRY, ) - def test_delete_hit(self): - USER_PROJECT = "user-project-123" - client = self._make_client() - bucket = self._make_bucket(client, user_project=USER_PROJECT) + def test_delete_hit_w_explicit_client_timeout_retry(self): + user_project = "user-project-123" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket_client = mock.Mock(spec=["project"]) + bucket_client.project = self.BUCKET_PROJECT + bucket = self._make_bucket(bucket_client, user_project=user_project) notification = self._make_one(bucket, self.TOPIC_NAME) notification._properties["id"] = self.NOTIFICATION_ID - api_request = client._connection.api_request - api_request.return_value = None + timeout = 42 + retry = mock.Mock(spec=[]) - notification.delete(client=client) + notification.delete(client=client, timeout=timeout, retry=retry) - api_request.assert_called_once_with( - method="DELETE", - path=self.NOTIFICATION_PATH, - query_params={"userProject": USER_PROJECT}, - timeout=self._get_default_timeout(), - retry=DEFAULT_RETRY, + client._delete_resource.assert_called_once_with( + self.NOTIFICATION_PATH, + query_params={"userProject": user_project}, + timeout=timeout, + retry=retry, )