Merge pull request #40 from octue/improve-docs

Add on_change callback plus minor fixes

Author: thclark

.devcontainer/devcontainer.json

@@ -71,7 +71,8 @@
         "thebarkman.vscode-djaneiro",
         "trond-snekvik.simple-rst",
         "ms-azuretools.vscode-docker",
-        "ryanluker.vscode-coverage-gutters"
+        "ryanluker.vscode-coverage-gutters",
+        "ms-python.black-formatter"
       ]
     }
   },

.pre-commit-config.yaml

@@ -20,7 +20,7 @@ repos:
       - id: check-json5
 
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.10.0
     hooks:
       - id: black
         args: ['--line-length', '120']

django_gcp/__init__.py

@@ -1 +1 @@
-default_app_config = "django_gcp.apps.DjangoGcpAppConfig"
+default_app_config = "django_gcp.apps.DjangoGCPAppConfig"

django_gcp/storage/fields.py

@@ -59,8 +59,8 @@ class BlobField(models.JSONField):
     We do this to support a clear and maintainable implementation.
 
     :param ingress_to: A string defining the path within the bucket to which direct uploads
-    will be ingressed, if temporary_ingress=True. These uploaded files will be moved to their
-    ultimate path in the store on save of the model.
+    will be ingressed. These uploaded files will be moved to their ultimate path in the store on save of the model.
+
     :param accept_mimetype: A string passed to widgets, suitable for use in the `accept` attribute
     of an html filepicker. This will allow you to narrow down, eg to `image/*` or an even more
     specific mimetype. No validation is done at the field level that objects actually are of this
@@ -77,7 +77,7 @@ class BlobField(models.JSONField):
 
     :param max_size_bytes: The maximum size in bytes of files that can be uploaded.
 
-    :overwrite_mode: One of `OVERWRITE_MODES` determining the circumstances under which overwrite
+    :param overwrite_mode: One of `OVERWRITE_MODES` determining the circumstances under which overwrite
     is allowed. overwrite mode behaves as follows:
     - never: Never allows overwrite
     - update: Only when updating an object
@@ -86,6 +86,10 @@ class BlobField(models.JSONField):
     - add-versioned: Only when adding an object to a versioned bucket (again, for completeness)
     - add-update: Always allow (ie when adding or updating an object)
     - add-update-versioned: When adding or updating an object to a versioned bucket
+
+    :param on_change: A callable that will be executed on change of the field value. This will be called
+    on commit of the transaction (ie once any file upload is ingressed to its final location) and allows you,
+    for example, to dispatch a worker task to further process the uploaded blob.
     """
 
     description = _("A GCP Cloud Storage object")
@@ -99,6 +103,7 @@ def __init__(
         accept_mimetype=DEFAULT_ACCEPT_MIMETYPE,
         max_size_bytes=DEFAULT_MAX_SIZE_BYTES,
         overwrite_mode=DEFAULT_OVERWRITE_MODE,
+        on_change=None,
         **kwargs,
     ):
         self._versioning_enabled = None
@@ -112,6 +117,7 @@ def __init__(
         self.store_key = store_key
         self.accept_mimetype = accept_mimetype
         self.max_size_bytes = max_size_bytes
+        self.on_change = on_change
         kwargs["default"] = kwargs.pop("default", None)
         kwargs["help_text"] = kwargs.pop("help_text", "GCP cloud storage object")
 
@@ -144,6 +150,7 @@ def check(self, **kwargs):
             *self._check_get_destination_path(),
             *self._check_ingress_to(),
             *self._check_overwrite_mode(),
+            *self._check_on_change(),
         ]
 
     def deconstruct(self):
@@ -153,6 +160,7 @@ def deconstruct(self):
         kwargs["accept_mimetype"] = self.accept_mimetype
         kwargs["overwrite_mode"] = self.overwrite_mode
         kwargs["get_destination_path"] = self.get_destination_path
+        kwargs["on_change"] = self.on_change
         return name, path, args, kwargs
 
     def formfield(self, **kwargs):
@@ -172,7 +180,11 @@ def formfield(self, **kwargs):
     @property
     def override_blobfield_value(self):
         """Shortcut to access the GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE setting"""
-        return getattr(settings, "GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE", DEFAULT_OVERRIDE_BLOBFIELD_VALUE)
+        return getattr(
+            settings,
+            "GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE",
+            DEFAULT_OVERRIDE_BLOBFIELD_VALUE,
+        )
 
     def pre_save(self, model_instance, add):
         """Return field's value just before saving."""
@@ -184,7 +196,12 @@ def pre_save(self, model_instance, add):
         # explicitly for the purpose of data migration and manipulation. You should never allow an untrusted
         # client to set paths directly, because knowing the path of a pre-existing object allows you to assume
         # access to it. Tip: You can use django's override_settings context manager to set this temporarily.
