From d457ce3e161555c9117ae288ec0c9cd5f8d5fe3a Mon Sep 17 00:00:00 2001 From: HemangChothani <50404902+HemangChothani@users.noreply.github.com> Date: Wed, 11 Nov 2020 15:20:09 -0500 Subject: [PATCH] fix: from_string method of blob and bucket class (#290) Fixes #286 --- google/cloud/storage/blob.py | 6 ++-- google/cloud/storage/client.py | 2 +- tests/unit/test_blob.py | 52 ++++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index d7303e0c3..e6b79dab5 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -1607,6 +1607,7 @@ def _do_multipart_upload( msg = _READ_LESS_THAN_SIZE.format(size, len(data)) raise ValueError(msg) + client = self._require_client(client) transport = self._get_transport(client) if "metadata" in self._properties and "metadata" not in self._changes: self._changes.add("metadata") @@ -1614,7 +1615,7 @@ def _do_multipart_upload( headers, object_metadata, content_type = info base_url = _MULTIPART_URL_TEMPLATE.format( - hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path + hostname=client._connection.API_BASE_URL, bucket_path=self.bucket.path ) name_value_pairs = [] @@ -1771,6 +1772,7 @@ def _initiate_resumable_upload( that was created * The ``transport`` used to initiate the upload. """ + client = self._require_client(client) if chunk_size is None: chunk_size = self.chunk_size if chunk_size is None: @@ -1785,7 +1787,7 @@ def _initiate_resumable_upload( headers.update(extra_headers) base_url = _RESUMABLE_URL_TEMPLATE.format( - hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path + hostname=client._connection.API_BASE_URL, bucket_path=self.bucket.path ) name_value_pairs = [] diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index 27c163a29..c211144f8 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -581,7 +581,7 @@ def download_blob_to_file(self, blob_or_uri, file_obj, start=None, end=None): try: blob_or_uri.download_to_file(file_obj, client=self, start=start, end=end) except AttributeError: - blob = Blob.from_string(blob_or_uri) + blob = Blob.from_string(blob_or_uri, self) blob.download_to_file(file_obj, client=self, start=start, end=end) def list_blobs( diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index 63f98eb28..cdf5f773e 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -1818,6 +1818,7 @@ def _mock_transport(self, status_code, headers, content=b""): def _do_multipart_success( self, mock_get_boundary, + client=None, size=None, num_retries=None, user_project=None, @@ -1840,12 +1841,13 @@ def _do_multipart_success( blob._properties["metadata"] = metadata self.assertEqual(len(blob._changes), 0) - # Create mocks to be checked for doing transport. - transport = self._mock_transport(http_client.OK, {}) - # Create some mock arguments. - client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"]) - client._connection.API_BASE_URL = "https://storage.googleapis.com" + if not client: + # Create mocks to be checked for doing transport. + transport = self._mock_transport(http_client.OK, {}) + + client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"]) + client._connection.API_BASE_URL = "https://storage.googleapis.com" data = b"data here hear hier" stream = io.BytesIO(data) content_type = u"application/xml" @@ -1872,7 +1874,7 @@ def _do_multipart_success( ) # Check the mocks and the returned value. - self.assertIs(response, transport.request.return_value) + self.assertIs(response, client._http.request.return_value) if size is None: data_read = data self.assertEqual(stream.tell(), len(data)) @@ -1929,7 +1931,7 @@ def _do_multipart_success( + b"\r\n--==0==--" ) headers = {"content-type": b'multipart/related; boundary="==0=="'} - transport.request.assert_called_once_with( + client._http.request.assert_called_once_with( "POST", upload_url, data=payload, headers=headers, timeout=expected_timeout ) @@ -1987,6 +1989,13 @@ def test__do_multipart_upload_with_generation_not_match(self, mock_get_boundary) mock_get_boundary, if_generation_not_match=4, if_metageneration_not_match=4 ) + @mock.patch(u"google.resumable_media._upload.get_boundary", return_value=b"==0==") + def test__do_multipart_upload_with_client(self, mock_get_boundary): + transport = self._mock_transport(http_client.OK, {}) + client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"]) + client._connection.API_BASE_URL = "https://storage.googleapis.com" + self._do_multipart_success(mock_get_boundary, client=client) + @mock.patch(u"google.resumable_media._upload.get_boundary", return_value=b"==0==") def test__do_multipart_upload_with_metadata(self, mock_get_boundary): self._do_multipart_success(mock_get_boundary, metadata={"test": "test"}) @@ -2010,6 +2019,7 @@ def test__do_multipart_upload_bad_size(self): def _initiate_resumable_helper( self, + client=None, size=None, extra_headers=None, chunk_size=None, @@ -2051,14 +2061,17 @@ def _initiate_resumable_helper( return_value=object_metadata, spec=[] ) - # Create mocks to be checked for doing transport. resumable_url = "http://test.invalid?upload_id=hey-you" - response_headers = {"location": resumable_url} - transport = self._mock_transport(http_client.OK, response_headers) - - # Create some mock arguments and call the method under test. - client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"]) - client._connection.API_BASE_URL = "https://storage.googleapis.com" + if not client: + # Create mocks to be checked for doing transport. + response_headers = {"location": resumable_url} + transport = self._mock_transport(http_client.OK, response_headers) + + # Create some mock arguments and call the method under test. + client = mock.Mock( + _http=transport, _connection=_Connection, spec=[u"_http"] + ) + client._connection.API_BASE_URL = "https://storage.googleapis.com" data = b"hello hallo halo hi-low" stream = io.BytesIO(data) content_type = u"text/plain" @@ -2149,7 +2162,7 @@ def _initiate_resumable_helper( else: self.assertIsNone(retry_strategy.max_cumulative_retry) self.assertEqual(retry_strategy.max_retries, num_retries) - self.assertIs(transport, transport) + self.assertIs(client._http, transport) # Make sure we never read from the stream. self.assertEqual(stream.tell(), 0) @@ -2237,6 +2250,15 @@ def test__initiate_resumable_upload_with_generation_not_match(self): def test__initiate_resumable_upload_with_predefined_acl(self): self._initiate_resumable_helper(predefined_acl="private") + def test__initiate_resumable_upload_with_client(self): + resumable_url = "http://test.invalid?upload_id=hey-you" + response_headers = {"location": resumable_url} + transport = self._mock_transport(http_client.OK, response_headers) + + client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"]) + client._connection.API_BASE_URL = "https://storage.googleapis.com" + self._initiate_resumable_helper(client=client) + def _make_resumable_transport( self, headers1, headers2, headers3, total_bytes, data_corruption=False ):