From feccf18b2cfcd7af8d7299c482e8750308d95a34 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Jul 2019 14:14:45 +0300 Subject: [PATCH 01/13] Storage: client redesign proposal --- raise | 0 return | 0 storage/google/cloud/storage/bucket.py | 35 ++---- storage/google/cloud/storage/client.py | 29 ++++- storage/tests/unit/test_bucket.py | 165 +++++++++++++++++-------- storage/tests/unit/test_client.py | 3 + 6 files changed, 158 insertions(+), 74 deletions(-) create mode 100644 raise create mode 100644 return diff --git a/raise b/raise new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/return b/return new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 1bf63ec11ab8..d4d1196345c1 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -437,6 +437,18 @@ def user_project(self): """ return self._user_project + @property + def properties(self): + """Properties of the current bucket object. + + Returns: + dict + Bucket properties transformed to dict. + """ + properties = {key: self._properties[key] for key in self._changes} + properties["name"] = self.name + return properties + def blob( self, blob_name, @@ -577,28 +589,7 @@ def create(self, client=None, project=None, location=None): raise ValueError("Cannot create bucket with 'user_project' set.") client = self._require_client(client) - - if project is None: - project = client.project - - if project is None: - raise ValueError("Client project not set: pass an explicit project.") - - query_params = {"project": project} - properties = {key: self._properties[key] for key in self._changes} - properties["name"] = self.name - - if location is not None: - properties["location"] = location - - api_response = client._connection.api_request( - method="POST", - path="/b", - query_params=query_params, - data=properties, - _target_object=self, - ) - self._set_properties(api_response) + client.create_bucket(self, project=project, location=location) def patch(self, client=None): """Sends all changed properties in a PATCH request. diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 782547ae43b0..1691c4d280a3 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -298,7 +298,7 @@ def lookup_bucket(self, bucket_name): except NotFound: return None - def create_bucket(self, bucket_or_name, requester_pays=None, project=None): + def create_bucket(self, bucket_or_name, requester_pays=None, project=None, location=None): """API call: create a new bucket via a POST request. See @@ -316,6 +316,10 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None): project (str): Optional. the project under which the bucket is to be created. If not passed, uses the project set on the client. + location (str): + Optional. The location of the bucket. If not passed, + the default location, US, will be used. See + https://cloud.google.com/storage/docs/bucket-locations Returns: google.cloud.storage.bucket.Bucket @@ -348,9 +352,30 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None): """ bucket = self._bucket_arg_to_bucket(bucket_or_name) + if project is None: + project = self.project + + if project is None: + raise ValueError("Client project not set: pass an explicit project.") + if requester_pays is not None: bucket.requester_pays = requester_pays - bucket.create(client=self, project=project) + + query_params = {"project": project} + properties = bucket.properties + + if location is not None: + properties["location"] = location + + api_response = self._connection.api_request( + method="POST", + path="/b", + query_params=query_params, + data=properties, + _target_object=bucket, + ) + + bucket._set_properties(api_response) return bucket def download_blob_to_file(self, blob_or_uri, file_obj, start=None, end=None): diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 2f0661537a09..d5006ffd0c42 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -18,6 +18,16 @@ import mock +def _make_connection(*responses): + import google.cloud.storage._http + from google.cloud.exceptions import NotFound + + mock_conn = mock.create_autospec(google.cloud.storage._http.Connection) + mock_conn.user_agent = "testing 1.2.3" + mock_conn.api_request.side_effect = list(responses) + return mock_conn + + def _create_signing_credentials(): import google.auth.credentials @@ -535,66 +545,109 @@ def test_create_w_user_project(self): bucket.create() def test_create_w_missing_client_project(self): + from google.cloud.storage.client import Client + BUCKET_NAME = "bucket-name" - connection = _Connection() - client = _Client(connection, project=None) - bucket = self._make_one(client, BUCKET_NAME) + connection = _make_connection() + client = Client(project=None) - with self.assertRaises(ValueError): - bucket.create() + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket = self._make_one(client, BUCKET_NAME) + + with self.assertRaises(ValueError): + bucket.create() def test_create_w_explicit_project(self): + from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" OTHER_PROJECT = "other-project-123" DATA = {"name": BUCKET_NAME} - connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) - bucket = self._make_one(client, BUCKET_NAME) - - bucket.create(project=OTHER_PROJECT) - - kw, = connection._requested - self.assertEqual(kw["method"], "POST") - self.assertEqual(kw["path"], "/b") - self.assertEqual(kw["query_params"], {"project": OTHER_PROJECT}) - self.assertEqual(kw["data"], DATA) + connection = _make_connection(DATA) + client = Client(project=PROJECT) + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket = self._make_one(client, BUCKET_NAME) + bucket.create(project=OTHER_PROJECT) + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": OTHER_PROJECT}, + data=DATA, + _target_object=bucket + ) def test_create_w_explicit_location(self): + from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" LOCATION = "us-central1" DATA = {"location": LOCATION, "name": BUCKET_NAME} - connection = _Connection( - DATA, "{'location': 'us-central1', 'name': 'bucket-name'}" + + connection = _make_connection( + DATA, + "{'location': 'us-central1', 'name': 'bucket-name'}" ) - client = _Client(connection, project=PROJECT) - bucket = self._make_one(client, BUCKET_NAME) - bucket.create(location=LOCATION) + client = Client(project=PROJECT) + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection - kw, = connection._requested - self.assertEqual(kw["method"], "POST") - self.assertEqual(kw["path"], "/b") - self.assertEqual(kw["data"], DATA) - self.assertEqual(bucket.location, LOCATION) + bucket = self._make_one(client, BUCKET_NAME) + + bucket.create(location=LOCATION) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + data=DATA, + _target_object=bucket, + query_params={'project': 'PROJECT'} + ) + self.assertEqual(bucket.location, LOCATION) def test_create_hit(self): + from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} - connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) - bucket = self._make_one(client=client, name=BUCKET_NAME) - bucket.create() + connection = _make_connection(DATA) + client = Client(project=PROJECT) + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket = self._make_one(client=client, name=BUCKET_NAME) + bucket.create() - kw, = connection._requested - self.assertEqual(kw["method"], "POST") - self.assertEqual(kw["path"], "/b") - self.assertEqual(kw["query_params"], {"project": PROJECT}) - self.assertEqual(kw["data"], DATA) + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": PROJECT}, + data=DATA, + _target_object=bucket + ) def test_create_w_extra_properties(self): + from google.cloud.storage.client import Client + BUCKET_NAME = "bucket-name" PROJECT = "PROJECT" CORS = [ @@ -619,22 +672,31 @@ def test_create_w_extra_properties(self): "billing": {"requesterPays": True}, "labels": LABELS, } - connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) - bucket = self._make_one(client=client, name=BUCKET_NAME) - bucket.cors = CORS - bucket.lifecycle_rules = LIFECYCLE_RULES - bucket.storage_class = STORAGE_CLASS - bucket.versioning_enabled = True - bucket.requester_pays = True - bucket.labels = LABELS - bucket.create(location=LOCATION) - kw, = connection._requested - self.assertEqual(kw["method"], "POST") - self.assertEqual(kw["path"], "/b") - self.assertEqual(kw["query_params"], {"project": PROJECT}) - self.assertEqual(kw["data"], DATA) + connection = _make_connection(DATA) + client = Client(project=PROJECT) + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket = self._make_one(client=client, name=BUCKET_NAME) + bucket.cors = CORS + bucket.lifecycle_rules = LIFECYCLE_RULES + bucket.storage_class = STORAGE_CLASS + bucket.versioning_enabled = True + bucket.requester_pays = True + bucket.labels = LABELS + bucket.create(location=LOCATION) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": PROJECT}, + data=DATA, + _target_object=bucket + ) def test_acl_property(self): from google.cloud.storage.acl import BucketACL @@ -2776,3 +2838,6 @@ def _connection(self): @property def _credentials(self): return self._base_connection.credentials + +if __name__ == '__main__': + unittest.main() diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 89ed8570dbfa..a78319479986 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -908,3 +908,6 @@ def dummy_response(): self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) + +if __name__ == '__main__': + unittest.main() From 1f57249683329bfad2e75b8064303db7184263a9 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Jul 2019 14:38:29 +0300 Subject: [PATCH 02/13] Delete unittest.main --- storage/tests/unit/test_bucket.py | 3 --- storage/tests/unit/test_client.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index d5006ffd0c42..584a627d40c0 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -2838,6 +2838,3 @@ def _connection(self): @property def _credentials(self): return self._base_connection.credentials - -if __name__ == '__main__': - unittest.main() diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index a78319479986..89ed8570dbfa 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -908,6 +908,3 @@ def dummy_response(): self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) - -if __name__ == '__main__': - unittest.main() From b9146a40914a5156f65d4195ba5d540672ff8399 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Jul 2019 14:46:24 +0300 Subject: [PATCH 03/13] Delete unnecessary import --- storage/tests/unit/test_bucket.py | 1 - 1 file changed, 1 deletion(-) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 584a627d40c0..2da7735675aa 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -20,7 +20,6 @@ def _make_connection(*responses): import google.cloud.storage._http - from google.cloud.exceptions import NotFound mock_conn = mock.create_autospec(google.cloud.storage._http.Connection) mock_conn.user_agent = "testing 1.2.3" From be8166c6bd562ebfe884aa3d36c2277a408793d4 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Jul 2019 17:52:37 +0300 Subject: [PATCH 04/13] Cosmetic changes --- storage/google/cloud/storage/bucket.py | 3 +-- storage/tests/unit/test_bucket.py | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index d4d1196345c1..d28f31158fd0 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -442,8 +442,7 @@ def properties(self): """Properties of the current bucket object. Returns: - dict - Bucket properties transformed to dict. + dict: Bucket properties transformed to dict. """ properties = {key: self._properties[key] for key in self._changes} properties["name"] = self.name diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 2da7735675aa..42249b66473b 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -21,10 +21,10 @@ def _make_connection(*responses): import google.cloud.storage._http - mock_conn = mock.create_autospec(google.cloud.storage._http.Connection) - mock_conn.user_agent = "testing 1.2.3" - mock_conn.api_request.side_effect = list(responses) - return mock_conn + mock_connection = mock.create_autospec(google.cloud.storage._http.Connection) + mock_connection.user_agent = "testing 1.2.3" + mock_connection.api_request.side_effect = list(responses) + return mock_connection def _create_signing_credentials(): From 09d0c6f9144a4945522e133447e20935e501a180 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 8 Aug 2019 13:37:12 +0300 Subject: [PATCH 05/13] Delete public property for bucket properties and mocks in bucket tests. Black reformat also --- storage/google/cloud/storage/bucket.py | 11 -- storage/google/cloud/storage/client.py | 7 +- storage/tests/unit/test_bucket.py | 142 ++++++++++--------------- 3 files changed, 64 insertions(+), 96 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index ebcb848896dc..195314a068e3 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -495,17 +495,6 @@ def user_project(self): """ return self._user_project - @property - def properties(self): - """Properties of the current bucket object. - - Returns: - dict: Bucket properties transformed to dict. - """ - properties = {key: self._properties[key] for key in self._changes} - properties["name"] = self.name - return properties - def blob( self, blob_name, diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index a34d21bb8e5e..a16b7b45bf21 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -299,7 +299,9 @@ def lookup_bucket(self, bucket_name): except NotFound: return None - def create_bucket(self, bucket_or_name, requester_pays=None, project=None, location=None): + def create_bucket( + self, bucket_or_name, requester_pays=None, project=None, location=None + ): """API call: create a new bucket via a POST request. See @@ -363,7 +365,8 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None, locat bucket.requester_pays = requester_pays query_params = {"project": project} - properties = bucket.properties + properties = {key: bucket._properties[key] for key in bucket._changes} + properties["name"] = bucket.name if location is not None: properties["location"] = location diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 91f17f9a66a1..d08ff09268eb 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -556,19 +556,12 @@ def test_create_w_missing_client_project(self): from google.cloud.storage.client import Client BUCKET_NAME = "bucket-name" - connection = _make_connection() - client = Client(project=None) - - with mock.patch( - 'google.cloud.storage.client.Client._connection', - new_callable=mock.PropertyMock - ) as client_mock: - client_mock.return_value = connection - bucket = self._make_one(client, BUCKET_NAME) + client = Client(project=None) + bucket = self._make_one(client, BUCKET_NAME) - with self.assertRaises(ValueError): - bucket.create() + with self.assertRaises(ValueError): + bucket.create() def test_create_w_explicit_project(self): from google.cloud.storage.client import Client @@ -578,22 +571,19 @@ def test_create_w_explicit_project(self): OTHER_PROJECT = "other-project-123" DATA = {"name": BUCKET_NAME} connection = _make_connection(DATA) + client = Client(project=PROJECT) - with mock.patch( - 'google.cloud.storage.client.Client._connection', - new_callable=mock.PropertyMock - ) as client_mock: - client_mock.return_value = connection - - bucket = self._make_one(client, BUCKET_NAME) - bucket.create(project=OTHER_PROJECT) - connection.api_request.assert_called_once_with( - method="POST", - path="/b", - query_params={"project": OTHER_PROJECT}, - data=DATA, - _target_object=bucket - ) + client._base_connection = connection + + bucket = self._make_one(client, BUCKET_NAME) + bucket.create(project=OTHER_PROJECT) + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": OTHER_PROJECT}, + data=DATA, + _target_object=bucket, + ) def test_create_w_explicit_location(self): from google.cloud.storage.client import Client @@ -604,29 +594,23 @@ def test_create_w_explicit_location(self): DATA = {"location": LOCATION, "name": BUCKET_NAME} connection = _make_connection( - DATA, - "{'location': 'us-central1', 'name': 'bucket-name'}" + DATA, "{'location': 'us-central1', 'name': 'bucket-name'}" ) client = Client(project=PROJECT) - with mock.patch( - 'google.cloud.storage.client.Client._connection', - new_callable=mock.PropertyMock - ) as client_mock: - client_mock.return_value = connection - - bucket = self._make_one(client, BUCKET_NAME) - - bucket.create(location=LOCATION) - - connection.api_request.assert_called_once_with( - method="POST", - path="/b", - data=DATA, - _target_object=bucket, - query_params={'project': 'PROJECT'} - ) - self.assertEqual(bucket.location, LOCATION) + client._base_connection = connection + + bucket = self._make_one(client, BUCKET_NAME) + bucket.create(location=LOCATION) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + data=DATA, + _target_object=bucket, + query_params={"project": "PROJECT"}, + ) + self.assertEqual(bucket.location, LOCATION) def test_create_hit(self): from google.cloud.storage.client import Client @@ -636,22 +620,18 @@ def test_create_hit(self): DATA = {"name": BUCKET_NAME} connection = _make_connection(DATA) client = Client(project=PROJECT) - with mock.patch( - 'google.cloud.storage.client.Client._connection', - new_callable=mock.PropertyMock - ) as client_mock: - client_mock.return_value = connection + client._base_connection = connection - bucket = self._make_one(client=client, name=BUCKET_NAME) - bucket.create() - - connection.api_request.assert_called_once_with( - method="POST", - path="/b", - query_params={"project": PROJECT}, - data=DATA, - _target_object=bucket - ) + bucket = self._make_one(client=client, name=BUCKET_NAME) + bucket.create() + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": PROJECT}, + data=DATA, + _target_object=bucket, + ) def test_create_w_extra_properties(self): from google.cloud.storage.client import Client @@ -683,28 +663,24 @@ def test_create_w_extra_properties(self): connection = _make_connection(DATA) client = Client(project=PROJECT) - with mock.patch( - 'google.cloud.storage.client.Client._connection', - new_callable=mock.PropertyMock - ) as client_mock: - client_mock.return_value = connection - - bucket = self._make_one(client=client, name=BUCKET_NAME) - bucket.cors = CORS - bucket.lifecycle_rules = LIFECYCLE_RULES - bucket.storage_class = STORAGE_CLASS - bucket.versioning_enabled = True - bucket.requester_pays = True - bucket.labels = LABELS - bucket.create(location=LOCATION) - - connection.api_request.assert_called_once_with( - method="POST", - path="/b", - query_params={"project": PROJECT}, - data=DATA, - _target_object=bucket - ) + client._base_connection = connection + + bucket = self._make_one(client=client, name=BUCKET_NAME) + bucket.cors = CORS + bucket.lifecycle_rules = LIFECYCLE_RULES + bucket.storage_class = STORAGE_CLASS + bucket.versioning_enabled = True + bucket.requester_pays = True + bucket.labels = LABELS + bucket.create(location=LOCATION) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": PROJECT}, + data=DATA, + _target_object=bucket, + ) def test_acl_property(self): from google.cloud.storage.acl import BucketACL From 5a31265644dacdbfdcc4257f000142ea957864cc Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 28 Oct 2019 13:38:05 +0300 Subject: [PATCH 06/13] Updated PRed changes to actualize them with changes from a79d98de3e9d6890c4e98d33cecae2f80550fdfd --- storage/google/cloud/storage/bucket.py | 2 +- storage/google/cloud/storage/client.py | 27 +++++++++++++++++++++++++- storage/tests/unit/test_bucket.py | 17 ++++++++++++---- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 9653ed66fed1..756a9ba4f43f 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -644,7 +644,7 @@ def create( raise ValueError("Cannot create bucket with 'user_project' set.") client = self._require_client(client) - client.create_bucket(self, project=project, location=location) + client.create_bucket(self, project=project, location=location, predefined_acl=predefined_acl, predefined_default_object_acl=predefined_default_object_acl) def patch(self, client=None): """Sends all changed properties in a PATCH request. diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 5643f151a9b8..ea590800cbbe 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -28,6 +28,8 @@ from google.cloud.storage.bucket import Bucket from google.cloud.storage.blob import Blob from google.cloud.storage.hmac_key import HMACKeyMetadata +from google.cloud.storage.acl import BucketACL +from google.cloud.storage.acl import DefaultObjectACL _marker = object() @@ -325,7 +327,13 @@ def lookup_bucket(self, bucket_name): return None def create_bucket( - self, bucket_or_name, requester_pays=None, project=None, location=None + self, + bucket_or_name, + requester_pays=None, + project=None, + location=None, + predefined_acl=None, + predefined_default_object_acl=None, ): """API call: create a new bucket via a POST request. @@ -348,6 +356,12 @@ def create_bucket( Optional. The location of the bucket. If not passed, the default location, US, will be used. See https://cloud.google.com/storage/docs/bucket-locations + predefined_acl (str): + Optional. Name of predefined ACL to apply to bucket. See: + https://cloud.google.com/storage/docs/access-control/lists#predefined-acl + predefined_default_object_acl (str): + Optional. Name of predefined ACL to apply to bucket's objects. See: + https://cloud.google.com/storage/docs/access-control/lists#predefined-acl Returns: google.cloud.storage.bucket.Bucket @@ -390,6 +404,17 @@ def create_bucket( bucket.requester_pays = requester_pays query_params = {"project": project} + + if predefined_acl is not None: + predefined_acl = BucketACL.validate_predefined(predefined_acl) + query_params["predefinedAcl"] = predefined_acl + + if predefined_default_object_acl is not None: + predefined_default_object_acl = DefaultObjectACL.validate_predefined( + predefined_default_object_acl + ) + query_params["predefinedDefaultObjectAcl"] = predefined_default_object_acl + properties = {key: bucket._properties[key] for key in bucket._changes} properties["name"] = bucket.name diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 72f66e311ec7..874b0cd8db90 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -684,22 +684,27 @@ def test_create_w_extra_properties(self): ) def test_create_w_predefined_acl_invalid(self): + from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) + client = Client(project=PROJECT) + client._base_connection = connection bucket = self._make_one(client=client, name=BUCKET_NAME) with self.assertRaises(ValueError): bucket.create(predefined_acl="bogus") def test_create_w_predefined_acl_valid(self): + from google.cloud.storage.client import Client PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) + client = Client(project=PROJECT) + client._base_connection = connection bucket = self._make_one(client=client, name=BUCKET_NAME) bucket.create(predefined_acl="publicRead") @@ -711,22 +716,26 @@ def test_create_w_predefined_acl_valid(self): self.assertEqual(kw["data"], DATA) def test_create_w_predefined_default_object_acl_invalid(self): + from google.cloud.storage.client import Client PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) + client = Client(project=PROJECT) + client._base_connection = connection bucket = self._make_one(client=client, name=BUCKET_NAME) with self.assertRaises(ValueError): bucket.create(predefined_default_object_acl="bogus") def test_create_w_predefined_default_object_acl_valid(self): + from google.cloud.storage.client import Client PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} connection = _Connection(DATA) - client = _Client(connection, project=PROJECT) + client = Client(project=PROJECT) + client._base_connection = connection bucket = self._make_one(client=client, name=BUCKET_NAME) bucket.create(predefined_default_object_acl="publicRead") From e4f94da28787b7289b16003e481c961cc8b43ab8 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 28 Oct 2019 14:34:50 +0300 Subject: [PATCH 07/13] Black reformat --- storage/google/cloud/storage/bucket.py | 8 +++++++- storage/tests/unit/test_bucket.py | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 756a9ba4f43f..577073a09985 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -644,7 +644,13 @@ def create( raise ValueError("Cannot create bucket with 'user_project' set.") client = self._require_client(client) - client.create_bucket(self, project=project, location=location, predefined_acl=predefined_acl, predefined_default_object_acl=predefined_default_object_acl) + client.create_bucket( + self, + project=project, + location=location, + predefined_acl=predefined_acl, + predefined_default_object_acl=predefined_default_object_acl, + ) def patch(self, client=None): """Sends all changed properties in a PATCH request. diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 874b0cd8db90..38a0f61e8c86 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -699,6 +699,7 @@ def test_create_w_predefined_acl_invalid(self): def test_create_w_predefined_acl_valid(self): from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} @@ -717,6 +718,7 @@ def test_create_w_predefined_acl_valid(self): def test_create_w_predefined_default_object_acl_invalid(self): from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} @@ -730,6 +732,7 @@ def test_create_w_predefined_default_object_acl_invalid(self): def test_create_w_predefined_default_object_acl_valid(self): from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" DATA = {"name": BUCKET_NAME} From e96d05ca7709d8f8e3be958f73f32a51fbd5d689 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 Nov 2019 14:06:15 +0300 Subject: [PATCH 08/13] Add user_project param into Client.create_bucket() method. --- storage/google/cloud/storage/bucket.py | 4 +--- storage/google/cloud/storage/client.py | 7 +++++++ storage/tests/unit/test_bucket.py | 8 ++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 577073a09985..ea0e33fda2f5 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -640,9 +640,6 @@ def create( Optional. Name of predefined ACL to apply to bucket's objects. See: https://cloud.google.com/storage/docs/access-control/lists#predefined-acl """ - if self.user_project is not None: - raise ValueError("Cannot create bucket with 'user_project' set.") - client = self._require_client(client) client.create_bucket( self, @@ -650,6 +647,7 @@ def create( location=location, predefined_acl=predefined_acl, predefined_default_object_acl=predefined_default_object_acl, + user_project=self.user_project, ) def patch(self, client=None): diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index ea590800cbbe..bdb55871995e 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -331,6 +331,7 @@ def create_bucket( bucket_or_name, requester_pays=None, project=None, + user_project=None, location=None, predefined_acl=None, predefined_default_object_acl=None, @@ -352,6 +353,9 @@ def create_bucket( project (str): Optional. the project under which the bucket is to be created. If not passed, uses the project set on the client. + user_project (str): + (Optional) the project ID to be billed for API + requests made via new bucket. location (str): Optional. The location of the bucket. If not passed, the default location, US, will be used. See @@ -392,6 +396,9 @@ def create_bucket( >>> bucket = client.create_bucket(bucket) # API request. """ + if user_project is not None: + raise ValueError("Cannot create bucket with 'user_project' set.") + bucket = self._bucket_arg_to_bucket(bucket_or_name) if project is None: diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 38a0f61e8c86..d88004872b4c 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -543,11 +543,15 @@ def api_request(cls, *args, **kwargs): self.assertEqual(_FakeConnection._called_with, expected_cw) def test_create_w_user_project(self): + from google.cloud.storage.client import Client + PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" USER_PROJECT = "user-project-123" - connection = _Connection() - client = _Client(connection, project=PROJECT) + + client = Client(project=PROJECT) + client._base_connection = _Connection() + bucket = self._make_one(client, BUCKET_NAME, user_project=USER_PROJECT) with self.assertRaises(ValueError): From 35427fbd037bd639870096b40b50c846f6aacfe4 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 1 Nov 2019 14:14:34 +0300 Subject: [PATCH 09/13] Fix comment. --- storage/google/cloud/storage/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index bdb55871995e..ec6c982c7195 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -351,11 +351,11 @@ def create_bucket( Optional. Whether requester pays for API requests for this bucket and its blobs. project (str): - Optional. the project under which the bucket is to be created. + Optional. The project under which the bucket is to be created. If not passed, uses the project set on the client. user_project (str): - (Optional) the project ID to be billed for API - requests made via new bucket. + Optional. The project ID to be billed for API requests + made via created bucket. location (str): Optional. The location of the bucket. If not passed, the default location, US, will be used. See From 8000ce6056381dd114ae3daee8368ed1c651c168 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 4 Nov 2019 11:14:08 +0300 Subject: [PATCH 10/13] Create bucket should support user_project. --- storage/google/cloud/storage/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index ec6c982c7195..9c89b342df4d 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -396,9 +396,6 @@ def create_bucket( >>> bucket = client.create_bucket(bucket) # API request. """ - if user_project is not None: - raise ValueError("Cannot create bucket with 'user_project' set.") - bucket = self._bucket_arg_to_bucket(bucket_or_name) if project is None: @@ -422,6 +419,9 @@ def create_bucket( ) query_params["predefinedDefaultObjectAcl"] = predefined_default_object_acl + if user_project is not None: + query_params["userProject"] = user_project + properties = {key: bucket._properties[key] for key in bucket._changes} properties["name"] = bucket.name From e206be2b5dada7565b1a4f6a6354d492e39dd47a Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 4 Nov 2019 12:50:36 +0300 Subject: [PATCH 11/13] Test for user_project query parameter. --- storage/tests/unit/test_client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index f8a857d164a0..b1623f3c5392 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -521,6 +521,7 @@ def test_create_bucket_with_string_conflict(self): from google.cloud.exceptions import Conflict project = "PROJECT" + user_project = "USER_PROJECT" other_project = "OTHER_PROJECT" credentials = _make_credentials() client = self._make_one(project=project, credentials=credentials) @@ -531,7 +532,7 @@ def test_create_bucket_with_string_conflict(self): client._connection.API_BASE_URL, "storage", client._connection.API_VERSION, - "b?project=%s" % (other_project,), + "b?project=%s&userProject=%s" % (other_project, user_project), ] ) data = {"error": {"message": "Conflict"}} @@ -542,7 +543,7 @@ def test_create_bucket_with_string_conflict(self): client._http_internal = http with self.assertRaises(Conflict): - client.create_bucket(bucket_name, project=other_project) + client.create_bucket(bucket_name, project=other_project, user_project=user_project) http.request.assert_called_once_with( method="POST", url=URI, data=mock.ANY, headers=mock.ANY From 675e11d54b6805d838dfc97545ac83e1d3296719 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 5 Nov 2019 14:00:49 +0300 Subject: [PATCH 12/13] Copy tests from bucket test to client. --- storage/noxfile.py | 40 ++++++------ storage/tests/unit/test_client.py | 100 +++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 21 deletions(-) diff --git a/storage/noxfile.py b/storage/noxfile.py index a391c6732b70..7341094f41df 100644 --- a/storage/noxfile.py +++ b/storage/noxfile.py @@ -40,7 +40,7 @@ def lint(session): session.run("flake8", "google", "tests") -@nox.session(python="3.6") +@nox.session(python="3.7") def blacken(session): """Run black. @@ -97,31 +97,31 @@ def unit(session): default(session) -@nox.session(python=["2.7", "3.6"]) -def system(session): - """Run the system test suite.""" +# @nox.session(python=["2.7", "3.6"]) +# def system(session): +# """Run the system test suite.""" - # Sanity check: Only run system tests if the environment variable is set. - if not os.environ.get("GOOGLE_APPLICATION_CREDENTIALS", ""): - session.skip("Credentials must be set via environment variable.") +# # Sanity check: Only run system tests if the environment variable is set. +# if not os.environ.get("GOOGLE_APPLICATION_CREDENTIALS", ""): +# session.skip("Credentials must be set via environment variable.") - # Use pre-release gRPC for system tests. - session.install("--pre", "grpcio") +# # Use pre-release gRPC for system tests. +# session.install("--pre", "grpcio") - # Install all test dependencies, then install local packages in-place. - session.install("mock", "pytest") - for local_dep in LOCAL_DEPS: - session.install("-e", local_dep) - systest_deps = ["../test_utils/", "../pubsub", "../kms"] - for systest_dep in systest_deps: - session.install("-e", systest_dep) - session.install("-e", ".") +# # Install all test dependencies, then install local packages in-place. +# session.install("mock", "pytest") +# for local_dep in LOCAL_DEPS: +# session.install("-e", local_dep) +# systest_deps = ["../test_utils/", "../pubsub", "../kms"] +# for systest_dep in systest_deps: +# session.install("-e", systest_dep) +# session.install("-e", ".") - # Run py.test against the system tests. - session.run("py.test", "--quiet", "tests/system.py", *session.posargs) +# # Run py.test against the system tests. +# session.run("py.test", "--quiet", "tests/system.py", *session.posargs) -@nox.session(python="3.6") +@nox.session(python="3.7") def cover(session): """Run the final coverage report. diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index b1623f3c5392..5141ba6e83d0 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -543,7 +543,9 @@ def test_create_bucket_with_string_conflict(self): client._http_internal = http with self.assertRaises(Conflict): - client.create_bucket(bucket_name, project=other_project, user_project=user_project) + client.create_bucket( + bucket_name, project=other_project, user_project=user_project + ) http.request.assert_called_once_with( method="POST", url=URI, data=mock.ANY, headers=mock.ANY @@ -586,6 +588,102 @@ def test_create_bucket_with_object_conflict(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) + def test_create_w_missing_client_project(self): + client = self._make_one(project=None) + + with self.assertRaises(ValueError): + client.create_bucket("bucket") + + def test_create_w_predefined_acl_invalid(self): + PROJECT = "PROJECT" + BUCKET_NAME = "bucket-name" + credentials = _make_credentials() + client = self._make_one(project=PROJECT, credentials=credentials) + + with self.assertRaises(ValueError): + client.create_bucket(BUCKET_NAME, predefined_acl="bogus") + + def test_create_w_predefined_acl_valid(self): + from google.cloud.storage.client import Client + + PROJECT = "PROJECT" + BUCKET_NAME = "bucket-name" + DATA = {"name": BUCKET_NAME} + + client = Client(project=PROJECT) + connection = _make_connection(DATA) + client._base_connection = connection + bucket = client.create_bucket(BUCKET_NAME, predefined_acl="publicRead") + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": PROJECT, "predefinedAcl": "publicRead"}, + data=DATA, + _target_object=bucket, + ) + + def test_create_w_predefined_default_object_acl_invalid(self): + PROJECT = "PROJECT" + BUCKET_NAME = "bucket-name" + + credentials = _make_credentials() + client = self._make_one(project=PROJECT, credentials=credentials) + + with self.assertRaises(ValueError): + client.create_bucket(BUCKET_NAME, predefined_default_object_acl="bogus") + + def test_create_w_predefined_default_object_acl_valid(self): + from google.cloud.storage.client import Client + + PROJECT = "PROJECT" + BUCKET_NAME = "bucket-name" + DATA = {"name": BUCKET_NAME} + + client = Client(project=PROJECT) + connection = _make_connection(DATA) + client._base_connection = connection + bucket = client.create_bucket( + BUCKET_NAME, predefined_default_object_acl="publicRead" + ) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={ + "project": PROJECT, + "predefinedDefaultObjectAcl": "publicRead", + }, + data=DATA, + _target_object=bucket, + ) + + def test_create_w_explicit_location(self): + from google.cloud.storage.client import Client + + PROJECT = "PROJECT" + BUCKET_NAME = "bucket-name" + LOCATION = "us-central1" + DATA = {"location": LOCATION, "name": BUCKET_NAME} + + connection = _make_connection( + DATA, "{'location': 'us-central1', 'name': 'bucket-name'}" + ) + + client = Client(project=PROJECT) + client._base_connection = connection + + bucket = client.create_bucket(BUCKET_NAME, location=LOCATION) + + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + data=DATA, + _target_object=bucket, + query_params={"project": "PROJECT"}, + ) + self.assertEqual(bucket.location, LOCATION) + def test_create_bucket_with_string_success(self): from google.cloud.storage.bucket import Bucket From 2530133a4bb74330ea5d14f25bfe0a2dda0adfed Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 5 Nov 2019 14:02:06 +0300 Subject: [PATCH 13/13] Revert "Copy tests from bucket test to client." This reverts commit 675e11d54b6805d838dfc97545ac83e1d3296719. --- storage/noxfile.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/storage/noxfile.py b/storage/noxfile.py index 7341094f41df..a391c6732b70 100644 --- a/storage/noxfile.py +++ b/storage/noxfile.py @@ -40,7 +40,7 @@ def lint(session): session.run("flake8", "google", "tests") -@nox.session(python="3.7") +@nox.session(python="3.6") def blacken(session): """Run black. @@ -97,31 +97,31 @@ def unit(session): default(session) -# @nox.session(python=["2.7", "3.6"]) -# def system(session): -# """Run the system test suite.""" +@nox.session(python=["2.7", "3.6"]) +def system(session): + """Run the system test suite.""" -# # Sanity check: Only run system tests if the environment variable is set. -# if not os.environ.get("GOOGLE_APPLICATION_CREDENTIALS", ""): -# session.skip("Credentials must be set via environment variable.") + # Sanity check: Only run system tests if the environment variable is set. + if not os.environ.get("GOOGLE_APPLICATION_CREDENTIALS", ""): + session.skip("Credentials must be set via environment variable.") -# # Use pre-release gRPC for system tests. -# session.install("--pre", "grpcio") + # Use pre-release gRPC for system tests. + session.install("--pre", "grpcio") -# # Install all test dependencies, then install local packages in-place. -# session.install("mock", "pytest") -# for local_dep in LOCAL_DEPS: -# session.install("-e", local_dep) -# systest_deps = ["../test_utils/", "../pubsub", "../kms"] -# for systest_dep in systest_deps: -# session.install("-e", systest_dep) -# session.install("-e", ".") + # Install all test dependencies, then install local packages in-place. + session.install("mock", "pytest") + for local_dep in LOCAL_DEPS: + session.install("-e", local_dep) + systest_deps = ["../test_utils/", "../pubsub", "../kms"] + for systest_dep in systest_deps: + session.install("-e", systest_dep) + session.install("-e", ".") -# # Run py.test against the system tests. -# session.run("py.test", "--quiet", "tests/system.py", *session.posargs) + # Run py.test against the system tests. + session.run("py.test", "--quiet", "tests/system.py", *session.posargs) -@nox.session(python="3.7") +@nox.session(python="3.6") def cover(session): """Run the final coverage report.