-        if getattr(settings, "GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE", DEFAULT_OVERRIDE_BLOBFIELD_VALUE):
+        # Note that you'll have to execute any on_changed
+        if getattr(
+            settings,
+            "GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE",
+            DEFAULT_OVERRIDE_BLOBFIELD_VALUE,
+        ):
             logger.warning(
                 "Overriding %s value to %s",
                 self.__class__.__name__,
@@ -206,8 +223,15 @@ def pre_save(self, model_instance, add):
             elif adding_blank or updating_valid_to_blank:
                 new_value = None
 
-            elif adding_valid or updating_blank_to_valid or updating_valid_to_valid:
+                # Trigger the on_change callback at the end of the commit when we know the
+                # database transaction will work
+                def on_commit_blank():
+                    if self.on_change is not None:
+                        self.on_change(new_value, instance=model_instance)
+
+                transaction.on_commit(on_commit_blank)
 
+            elif adding_valid or updating_blank_to_valid or updating_valid_to_valid:
                 new_value = {}
                 new_value["path"], allow_overwrite = self.get_destination_path(
                     instance=model_instance,
@@ -228,8 +252,8 @@ def pre_save(self, model_instance, add):
                 # capture the dual edge cases of the file not moving correctly, and the database
                 # row not saving (eg due to validation errors in other model fields).
                 # https://stackoverflow.com/questions/33180727/trigering-post-save-signal-only-after-transaction-has-completed
-                transaction.on_commit(
-                    lambda: copy_blob(
+                def on_commit_valid():
+                    copy_blob(
                         self.storage.bucket,
                         value["_tmp_path"],
                         self.storage.bucket,
@@ -238,9 +262,15 @@ def pre_save(self, model_instance, add):
                         overwrite=allow_overwrite,
                         attributes=value.get("attributes", None),
                     )
-                )
+                    if self.on_change is not None:
+                        self.on_change(new_value, instance=model_instance)
+
+                transaction.on_commit(on_commit_valid)
+
                 logger.info(
-                    "Registered move of %s to %s to happen on transaction commit", value["_tmp_path"], new_value["path"]
+                    "Registered move of %s to %s to happen on transaction commit",
+                    value["_tmp_path"],
+                    new_value["path"],
                 )
 
             else:
@@ -407,6 +437,17 @@ def _check_get_destination_path(self):
             ]
         return []
 
+    def _check_on_change(self):
+        if self.on_change is not None and not callable(self.on_change):
+            return [
+                checks.Error(
+                    f"'on_change' argument in {self.__class__.__name__} must be or a callable function, or None",
+                    obj=self,
+                    id="fields.E201",
+                )
+            ]
+        return []
+
     def _get_allow_overwrite(self, add):
         """Return a boolean determining if overwrite should be allowed for this operation"""
         mode_map = {

django_gcp/storage/forms.py

@@ -12,59 +12,3 @@ class CloudObjectFormField(JSONField):
 
     empty_value = None
     widget = CloudObjectWidget
-
-
-# from django.forms import FileField
-# from django.core.files.uploadedfile import UploadedFile
-# from django.core.exceptions import ValidationError
-
-# from .direct_upload_widgets import DirectUploadWidget
-# class DirectUploadedFile(UploadedFile):
-#     """Represents a file that was uploaded direct to bucket"""
-
-
-# class DirectUploadFormField(FileField):
-#     """Overrides the usual forms.FileField with a widget that handles direct uploads"""
-
-#     widget = DirectUploadWidget
-
-#     def to_python(self, data):
-#         """Override the FileField to_python method to accept data as a string file name"""
-
-#         if data in self.empty_values:
-#             return None
-
-#         # A file was attached as an indirect upload (the normal way)
-#         if isinstance(data, UploadedFile):
-#             try:
-#                 file_name = data.name
-#                 file_size = data.size
-#             except AttributeError:
-#                 # pylint: disable=raise-missing-from
-#                 raise ValidationError(self.error_messages["invalid"], code="invalid")
-#                 # pylint: enable=raise-missing-from
-
-#             if not self.allow_empty_file and not file_size:
-#                 raise ValidationError(self.error_messages["empty"], code="empty")
-
-#         # A file was uploaded direct-to-bucket, data only contains its name
-#         elif isinstance(data, str):
-#             file_name = data
-#             file_size = None
-#             data = DirectUploadedFile(name=file_name, size=None)
-#             if not self.allow_empty_file:
-#                 logger.warning(
-#                     "`allow_empty_file` is set but file has been directly uploaded. Cannot validate emptiness of file."
-#                 )
-
-#         else:
-#             raise ValueError(f"Unknown data of class {data.__class__}: {data}")
-#         print("FILE NAME", file_name)
-#         if self.max_length is not None and len(file_name) > self.max_length:
-#             params = {"max": self.max_length, "length": len(file_name)}
-#             raise ValidationError(self.error_messages["max_length"], code="max_length", params=params)
-
-#         if not file_name:
-#             raise ValidationError(self.error_messages["invalid"], code="invalid")
-
-#         return data

django_gcp/storage/gcloud.py

@@ -1,6 +1,7 @@
 import logging
 import mimetypes
 from tempfile import SpooledTemporaryFile
+from django.conf import settings
 from django.core.exceptions import SuspiciousOperation
 from django.core.files.base import File
 from django.core.files.storage import Storage
@@ -12,7 +13,7 @@
 
 from .compress import CompressedFileMixin, CompressStorageMixin
 from .settings import StorageSettings
-from .utils import clean_name, get_available_overwrite_name, safe_join, setting, to_bytes
+from .utils import clean_name, get_available_overwrite_name, safe_join, to_bytes
 
 
 CONTENT_ENCODING = "content_encoding"
@@ -44,7 +45,7 @@ def _get_file(self):
             self._file = SpooledTemporaryFile(
                 max_size=self._storage.settings.max_memory_size,
                 suffix=".GSStorageFile",
-                dir=setting("FILE_UPLOAD_TEMP_DIR"),
+                dir=getattr(settings, "FILE_UPLOAD_TEMP_DIR"),
             )
             if "r" in self._mode:
                 self._is_dirty = False
@@ -286,7 +287,7 @@ def get_modified_time(self, name):
         name = self._normalize_name(clean_name(name))
         blob = self._get_blob(name)
         updated = blob.updated
-        return updated if setting("USE_TZ") else timezone.make_naive(updated)
+        return updated if getattr(settings, "USE_TZ") else timezone.make_naive(updated)
 
     def get_created_time(self, name):
         """
@@ -296,7 +297,7 @@ def get_created_time(self, name):
         name = self._normalize_name(clean_name(name))
         blob = self._get_blob(name)
         created = blob.time_created
-        return created if setting("USE_TZ") else timezone.make_naive(created)
+        return created if getattr(settings, "USE_TZ") else timezone.make_naive(created)
 
     def url(self, name):
         """

django_gcp/storage/utils.py

@@ -1,6 +1,5 @@
 import os
 import posixpath
-from django.conf import settings
 from django.core.exceptions import SuspiciousFileOperation
 from django.utils.encoding import force_bytes
 
@@ -13,37 +12,24 @@ def to_bytes(content):
     return force_bytes(content)
 
 
-def setting(name, default=None):
-    """
-    Helper function to get a Django setting by name. If setting doesn't exists
-    it will return a default.
-
-    :param name: Name of setting
-    :type name: str
-    :param default: Value if setting is unfound
-    :returns: Setting's value
-    """
-    return getattr(settings, name, default)
-
-
 def clean_name(name):
     """
     Cleans the name so that Windows style paths work
     """
     # Normalize Windows style paths
-    clean_name = posixpath.normpath(name).replace("\\", "/")
+    cleaned = posixpath.normpath(name).replace("\\", "/")
 
     # os.path.normpath() can strip trailing slashes so we implement
     # a workaround here.
-    if name.endswith("/") and not clean_name.endswith("/"):
+    if name.endswith("/") and not cleaned.endswith("/"):
         # Add a trailing slash as it was stripped.
-        clean_name = clean_name + "/"
+        cleaned = cleaned + "/"
 
     # Given an empty string, os.path.normpath() will return ., which we don't want
-    if clean_name == ".":
-        clean_name = ""
+    if cleaned == ".":
+        cleaned = ""
 
-    return clean_name
+    return cleaned
 
 
 def safe_join(base, *paths):
@@ -77,12 +63,13 @@ def safe_join(base, *paths):
     # the base path is /.
     base_path_len = len(base_path)
     if not final_path.startswith(base_path) or final_path[base_path_len] != "/":
-        raise ValueError("the joined path is located outside of the base path" " component")
+        raise ValueError("the joined path is located outside of the base path component")
 
     return final_path.lstrip("/")
 
 
 def get_available_overwrite_name(name, max_length):
+    """Truncate a filename to obey max_length limit"""
     if max_length is None or len(name) <= max_length:
         return name
 
@@ -94,8 +81,6 @@ def get_available_overwrite_name(name, max_length):
     file_root = file_root[:-truncation]
     if not file_root:
         raise SuspiciousFileOperation(
-            'Storage tried to truncate away entire filename "%s". '
-            "Please make sure that the corresponding file field "
-            'allows sufficient "max_length".' % name
+            f'Storage tried to truncate away entire filename "{name}". Please make sure that the corresponding file field allows sufficient "max_length".'
         )
-    return os.path.join(dir_name, "{}{}".format(file_root, file_ext))
+    return os.path.join(dir_name, f"{file_root}{file_ext}")

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "django-gcp"
-version = "0.9.3"
+version = "0.10.0"
 description = "Utilities to run Django on Google Cloud Platform"
 authors = ["Tom Clark"]
 license = "MIT"