Google: Only verify bucket if auto_create is True (#718)

Motivation for this change is to send fewer requests to Google Storage
API. We do not check existence of bucket unless auto_create_bucket is
configured or exists('') is called.

When creating bucket, instead of making two requests to get and create,
we just create and look for conflict.

Based on the work in #575 and supersedes #412.

storages/backends/gcloud.py

@@ -15,9 +15,8 @@
 )
 
 try:
-    from google.cloud.storage.client import Client
-    from google.cloud.storage.blob import Blob
-    from google.cloud.exceptions import NotFound
+    from google.cloud.storage import Blob, Client
+    from google.cloud.exceptions import Conflict, NotFound
 except ImportError:
     raise ImproperlyConfigured("Could not load Google Cloud Storage bindings.\n"
                                "See https://github.com/GoogleCloudPlatform/gcloud-python")
@@ -129,19 +128,19 @@ def bucket(self):
 
     def _get_or_create_bucket(self, name):
         """
-        Retrieves a bucket if it exists, otherwise creates it.
+        Returns bucket. If auto_create_bucket is True, creates bucket if it
+        doesn't exist.
         """
-        try:
-            return self.client.get_bucket(name)
-        except NotFound:
-            if self.auto_create_bucket:
-                bucket = self.client.create_bucket(name)
-                bucket.acl.save_predefined(self.auto_create_acl)
-                return bucket
-            raise ImproperlyConfigured("Bucket %s does not exist. Buckets "
-                                       "can be automatically created by "
-                                       "setting GS_AUTO_CREATE_BUCKET to "
-                                       "``True``." % name)
+        bucket = self.client.bucket(name)
+        if self.auto_create_bucket:
+            try:
+                new_bucket = self.client.create_bucket(name)
+                new_bucket.acl.save_predefined(self.auto_create_acl)
+                return new_bucket
+            except Conflict:
+                # Bucket already exists
+                pass
+        return bucket
 
     def _normalize_name(self, name):
         """
@@ -191,9 +190,9 @@ def delete(self, name):
     def exists(self, name):
         if not name:  # root element aka the bucket
             try:
-                self.bucket
+                self.client.get_bucket(self.bucket)
                 return True
-            except ImproperlyConfigured:
+            except NotFound:
                 return False
 
         name = self._normalize_name(clean_name(name))

tests/test_gcloud.py

@@ -12,7 +12,7 @@
 from django.core.files.base import ContentFile
 from django.test import TestCase
 from django.utils import timezone
-from google.cloud.exceptions import NotFound
+from google.cloud.exceptions import Conflict, NotFound
 from google.cloud.storage.blob import Blob
 
 from storages.backends import gcloud
@@ -41,7 +41,7 @@ def test_open_read(self):
         data = b'This is some test read data.'
 
         f = self.storage.open(self.filename)
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.get_blob.assert_called_with(self.filename)
 
         f.blob.download_to_file = lambda tmpfile: tmpfile.write(data)
@@ -52,7 +52,7 @@ def test_open_read_num_bytes(self):
         num_bytes = 10
 
         f = self.storage.open(self.filename)
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.get_blob.assert_called_with(self.filename)
 
         f.blob.download_to_file = lambda tmpfile: tmpfile.write(data)
@@ -102,7 +102,7 @@ def test_save(self):
 
         self.storage.save(self.filename, content)
 
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.get_blob().upload_from_file.assert_called_with(
             content, rewind=True, size=len(data), content_type=mimetypes.guess_type(self.filename)[0])
 
@@ -113,7 +113,7 @@ def test_save2(self):
 
         self.storage.save(filename, content)
 
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.get_blob().upload_from_file.assert_called_with(
             content, rewind=True, size=len(data), content_type=mimetypes.guess_type(filename)[0])
 
@@ -129,15 +129,15 @@ def test_save_with_default_acl(self):
 
         self.storage.save(filename, content)
 
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.get_blob().upload_from_file.assert_called_with(
             content, rewind=True, size=len(data), content_type=mimetypes.guess_type(filename)[0],
             predefined_acl='publicRead')
 
     def test_delete(self):
         self.storage.delete(self.filename)
 
-        self.storage._client.get_bucket.assert_called_with(self.bucket_name)
+        self.storage._client.bucket.assert_called_with(self.bucket_name)
         self.storage._bucket.delete_blob.assert_called_with(self.filename)
 
     def test_exists(self):
@@ -160,12 +160,22 @@ def test_exists_bucket(self):
         # exists('') should return True if the bucket exists
         self.assertTrue(self.storage.exists(''))
 
+    def test_exists_no_bucket_auto_create(self):
+        # exists('') should return true when auto_create_bucket is configured
+        # and bucket already exists
+        # exists('') should automatically create the bucket if
+        # auto_create_bucket is configured
+        self.storage.auto_create_bucket = True
+        self.storage._client = mock.MagicMock()
+        self.storage._client.create_bucket.side_effect = Conflict('dang')
+
+        self.assertTrue(self.storage.exists(''))
+
     def test_exists_bucket_auto_create(self):
         # exists('') should automatically create the bucket if
         # auto_create_bucket is configured
         self.storage.auto_create_bucket = True
         self.storage._client = mock.MagicMock()
-        self.storage._client.get_bucket.side_effect = NotFound('dang')
 
         self.assertTrue(self.storage.exists(''))
         self.storage._client.create_bucket.assert_called_with(self.bucket_name)
@@ -383,7 +393,7 @@ def test_cache_control(self):
         self.storage.cache_control = cache_control
         self.storage.save(filename, content)
 
-        bucket = self.storage.client.get_bucket(self.bucket_name)
+        bucket = self.storage.client.bucket(self.bucket_name)
         blob = bucket.get_blob(filename)
         self.assertEqual(blob.cache_control, cache_control)