Merge pull request #4 from octue/feature/clarify-storage-api

Allow many stores and clarify storage API

.devcontainer/devcontainer.json

@@ -38,6 +38,9 @@
     "python.linting.pylintPath": "/usr/local/py-utils/bin/pylint",
     "python.pythonPath": "/usr/local/bin/python",
     "restructuredtext.confPath": "${workspaceFolder}/docs/source",
+    // Scrolling the editor is a nice idea but it doesn't work, always out of sync and impossible to manage
+    "restructuredtext.preview.scrollEditorWithPreview": false,
+    "restructuredtext.preview.scrollPreviewWithEditor": false,
     "terminal.integrated.defaultProfile.linux": "zsh"
   },
 

.pre-commit-config.yaml

@@ -61,6 +61,8 @@ repos:
     hooks:
       - id: check-commit-message-is-conventional
         stages: [commit-msg]
+        args:
+          - --maximum-body-line-length=2000
 
   - repo: https://github.com/thclark/pre-commit-sphinx
     rev: 0.0.1

README.md

@@ -1,5 +1,5 @@
 [![PyPI version](https://badge.fury.io/py/django_gcp.svg)](https://badge.fury.io/py/django_gcp)
-[![codecov](https://codecov.io/gh/octue/django_gcp/branch/master/graph/badge.svg)](https://codecov.io/gh/octue/django_gcp)
+[![codecov](https://codecov.io/gh/octue/django-gcp/branch/main/graph/badge.svg?token=H2QLSCF3DU)](https://codecov.io/gh/octue/django-gcp)
 [![Documentation](https://readthedocs.org/projects/django_gcp/badge/?version=latest)](https://django_gcp.readthedocs.io/en/latest/?badge=latest)
 [![pre-commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit&logoColor=white)](https://github.com/pre-commit/pre-commit)
 [![black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/ambv/black)
@@ -20,9 +20,7 @@ If so, get in touch for a chat. We're doing fun things with Google Cloud. Way fu
 
 ## All the :heart:
 
-This app is based heavily on [django-storages](), [django-cloud-tasks]() and uses on the [django-rabid-armadillo]() template. Big love.
-
-- Template django app, with:
+This app is based heavily on [django-storages](https://django-storages.readthedocs.io/en/latest/), [django-google-cloud-tasks](https://github.com/flamingo-run/django-cloud-tasks) and uses the [django-rabid-armadillo](https://github.com/thclark/django-rabid-armadillo) template. Big love.
 
 ## Contributing
 

django_gcp/storage/gcloud.py

@@ -2,27 +2,19 @@
 # pylint: disable=arguments-renamed
 
 import mimetypes
-import warnings
-from datetime import timedelta
 from tempfile import SpooledTemporaryFile
-from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
+from django.core.exceptions import SuspiciousOperation
 from django.core.files.base import File
+from django.core.files.storage import Storage
 from django.utils import timezone
 from django.utils.deconstruct import deconstructible
+from google.cloud.exceptions import NotFound
+from google.cloud.storage import Blob, Client
+from google.cloud.storage.blob import _quote
 
-from .base import BaseStorage
 from .compress import CompressedFileMixin, CompressStorageMixin
-from .utils import check_location, clean_name, get_available_overwrite_name, safe_join, setting, to_bytes
-
-
-try:
-    from google.cloud.exceptions import NotFound
-    from google.cloud.storage import Blob, Client
-    from google.cloud.storage.blob import _quote
-except ImportError as e:
-    raise ImproperlyConfigured(
-        "Could not load Google Cloud Storage bindings.\n" "See https://github.com/GoogleCloudPlatform/gcloud-python"
-    ) from e
+from .settings import StorageSettings
+from .utils import clean_name, get_available_overwrite_name, safe_join, setting, to_bytes
 
 
 CONTENT_ENCODING = "content_encoding"
@@ -32,31 +24,32 @@
 class GoogleCloudFile(CompressedFileMixin, File):
     """A django File object representing a GCP storage object"""
 
-    def __init__(self, name, mode, storage):
+    def __init__(self, name, mode, storage):  # pylint: disable=super-init-not-called
         self.name = name
         self.mime_type = mimetypes.guess_type(name)[0]
         self._mode = mode
         self._storage = storage
         self.blob = storage.bucket.get_blob(name)
         if not self.blob and "w" in mode:
-            self.blob = Blob(self.name, storage.bucket, chunk_size=storage.blob_chunk_size)
+            self.blob = Blob(self.name, storage.bucket, chunk_size=storage.settings.blob_chunk_size)
         self._file = None
         self._is_dirty = False
 
-    @property
     def size(self):
         return self.blob.size
 
     def _get_file(self):
         if self._file is None:
             self._file = SpooledTemporaryFile(
-                max_size=self._storage.max_memory_size, suffix=".GSStorageFile", dir=setting("FILE_UPLOAD_TEMP_DIR")
+                max_size=self._storage.settings.max_memory_size,
+                suffix=".GSStorageFile",
+                dir=setting("FILE_UPLOAD_TEMP_DIR"),
             )
             if "r" in self._mode:
                 self._is_dirty = False
                 self.blob.download_to_file(self._file)
                 self._file.seek(0)
-            if self._storage.gzip and self.blob.content_encoding == "gzip":
+            if self._storage.settings.gzip and self.blob.content_encoding == "gzip":
                 self._file = self._decompress_file(mode=self._mode, file=self._file)
         return self._file
 
@@ -91,75 +84,55 @@ def close(self):
                     self.file,
                     rewind=True,
                     content_type=self.mime_type,
-                    predefined_acl=blob_params.get("acl", self._storage.default_acl),
+                    predefined_acl=blob_params.get("acl", self._storage.settings.default_acl),
                 )
             self._file.close()
             self._file = None
 
 
 @deconstructible
-class GoogleCloudStorage(CompressStorageMixin, BaseStorage):
-    """Storage class allowing django to use GCS as an object store"""
+class GoogleCloudStorage(CompressStorageMixin, Storage):
+    """Storage class allowing django to use GCS as an object store
 
-    def __init__(self, **settings):
-        super().__init__(**settings)
+    Instantiates as the `media` store by default. Pass `media`, `static` or any of the keys in"""
 
-        check_location(self)
+    def __init__(self, store_key="media", **overrides):
+        super().__init__()
 
+        self.settings = StorageSettings(store_key, **overrides)
         self._bucket = None
         self._client = None
 
     def get_accessed_time(self, *args, **kwargs):
-        """Not implemented yet"""
-        raise NotImplementedError("Need to override the abstract base class for this method")
+        """Get the last accessed time of the file
+
+        This value is ALWAYS None because we cannot know the last accessed time on a cloud file.
+
+        This method is here for API compatibility with django's Storage class.
+        """
+        return None
 
     def path(self, *args, **kwargs):
-        """Not implemented yet"""
-        raise NotImplementedError("Need to override the abstract base class for this method")
-
-    def get_default_settings(self):
-        return {
-            "project_id": setting("GS_PROJECT_ID"),
-            "credentials": setting("GS_CREDENTIALS"),
-            "bucket_name": setting("GS_BUCKET_NAME"),
-            "custom_endpoint": setting("GS_CUSTOM_ENDPOINT", None),
-            "location": setting("GS_LOCATION", ""),
-            "default_acl": setting("GS_DEFAULT_ACL"),
-            "querystring_auth": setting("GS_QUERYSTRING_AUTH", True),
-            "expiration": setting("GS_EXPIRATION", timedelta(seconds=86400)),
-            "gzip": setting("GS_IS_GZIPPED", False),
-            "gzip_content_types": setting(
-                "GZIP_CONTENT_TYPES",
-                (
-                    "text/css",
-                    "text/javascript",
-                    "application/javascript",
-                    "application/x-javascript",
-                    "image/svg+xml",
-                ),
-            ),
-            "file_overwrite": setting("GS_FILE_OVERWRITE", True),
-            "cache_control": setting("GS_CACHE_CONTROL"),
-            "object_parameters": setting("GS_OBJECT_PARAMETERS", {}),
-            # The max amount of memory a returned file can take up before being
-            # rolled over into a temporary file on disk. Default is 0: Do not
-            # roll over.
-            "max_memory_size": setting("GS_MAX_MEMORY_SIZE", 0),
-            "blob_chunk_size": setting("GS_BLOB_CHUNK_SIZE"),
-        }
+        """Get the local path of the file
+
+        This value is ALWAYS None because the path is not necessarily distinct for an object
+        not on the local filesystem.
+
+        This method is here for API compatibility with django's Storage class.
+        """
 
     @property
     def client(self):
         """The google-storage client for this store"""
         if self._client is None:
-            self._client = Client(project=self.project_id, credentials=self.credentials)
+            self._client = Client(project=self.settings.project_id, credentials=self.settings.credentials)
         return self._client
 
     @property
     def bucket(self):
         """The google-storage bucket object for this store"""
         if self._bucket is None:
-            self._bucket = self.client.bucket(self.bucket_name)
+            self._bucket = self.client.bucket(self.settings.bucket_name)
         return self._bucket
 
     def _normalize_name(self, name):
@@ -170,7 +143,7 @@ def _normalize_name(self, name):
         to is not outside the directory specified by the LOCATION setting.
         """
         try:
-            return safe_join(self.location, name)
+            return safe_join(self.settings.location, name)
         except ValueError:
             raise SuspiciousOperation("Attempted access to '%s' denied." % name)
 
@@ -190,10 +163,14 @@ def _save(self, name, content):
 
         upload_params = {}
         blob_params = self.get_object_parameters(name)
-        upload_params["predefined_acl"] = blob_params.pop("acl", self.default_acl)
+        upload_params["predefined_acl"] = blob_params.pop("acl", self.settings.default_acl)
         upload_params[CONTENT_TYPE] = blob_params.pop(CONTENT_TYPE, file_object.mime_type)
 
-        if self.gzip and upload_params[CONTENT_TYPE] in self.gzip_content_types and CONTENT_ENCODING not in blob_params:
+        if (
+            self.settings.gzip
+            and upload_params[CONTENT_TYPE] in self.settings.gzip_content_types
+            and CONTENT_ENCODING not in blob_params
+        ):
             content = self._compress_content(content)
             blob_params[CONTENT_ENCODING] = "gzip"
 
@@ -206,20 +183,9 @@ def _save(self, name, content):
     def get_object_parameters(self, name):
         """Override this to return a dictionary of overwritable blob-property to value.
 
-        Returns GS_OBJECT_PARAMETRS by default. See the docs for all possible options.
+        Returns GS_OBJECT_PARAMETERS by default. See the docs for all possible options.
         """
-        object_parameters = self.object_parameters.copy()
-
-        if "cache_control" not in object_parameters and self.cache_control:
-            warnings.warn(
-                "The GS_CACHE_CONTROL setting is deprecated. Use GS_OBJECT_PARAMETERS to set any "
-                "writable blob property or override GoogleCloudStorage.get_object_parameters to "
-                "vary the parameters per object.",
-                DeprecationWarning,
-            )
-            object_parameters["cache_control"] = self.cache_control
-
-        return object_parameters
+        return self.settings.object_parameters.copy()
 
     def delete(self, name):
         name = self._normalize_name(clean_name(name))
@@ -306,26 +272,60 @@ def url(self, name):
         name = self._normalize_name(clean_name(name))
         blob = self.bucket.blob(name)
         blob_params = self.get_object_parameters(name)
-        no_signed_url = blob_params.get("acl", self.default_acl) == "publicRead" or not self.querystring_auth
+        no_signed_url = (
+            blob_params.get("acl", self.settings.default_acl) == "publicRead" or not self.settings.querystring_auth
+        )
 
-        if not self.custom_endpoint and no_signed_url:
+        if not self.settings.custom_endpoint and no_signed_url:
             return blob.public_url
         elif no_signed_url:
             return "{storage_base_url}/{quoted_name}".format(
-                storage_base_url=self.custom_endpoint,
+                storage_base_url=self.settings.custom_endpoint,
                 quoted_name=_quote(name, safe=b"/~"),
             )
-        elif not self.custom_endpoint:
-            return blob.generate_signed_url(expiration=self.expiration, version="v4")
+        elif not self.settings.custom_endpoint:
+            return blob.generate_signed_url(expiration=self.settings.expiration, version="v4")
         else:
             return blob.generate_signed_url(
-                bucket_bound_hostname=self.custom_endpoint,
-                expiration=self.expiration,
+                bucket_bound_hostname=self.settings.custom_endpoint,
+                expiration=self.settings.expiration,
                 version="v4",
             )
 
     def get_available_name(self, name, max_length=None):
         name = clean_name(name)
-        if self.file_overwrite:
+        if self.settings.file_overwrite:
             return get_available_overwrite_name(name, max_length)
         return super().get_available_name(name, max_length)
+
+
+class GoogleCloudMediaStorage(GoogleCloudStorage):
+    """Storage whose bucket name is taken from the GCP_STORAGE_MEDIA_NAME setting
+
+    This actually behaves exactly as a default instantitation of the base
+    ``GoogleCloudStorage`` class, but is there to make configuration more
+    explicit for first-timers.
+
+    """
+
+    def __init__(self, **overrides):
+        if overrides.pop("store_key", "media") != "media":
+            raise ValueError("You cannot instantiate GoogleCloudMediaStorage with a store_key other than 'media'")
+        super().__init__(store_key="media", **overrides)
+
+
+class GoogleCloudStaticStorage(GoogleCloudStorage):
+    """Storage defined with an appended bucket name (called "<bucket>-static")
+
+    We define that static files are stored in a different bucket than the (private) media files, which:
+        1. gives us less risk of accidentally setting private files as public
+        2. allows us easier visual inspection in the console of what's private and what's public static
+        3. allows us to set blanket public ACLs on the static bucket
+        4. makes it easier to clean up private files with no entry in the DB
+
+    """
+
+    def __init__(self, **overrides):
+        if overrides.pop("store_key", "static") != "static":
+            raise ValueError("You cannot instantiate GoogleCloudStaticStorage with a store_key other than 'static'")
+        super().__init__(store_key="static", **overrides)