diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index c3b104edd..5be46832b 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -32,7 +32,11 @@ STORAGE_EMULATOR_ENV_VAR = "STORAGE_EMULATOR_HOST" """Environment variable defining host for Storage emulator.""" -_DEFAULT_STORAGE_HOST = u"https://storage.googleapis.com" +_DEFAULT_STORAGE_HOST = "https://storage.googleapis.com" +"""Default storage host for JSON API.""" + +_BASE_STORAGE_URI = "storage.googleapis.com" +"""Base request endpoint URI for JSON API.""" # etag match parameters in snake case and equivalent header _ETAG_MATCH_PARAMETERS = ( diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index a62ed711f..76fa1a9cf 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -33,6 +33,7 @@ from google.cloud.exceptions import NotFound from google.cloud.storage._helpers import _get_environ_project from google.cloud.storage._helpers import _get_storage_host +from google.cloud.storage._helpers import _BASE_STORAGE_URI 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 @@ -146,7 +147,7 @@ def __init__( # STORAGE_EMULATOR_HOST or a non-default api_endpoint is set. if ( kw_args["api_endpoint"] is not None - and kw_args["api_endpoint"].find("storage.googleapis.com") < 0 + and _BASE_STORAGE_URI not in kw_args["api_endpoint"] ): if credentials is None: credentials = AnonymousCredentials() @@ -932,12 +933,22 @@ def create_bucket( """ bucket = self._bucket_arg_to_bucket(bucket_or_name) + query_params = {} if project is None: project = self.project - if project is None: - raise ValueError("Client project not set: pass an explicit project.") + # Use no project if STORAGE_EMULATOR_HOST is set + if _BASE_STORAGE_URI not in _get_storage_host(): + if project is None: + project = _get_environ_project() + if project is None: + project = "" + + # Only include the project parameter if a project is set. + # If a project is not set, falls back to API validation (BadRequest). + if project is not None: + query_params = {"project": project} if requester_pays is not None: warnings.warn( @@ -947,8 +958,6 @@ 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 @@ -1375,13 +1384,22 @@ def list_buckets( :returns: Iterator of all :class:`~google.cloud.storage.bucket.Bucket` belonging to this project. """ + extra_params = {} + if project is None: project = self.project - if project is None: - raise ValueError("Client project not set: pass an explicit project.") + # Use no project if STORAGE_EMULATOR_HOST is set + if _BASE_STORAGE_URI not in _get_storage_host(): + if project is None: + project = _get_environ_project() + if project is None: + project = "" - extra_params = {"project": project} + # Only include the project parameter if a project is set. + # If a project is not set, falls back to API validation (BadRequest). + if project is not None: + extra_params = {"project": project} if prefix is not None: extra_params["prefix"] = prefix diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 2f76041bd..52a1bea44 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -23,15 +23,15 @@ import unittest import urllib - from google.api_core import exceptions - +from google.auth.credentials import AnonymousCredentials from google.oauth2.service_account import Credentials -from . import _read_local_json +from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR from google.cloud.storage.retry import DEFAULT_RETRY from google.cloud.storage.retry import DEFAULT_RETRY_IF_GENERATION_SPECIFIED +from . import _read_local_json _SERVICE_ACCOUNT_JSON = _read_local_json("url_signer_v4_test_account.json") _CONFORMANCE_TESTS = _read_local_json("url_signer_v4_test_data.json")[ @@ -237,9 +237,6 @@ def test_ctor_mtls(self): self.assertEqual(client._connection.API_BASE_URL, "http://foo") def test_ctor_w_emulator_wo_project(self): - from google.auth.credentials import AnonymousCredentials - from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR - # avoids authentication if STORAGE_EMULATOR_ENV_VAR is set host = "http://localhost:8080" environ = {STORAGE_EMULATOR_ENV_VAR: host} @@ -259,9 +256,6 @@ def test_ctor_w_emulator_wo_project(self): self.assertIsInstance(client._connection.credentials, AnonymousCredentials) def test_ctor_w_emulator_w_environ_project(self): - from google.auth.credentials import AnonymousCredentials - from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR - # avoids authentication and infers the project from the environment host = "http://localhost:8080" environ_project = "environ-project" @@ -277,9 +271,6 @@ def test_ctor_w_emulator_w_environ_project(self): self.assertIsInstance(client._connection.credentials, AnonymousCredentials) def test_ctor_w_emulator_w_project_arg(self): - from google.auth.credentials import AnonymousCredentials - from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR - # project argument overrides project set in the enviroment host = "http://localhost:8080" environ_project = "environ-project" @@ -296,7 +287,6 @@ def test_ctor_w_emulator_w_project_arg(self): self.assertIsInstance(client._connection.credentials, AnonymousCredentials) def test_create_anonymous_client(self): - from google.auth.credentials import AnonymousCredentials from google.cloud.storage._http import Connection klass = self._get_target_class() @@ -1187,11 +1177,91 @@ def test_lookup_bucket_hit_w_retry(self): ) def test_create_bucket_w_missing_client_project(self): + from google.cloud.exceptions import BadRequest + credentials = _make_credentials() client = self._make_one(project=None, credentials=credentials) - with self.assertRaises(ValueError): - client.create_bucket("bucket") + client._post_resource = mock.Mock() + client._post_resource.side_effect = BadRequest("Required parameter: project") + + bucket_name = "bucket-name" + + with self.assertRaises(BadRequest): + client.create_bucket(bucket_name) + + expected_path = "/b" + expected_data = {"name": bucket_name} + # no required parameter: project + expected_query_params = {} + client._post_resource.assert_called_once_with( + expected_path, + expected_data, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=mock.ANY, + ) + + def test_create_bucket_w_missing_client_project_w_emulator(self): + # mock STORAGE_EMULATOR_ENV_VAR is set + host = "http://localhost:8080" + environ = {STORAGE_EMULATOR_ENV_VAR: host} + with mock.patch("os.environ", environ): + client = self._make_one() + + bucket_name = "bucket-name" + api_response = {"name": bucket_name} + client._post_resource = mock.Mock() + client._post_resource.return_value = api_response + + # mock STORAGE_EMULATOR_ENV_VAR is set + with mock.patch("os.environ", environ): + bucket = client.create_bucket(bucket_name) + + expected_path = "/b" + expected_data = api_response + expected_query_params = {"project": ""} + client._post_resource.assert_called_once_with( + expected_path, + expected_data, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=bucket, + ) + + def test_create_bucket_w_environ_project_w_emulator(self): + # mock STORAGE_EMULATOR_ENV_VAR is set + host = "http://localhost:8080" + environ_project = "environ-project" + environ = { + STORAGE_EMULATOR_ENV_VAR: host, + "GOOGLE_CLOUD_PROJECT": environ_project, + } + with mock.patch("os.environ", environ): + client = self._make_one() + + bucket_name = "bucket-name" + api_response = {"name": bucket_name} + client._post_resource = mock.Mock() + client._post_resource.return_value = api_response + + # mock STORAGE_EMULATOR_ENV_VAR is set + with mock.patch("os.environ", environ): + bucket = client.create_bucket(bucket_name) + + expected_path = "/b" + expected_data = api_response + expected_query_params = {"project": environ_project} + client._post_resource.assert_called_once_with( + expected_path, + expected_data, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=bucket, + ) def test_create_bucket_w_conflict_w_user_project(self): from google.cloud.exceptions import Conflict @@ -1787,12 +1857,112 @@ def test_list_blobs_w_explicit_w_user_project(self): ) def test_list_buckets_wo_project(self): + from google.cloud.exceptions import BadRequest + from google.cloud.storage.client import _item_to_bucket + credentials = _make_credentials() client = self._make_one(project=None, credentials=credentials) - with self.assertRaises(ValueError): + client._list_resource = mock.Mock() + client._list_resource.side_effect = BadRequest("Required parameter: project") + + with self.assertRaises(BadRequest): client.list_buckets() + expected_path = "/b" + expected_item_to_value = _item_to_bucket + expected_page_token = None + expected_max_results = None + expected_page_size = None + # no required parameter: project + expected_extra_params = { + "projection": "noAcl", + } + client._list_resource.assert_called_once_with( + expected_path, + expected_item_to_value, + page_token=expected_page_token, + max_results=expected_max_results, + extra_params=expected_extra_params, + page_size=expected_page_size, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + def test_list_buckets_wo_project_w_emulator(self): + from google.cloud.storage.client import _item_to_bucket + + # mock STORAGE_EMULATOR_ENV_VAR is set + host = "http://localhost:8080" + environ = {STORAGE_EMULATOR_ENV_VAR: host} + with mock.patch("os.environ", environ): + client = self._make_one() + + client._list_resource = mock.Mock(spec=[]) + + # mock STORAGE_EMULATOR_ENV_VAR is set + with mock.patch("os.environ", environ): + client.list_buckets() + + expected_path = "/b" + expected_item_to_value = _item_to_bucket + expected_page_token = None + expected_max_results = None + expected_page_size = None + expected_extra_params = { + "project": "", + "projection": "noAcl", + } + client._list_resource.assert_called_once_with( + expected_path, + expected_item_to_value, + page_token=expected_page_token, + max_results=expected_max_results, + extra_params=expected_extra_params, + page_size=expected_page_size, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + def test_list_buckets_w_environ_project_w_emulator(self): + from google.cloud.storage.client import _item_to_bucket + + # mock STORAGE_EMULATOR_ENV_VAR is set + host = "http://localhost:8080" + environ_project = "environ-project" + environ = { + STORAGE_EMULATOR_ENV_VAR: host, + "GOOGLE_CLOUD_PROJECT": environ_project, + } + with mock.patch("os.environ", environ): + client = self._make_one() + + client._list_resource = mock.Mock(spec=[]) + + # mock STORAGE_EMULATOR_ENV_VAR is set + with mock.patch("os.environ", environ): + client.list_buckets() + + expected_path = "/b" + expected_item_to_value = _item_to_bucket + expected_page_token = None + expected_max_results = None + expected_page_size = None + expected_extra_params = { + "project": environ_project, + "projection": "noAcl", + } + client._list_resource.assert_called_once_with( + expected_path, + expected_item_to_value, + page_token=expected_page_token, + max_results=expected_max_results, + extra_params=expected_extra_params, + page_size=expected_page_size, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + def test_list_buckets_w_defaults(self): from google.cloud.storage.client import _item_to_bucket