diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e36c63..0b07144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] +### Added + +- Added `source_filename` and `original_filesize` fields to all base models, and updated `update_from_asset_data()` to set them accordingly. +- Added `original_height` and `original_width` fields to image and video base models, and updated `update_from_asset_data()` to set them accordingly. +- Added "What to ask of Bynder" section to `README.md`. +- Improved test coverage + +### Removed + +- Removed the `metadata` field from all base models, along with `directly_mapped_bynder_fields` attributes, and the `extract_relevant_metadata()` method used for setting the field value during an update. +- Removed the `bynder_original_filename` field from all base models. This has now been succeeded by `source_filename`, which stores a value more relevant to each type. +- Removed the `bynder_id_hash` field from all base models. +- Removed the `download_asset_file()` method from all base models. The responsibility for downloading assets now falls to the `update_file()` method (applies to image and document models only). + +### Changed + +- The `bynder_id` field on all base models now supports `null` values, allowing a mix of images/documents from different sources to be added to be saved. +- Fixed an issue with `file_hash` and `file_size` fields not being set correctly when a model instance is created or updated to reflect Bynder asset data. +- Updated `asset_file_has_changed()` implementations on all models to take into account values from new `source_filename`, `original_filesize`, `original_height` and `original_width` model fields. +- Consistently raise `wagtail_bynder.exceptions.BynderAssetDataError` (instead of `django.core.exceptions.ImproperlyConfigured` or `KeyError`) when API representations from Bynder do not contain data required for the integration to work properly. +- Changed the default `BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME` setting value from `"Web-Primary"` to `"WebPrimary"` to reflect new guidance in `README.md`. +- Changed the default `BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME` setting value from `"Web-Fallback"` to `"WebFallback"` to reflect new guidance in `README.md`. +- Changed the default `BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME` setting value from `"webimage"` to `"WagtailSource"` to reflect new guidance in `README.md`. + ## [0.2] - 2024-02-25 ### Added diff --git a/README.md b/README.md index 34bde53..a9e199a 100644 --- a/README.md +++ b/README.md @@ -244,9 +244,7 @@ and only needs to have basic read permissions. ### `BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME` -Example: `"WagtailSource"` - -Default: `"webimage"` +Default: `"WagtailSource"` The name of the automatically generated derivative that should be downloaded and used as the `file` value for the representative Wagtail image (as it appears in `thumbnails` in the API representation). @@ -263,11 +261,11 @@ Default: `None` ### `BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME` -Default: `"Web-Primary"` +Default: `"WebPrimary"` ### `BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME` -Default: `"Web-Fallback"` +Default: `"WebFallback"` ### `BYNDER_VIDEO_POSTER_IMAGE_DERIVATIVE_NAME` diff --git a/src/wagtail_bynder/exceptions.py b/src/wagtail_bynder/exceptions.py new file mode 100644 index 0000000..b1f4d38 --- /dev/null +++ b/src/wagtail_bynder/exceptions.py @@ -0,0 +1,5 @@ +class BynderAssetDataError(Exception): + """ + Raised when values expected to be present in an asset API representation + from Bynder are missing. + """ diff --git a/src/wagtail_bynder/models.py b/src/wagtail_bynder/models.py index 09c823d..3d36ed2 100644 --- a/src/wagtail_bynder/models.py +++ b/src/wagtail_bynder/models.py @@ -1,14 +1,11 @@ import logging import math -import os from datetime import datetime from mimetypes import guess_type from typing import Any from django.conf import settings -from django.core.exceptions import ImproperlyConfigured -from django.core.serializers.json import DjangoJSONEncoder from django.db import models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ @@ -20,6 +17,8 @@ from wagtail_bynder import utils +from .exceptions import BynderAssetDataError + logger = logging.getLogger("wagtail.images") @@ -33,14 +32,20 @@ class BynderAssetMixin(models.Model): unique=True, editable=False, db_index=True, - ) - bynder_id_hash = models.CharField(max_length=100, blank=True, editable=False) - bynder_original_filename = models.CharField( - max_length=255, blank=True, editable=False + null=True, ) bynder_last_modified = models.DateTimeField( null=True, editable=False, db_index=True ) + source_filename = models.CharField( + verbose_name=_("source filename"), + max_length=255, + blank=True, + editable=False, + ) + original_filesize = models.IntegerField( + verbose_name=_("original filesize"), editable=False, null=True + ) # Fields for broader use description = models.TextField(verbose_name=_("description"), blank=True) @@ -55,37 +60,15 @@ class BynderAssetMixin(models.Model): is_public = models.BooleanField( verbose_name=_("asset is marked as public"), default=False ) - metadata = models.JSONField( - verbose_name=_("additional metadata"), - default=dict, - encoder=DjangoJSONEncoder, - blank=True, - ) extra_admin_form_fields = ( "description", "copyright", - "metadata", "is_archived", "is_limited_use", "is_public", ) - directly_mapped_bynder_fields = ( - "id", - "name", - "copyright", - "description", - "idHash", - "dateModified", - "archive", - "limited", - "isPublic", - "extension", - "original", - "type", - ) - extra_search_fields = [ index.SearchField("bynder_id", boost=3), index.SearchField("description"), @@ -121,9 +104,7 @@ def update_from_asset_data( self.copyright = asset_data.get("copyright", self.copyright) self.description = asset_data.get("description", self.description) self.collection = self.get_target_collection(asset_data) - self.bynder_id_hash = asset_data["idHash"] self.bynder_last_modified = asset_data["dateModified"] - self.metadata = self.extract_relevant_metadata(asset_data) self.is_archived = bool(asset_data.get("archive", 0)) self.is_limited_use = bool(asset_data.get("limited", 0)) self.is_public = bool(asset_data.get("isPublic", 0)) @@ -131,10 +112,6 @@ def update_from_asset_data( def get_target_collection(self, asset_data: dict[str, Any]) -> Collection: return utils.get_default_collection() - def extract_relevant_metadata(self, asset_data: dict[str, Any]) -> dict[str, Any]: - directly_mapped = set(self.directly_mapped_bynder_fields) - return {k: v for k, v in asset_data.items() if k not in directly_mapped} - class BynderAssetWithFileMixin(BynderAssetMixin): extra_search_fields = BynderAssetMixin.extra_search_fields + [ @@ -145,18 +122,34 @@ class BynderAssetWithFileMixin(BynderAssetMixin): class Meta: abstract = True + @staticmethod + def extract_file_source(asset_data: dict[str, Any]) -> str: + raise NotImplementedError + def update_from_asset_data(self, asset_data: dict[str, Any]) -> None: super().update_from_asset_data(asset_data) - if self.asset_file_has_changed(asset_data): - self.bynder_original_filename = os.path.basename(asset_data["original"]) - self.file = self.download_asset_file(asset_data) + if not self.file or self.asset_file_has_changed(asset_data): + self.update_file(asset_data) def asset_file_has_changed(self, asset_data: dict[str, Any]) -> bool: - filename = os.path.basename(asset_data["original"]) - return self.bynder_original_filename != filename + source_url = self.extract_file_source(asset_data) + filename = utils.filename_from_url(source_url) + return ( + self.source_filename != filename + or self.original_filesize is None + or self.original_filesize != int(asset_data["fileSize"]) + ) - def download_asset_file(self, asset_data: dict[str, Any]) -> utils.DownloadedFile: - raise NotImplementedError + def update_file(self, asset_data: dict[str, Any]) -> None: + source_url = self.extract_file_source(asset_data) + self.file = utils.download_asset(source_url) + + # Used to trigger additional updates on save() + self._file_changed = True + + # Update supplementary field values + self.source_filename = utils.filename_from_url(source_url) + self.original_filesize = int(asset_data["fileSize"]) class BynderSyncedImage(BynderAssetWithFileMixin, AbstractImage): @@ -164,17 +157,25 @@ class BynderSyncedImage(BynderAssetWithFileMixin, AbstractImage): Image.admin_form_fields + BynderAssetMixin.extra_admin_form_fields ) - directly_mapped_bynder_fields = BynderAssetMixin.directly_mapped_bynder_fields + ( - "activeOriginalFocusPoint", + original_width = models.IntegerField( + verbose_name=_("original width"), null=True, editable=False + ) + original_height = models.IntegerField( + verbose_name=_("original height"), null=True, editable=False ) search_fields = ( AbstractImage.search_fields + BynderAssetWithFileMixin.extra_search_fields ) - class Meta: + class Meta(AbstractImage.Meta): abstract = True + def save(self, *args, **kwargs): + if getattr(self, "_file_changed", False): + self._set_image_file_metadata() + super().save(*args, **kwargs) + def update_from_asset_data( self, asset_data: dict[str, Any], @@ -192,6 +193,18 @@ def update_from_asset_data( int(asset_data["width"]), ) + def asset_file_has_changed(self, asset_data: dict[str, Any]) -> bool: + return ( + super().asset_file_has_changed(asset_data) + or (self.original_height or 0) != int(asset_data["height"]) + or (self.original_width or 0) != int(asset_data["width"]) + ) + + def update_file(self, asset_data: dict[str, Any]) -> None: + self.original_width = int(asset_data["width"]) + self.original_height = int(asset_data["height"]) + return super().update_file(asset_data) + def set_focal_area_from_focus_point( self, x: int, y: int, original_height: int, original_width: int ) -> None: @@ -221,22 +234,22 @@ def set_focal_area_from_focus_point( # For the height, span outwards until we hit the top or bottom bounds self.focal_point_height = min(y, self.height - y) * 2 - def download_asset_file(self, asset_data: dict[str, Any]) -> utils.DownloadedFile: - # Use the thumbnail specified in settings as the source image for - # Wagtail. NOTE: This should still be a high quality image. We just - # don't need a print quality original to generate renditions from. - key = getattr(settings, "BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME", "webimage") + @staticmethod + def extract_file_source(asset_data: dict[str, Any]) -> str: + # For images, we store and use the source derivative filename, + # because the 'original' isn't always present + asset_id = asset_data["id"] + key = getattr(settings, "BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME", "WagtailSource") thumbnails = asset_data["thumbnails"] try: - source_url = thumbnails[key] + return thumbnails[key] except KeyError as e: - raise ImproperlyConfigured( - f"The '{key}' derivative is missing from 'thumbnails' for image asset '{self.bynder_id}'. " + raise BynderAssetDataError( + f"The '{key}' derivative is missing from 'thumbnails' for image asset '{asset_id}'. " "You might need to update the 'BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME' setting value to reflect " - "derivative names used in your Bynder instance. The available derivative for this asset " - f"are: {thumbnails.keys()}" + "derivative names used in your Bynder instance. The available derivatives for this asset " + f"are: {list(thumbnails.keys())}" ) from e - return utils.download_image(source_url) class BynderSyncedDocument(BynderAssetWithFileMixin, AbstractDocument): @@ -248,11 +261,25 @@ class BynderSyncedDocument(BynderAssetWithFileMixin, AbstractDocument): AbstractDocument.search_fields + BynderAssetWithFileMixin.extra_search_fields ) - class Meta: + class Meta(AbstractDocument.Meta): abstract = True - def download_asset_file(self, asset_data: dict[str, Any]) -> utils.DownloadedFile: - return utils.download_document(asset_data["original"]) + def save(self, *args, **kwargs): + if getattr(self, "_file_changed", False): + self._set_document_file_metadata() + super().save(*args, **kwargs) + + @staticmethod + def extract_file_source(asset_data: dict[str, Any]) -> str: + asset_id = asset_data["id"] + try: + return asset_data["original"] + except KeyError as e: + raise BynderAssetDataError( + f"'original' is missing from the API representation for document asset '{asset_id}'. " + "This is likely because the asset is marked as 'private' in Bynder. Wagtail needs the " + "'original' asset URL in order to download and save its own copy." + ) from e class BynderSyncedVideo( @@ -265,6 +292,12 @@ class BynderSyncedVideo( updated_at = models.DateTimeField( verbose_name=_("last updated at"), auto_now=True, db_index=True ) + original_width = models.IntegerField( + verbose_name=_("original width"), null=True, editable=False + ) + original_height = models.IntegerField( + verbose_name=_("original height"), null=True, editable=False + ) primary_source_url = models.URLField( verbose_name=_("primary source URL"), help_text=_( @@ -308,11 +341,20 @@ class BynderSyncedVideo( FieldPanel("is_public", read_only=True), ], ), - FieldPanel("metadata", read_only=True), + MultiFieldPanel( + heading=_("Further information"), + children=[ + FieldPanel("original_filesize", read_only=True), + FieldPanel("original_height", read_only=True), + FieldPanel("original_width", read_only=True), + ], + ), ] class Meta: abstract = True + verbose_name = _("video") + verbose_name_plural = _("videos") def __str__(self): return self.title @@ -333,13 +375,11 @@ def update_from_asset_data( self, asset_data: dict[str, Any], ) -> None: - super().update_from_asset_data(asset_data) - primary_derivative_name = getattr( - settings, "BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME", "Web-Primary" + settings, "BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME", "WebPrimary" ) fallback_derivative_name = getattr( - settings, "BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME", "Web-Fallback" + settings, "BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME", "WebFallback" ) poster_image_derivative_name = getattr( settings, "BYNDER_VIDEO_POSTER_IMAGE_DERIVATIVE_NAME", "webimage" @@ -348,14 +388,16 @@ def update_from_asset_data( derivatives = {v.split("/")[-2]: v for v in asset_data["videoPreviewURLs"]} try: self.primary_source_url = derivatives[primary_derivative_name] - except KeyError: - raise ImproperlyConfigured( + except KeyError as e: + raise BynderAssetDataError( "'videoPreviewURLs' does not contain a URL matching the derivative name " f"'{primary_derivative_name}'. You might need to update the " "'BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME' setting value to reflect the derivative " "names set by Bynder for your instance. The available derivatives for this " - f"asset are: {derivatives}" - ) from None + f"asset are: {list(derivatives.keys())}" + ) from e + else: + self.source_filename = utils.filename_from_url(self.primary_source_url) self.fallback_source_url = derivatives.get(fallback_derivative_name) @@ -363,10 +405,16 @@ def update_from_asset_data( try: self.poster_image_url = thumbnails[poster_image_derivative_name] except KeyError as e: - raise ImproperlyConfigured( + raise BynderAssetDataError( f"The '{poster_image_derivative_name}' derivative is missing from 'thumbnails' for " f"video asset '{self.bynder_id}'. You might need to update the " "'BYNDER_VIDEO_POSTER_IMAGE_DERIVATIVE_NAME' setting value to reflect the " "derivative names set by Bynder for you instance. The available derivative names " - f"for this asset are: {thumbnails.keys()}" + f"for this asset are: {list(thumbnails.keys())}" ) from e + + self.original_filesize = int(asset_data["fileSize"]) + self.original_width = int(asset_data["width"]) + self.original_height = int(asset_data["height"]) + + super().update_from_asset_data(asset_data) diff --git a/src/wagtail_bynder/utils.py b/src/wagtail_bynder/utils.py index c3a9539..e52d885 100644 --- a/src/wagtail_bynder/utils.py +++ b/src/wagtail_bynder/utils.py @@ -17,9 +17,9 @@ class DownloadedFile(BytesIO): name: str - content_type: str - charset: str size: int + content_type: str | None + charset: str | None def download_file(url: str) -> DownloadedFile: @@ -31,9 +31,9 @@ def download_file(url: str) -> DownloadedFile: return f -def download_document(url: str) -> InMemoryUploadedFile: +def download_asset(url: str) -> InMemoryUploadedFile: f = download_file(url) - uploadedfile = InMemoryUploadedFile( + return InMemoryUploadedFile( f, name=f.name, field_name="file", @@ -41,20 +41,10 @@ def download_document(url: str) -> InMemoryUploadedFile: charset=f.charset, content_type=f.content_type, ) - return uploadedfile -def download_image(url: str) -> InMemoryUploadedFile: - f = download_file(url) - uploadedfile = InMemoryUploadedFile( - f, - name=f.name, - field_name="file", - size=f.size, - charset=f.charset, - content_type=f.content_type, - ) - return uploadedfile +def filename_from_url(url: str) -> str: + return os.path.basename(url) def get_bynder_client() -> BynderClient: diff --git a/tests/test_models.py b/tests/test_models.py new file mode 100644 index 0000000..4fd712c --- /dev/null +++ b/tests/test_models.py @@ -0,0 +1,342 @@ +from unittest import mock + +from django.conf import settings +from django.test import SimpleTestCase +from wagtail.documents import get_document_model +from wagtail.images import get_image_model + +from wagtail_bynder import get_video_model +from wagtail_bynder.exceptions import BynderAssetDataError +from wagtail_bynder.utils import filename_from_url + +from .utils import get_test_asset_data + + +class BynderSyncedDocumentTests(SimpleTestCase): + def setUp(self): + model_class = get_document_model() + self.asset_data = get_test_asset_data( + name="My Groovy Document", + type="document", + id="7df5c640-af36-4502-84ca-c1e0005dd229", + ) + self.obj = model_class( + title=self.asset_data["name"], + bynder_id=self.asset_data["id"], + source_filename=filename_from_url(self.asset_data["original"]), + original_filesize=self.asset_data["fileSize"], + collection_id=1, + ) + super().setUp() + + def test_extract_file_source(self): + # When 'original' is present, that should be used + self.assertIs( + self.obj.extract_file_source(self.asset_data), self.asset_data["original"] + ) + # When it is not present, a special KeyError should be raised + asset_id = self.asset_data["id"] + del self.asset_data["original"] + with self.assertRaisesMessage( + BynderAssetDataError, + ( + f"'original' is missing from the API representation for document asset '{asset_id}'. " + "This is likely because the asset is marked as 'private' in Bynder. Wagtail needs the " + "'original' asset URL in order to download and save its own copy." + ), + ): + self.obj.extract_file_source(self.asset_data) + + def test_asset_file_has_changed(self): + # `self.obj` is initialized with values from the API representation, + # so no changes should be reported by default + self.assertFalse(self.obj.asset_file_has_changed(self.asset_data)) + # That should change if a change in 'fileSize' is detected + data = self.asset_data.copy() + data["fileSize"] = 999 + self.assertTrue(self.obj.asset_file_has_changed(data)) + # OR if a change in filename is detected + data = self.asset_data.copy() + data["original"] = data["original"][:-5] + "foobar" + data["original"][-5:] + self.assertTrue(self.obj.asset_file_has_changed(data)) + + def test_update_file(self): + self.obj.source_filename = None + self.obj.original_filesize = None + self.assertFalse(hasattr(self.obj, "_file_changed")) + + with mock.patch( + "wagtail_bynder.models.utils.download_asset", return_value=None + ): + self.obj.update_file(self.asset_data) + + self.assertTrue(self.obj._file_changed) + self.assertEqual( + self.obj.source_filename, filename_from_url(self.asset_data["original"]) + ) + self.assertEqual(self.obj.original_filesize, self.asset_data["fileSize"]) + + def test_update_from_asset_data(self): + self.obj.title = None + self.obj.copyright = None + self.obj.description = None + self.obj.bynder_last_modified = None + self.obj.is_archived = None + self.obj.is_limited_use = None + self.obj.is_public = None + + with ( + mock.patch( + "wagtail_bynder.models.utils.get_default_collection", return_value=None + ), + mock.patch.object(self.obj, "update_file"), + ): + self.obj.update_from_asset_data(self.asset_data) + self.assertEqual(self.obj.title, self.asset_data["name"]) + self.assertEqual(self.obj.copyright, self.asset_data["copyright"]) + self.assertEqual(self.obj.description, self.asset_data["description"]) + self.assertEqual(self.obj.bynder_last_modified, self.asset_data["dateModified"]) + self.assertEqual(self.obj.is_archived, self.asset_data["archive"] == 1) + self.assertEqual(self.obj.is_limited_use, self.asset_data["limited"] == 1) + self.assertEqual(self.obj.is_public, self.asset_data["isPublic"] == 1) + + +class BynderSyncedImageTests(SimpleTestCase): + def setUp(self): + model_class = get_image_model() + self.asset_data = get_test_asset_data( + name="My Groovy Image", + type="image", + id="5e1207a7-0a11-40bd-95c8-907b8233520a", + ) + self.obj = model_class( + title=self.asset_data["name"], + bynder_id=self.asset_data["id"], + height=50, + width=50, + source_filename=filename_from_url( + self.asset_data["thumbnails"]["WagtailSource"] + ), + original_filesize=self.asset_data["fileSize"], + original_height=self.asset_data["height"], + original_width=self.asset_data["width"], + collection_id=1, + ) + super().setUp() + + def test_extract_file_source(self): + # For images, this method should extract the URL for the derivative + # named by the BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME setting + derivative_name = settings.BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME + self.assertIs( + self.obj.extract_file_source(self.asset_data), + self.asset_data["thumbnails"][derivative_name], + ) + # When that derivative is not present, a special KeyError should be raised + asset_id = self.asset_data["id"] + self.asset_data["thumbnails"] = {} + with self.assertRaisesMessage( + BynderAssetDataError, + ( + f"The '{derivative_name}' derivative is missing from 'thumbnails' for image asset '{asset_id}'. " + "You might need to update the 'BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME' setting value to reflect " + "derivative names used in your Bynder instance. The available derivatives for this asset " + "are: []" + ), + ): + self.obj.extract_file_source(self.asset_data) + + def test_asset_file_has_changed(self): + # `self.obj` is initialized with values from the API representation, + # so no changes should be reported by default + self.assertFalse(self.obj.asset_file_has_changed(self.asset_data)) + # That should change if a change in 'fileSize' is detected + data = self.asset_data.copy() + data["fileSize"] = 999 + self.assertTrue(self.obj.asset_file_has_changed(data)) + # OR if a change in filename is detected + data = self.asset_data.copy() + thumb = data["thumbnails"]["WagtailSource"] + data["thumbnails"]["WagtailSource"] = thumb[:-5] + "foobar" + thumb[-5:] + self.assertTrue(self.obj.asset_file_has_changed(data)) + # OR a change in width + data = self.asset_data.copy() + data["width"] += 1 + self.assertTrue(self.obj.asset_file_has_changed(data)) + # OR a change in height + data = self.asset_data.copy() + data["height"] += 1 + self.assertTrue(self.obj.asset_file_has_changed(data)) + + def test_update_file(self): + self.obj.source_filename = None + self.obj.original_filesize = None + self.obj.original_height = None + self.obj.original_width = None + self.assertFalse(hasattr(self.obj, "_file_changed")) + + with mock.patch( + "wagtail_bynder.models.utils.download_asset", return_value=None + ): + self.obj.update_file(self.asset_data) + + self.assertTrue(self.obj._file_changed) + self.assertEqual( + self.obj.source_filename, + filename_from_url(self.asset_data["thumbnails"]["WagtailSource"]), + ) + self.assertEqual(self.obj.original_filesize, self.asset_data["fileSize"]) + self.assertEqual(self.obj.original_height, self.asset_data["height"]) + self.assertEqual(self.obj.original_width, self.asset_data["width"]) + + def test_update_from_asset_data(self): + self.obj.title = None + self.obj.copyright = None + self.obj.description = None + self.obj.bynder_last_modified = None + self.obj.is_archived = None + self.obj.is_limited_use = None + self.obj.is_public = None + + with ( + mock.patch( + "wagtail_bynder.models.utils.get_default_collection", return_value=None + ), + mock.patch.object(self.obj, "update_file"), + ): + self.obj.update_from_asset_data(self.asset_data) + + self.assertEqual(self.obj.title, self.asset_data["name"]) + self.assertEqual(self.obj.copyright, self.asset_data["copyright"]) + self.assertEqual(self.obj.description, self.asset_data["description"]) + self.assertEqual(self.obj.bynder_last_modified, self.asset_data["dateModified"]) + self.assertEqual(self.obj.is_archived, self.asset_data["archive"] == 1) + self.assertEqual(self.obj.is_limited_use, self.asset_data["limited"] == 1) + self.assertEqual(self.obj.is_public, self.asset_data["isPublic"] == 1) + self.assertEqual(self.obj.focal_point_x, 13) + self.assertEqual(self.obj.focal_point_y, 13) + self.assertEqual(self.obj.focal_point_height, 26) + self.assertEqual(self.obj.focal_point_width, 26) + + +class BynderSyncedVideoTests(SimpleTestCase): + def setUp(self): + model_class = get_video_model() + self.asset_data = get_test_asset_data( + name="My Groovy Video", + type="video", + id="5bae2917-0ea4-45dc-9eeb-02a65d9ac0a2", + ) + self.obj = model_class( + title=self.asset_data["name"], + bynder_id=self.asset_data["id"], + source_filename=filename_from_url(self.asset_data["videoPreviewURLs"][0]), + original_height=self.asset_data["height"], + original_width=self.asset_data["width"], + collection_id=1, + ) + super().setUp() + + def test_primary_source_mimetype(self): + # When 'primary_source_url' is set, the property should return a value appropriate to the URL + self.obj.primary_source_url = self.asset_data["videoPreviewURLs"][0] + self.assertEqual(self.obj.primary_source_mimetype, "video/webm") + # The cached_property decorator is used, which means the value + # is cached per instance, even if 'primary_source_url' changes + self.obj.primary_source_url = self.asset_data["videoPreviewURLs"][1] + self.assertEqual(self.obj.primary_source_mimetype, "video/webm") + # If we clear the cache and unset 'primary_source_url', we should get an empty string back + self.obj.__dict__.pop("primary_source_mimetype") + self.obj.primary_source_url = "" + self.assertEqual(self.obj.primary_source_mimetype, "") + + def test_fallback_source_mimetype(self): + # When 'fallback_source_url' is set, the property should return a value appropriate to the URL + self.obj.fallback_source_url = self.asset_data["videoPreviewURLs"][1] + self.assertEqual(self.obj.fallback_source_mimetype, "video/mp4") + # The cached_property decorator is used, which means the value + # is cached per instance, even if 'primary_source_url' changes + self.obj.fallback_source_url = self.asset_data["videoPreviewURLs"][0] + self.assertEqual(self.obj.fallback_source_mimetype, "video/mp4") + # If we clear the cache and unset 'primary_source_url', we should get an empty string back + self.obj.__dict__.pop("fallback_source_mimetype") + self.obj.fallback_source_url = "" + self.assertEqual(self.obj.fallback_source_mimetype, "") + + def test_update_from_asset_data(self): + self.obj.title = None + self.obj.copyright = None + self.obj.description = None + self.obj.bynder_last_modified = None + self.obj.is_archived = None + self.obj.is_limited_use = None + self.obj.is_public = None + self.obj.source_filename = None + self.obj.original_filesize = None + self.obj.original_height = None + self.obj.original_width = None + + with mock.patch( + "wagtail_bynder.models.utils.get_default_collection", return_value=None + ): + self.obj.update_from_asset_data(self.asset_data) + + self.assertEqual(self.obj.title, self.asset_data["name"]) + self.assertEqual(self.obj.copyright, self.asset_data["copyright"]) + self.assertEqual(self.obj.description, self.asset_data["description"]) + self.assertEqual(self.obj.bynder_last_modified, self.asset_data["dateModified"]) + self.assertEqual(self.obj.is_archived, self.asset_data["archive"] == 1) + self.assertEqual(self.obj.is_limited_use, self.asset_data["limited"] == 1) + self.assertEqual(self.obj.is_public, self.asset_data["isPublic"] == 1) + self.assertEqual( + self.obj.primary_source_url, self.asset_data["videoPreviewURLs"][0] + ) + self.assertEqual( + self.obj.fallback_source_url, self.asset_data["videoPreviewURLs"][1] + ) + self.assertEqual( + self.obj.poster_image_url, self.asset_data["thumbnails"]["webimage"] + ) + self.assertEqual( + self.obj.source_filename, + filename_from_url(self.asset_data["videoPreviewURLs"][0]), + ) + self.assertEqual(self.obj.original_filesize, self.asset_data["fileSize"]) + self.assertEqual(self.obj.original_height, self.asset_data["height"]) + self.assertEqual(self.obj.original_width, self.asset_data["width"]) + + def test_missing_primary_source(self): + del self.asset_data["videoPreviewURLs"][0] + with self.assertRaisesMessage( + BynderAssetDataError, + ( + "'videoPreviewURLs' does not contain a URL matching the derivative name " + "'WebPrimary'. You might need to update the 'BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME' " + "setting value to reflect the derivative names set by Bynder for your instance. " + "The available derivatives for this asset are: ['WebFallback']" + ), + ): + self.obj.update_from_asset_data(self.asset_data) + + def test_missing_fallback_source(self): + del self.asset_data["videoPreviewURLs"][1] + with mock.patch( + "wagtail_bynder.models.utils.get_default_collection", return_value=None + ): + self.obj.update_from_asset_data(self.asset_data) + self.assertIsNone(self.obj.fallback_source_url) + + def test_missing_poster_image(self): + asset_id = self.asset_data["id"] + del self.asset_data["thumbnails"]["webimage"] + with self.assertRaisesMessage( + BynderAssetDataError, + ( + "The 'webimage' derivative is missing from 'thumbnails' " # noqa: S608 + f"for video asset '{asset_id}'. You might need to update the " + "'BYNDER_VIDEO_POSTER_IMAGE_DERIVATIVE_NAME' setting value to " + "reflect the derivative names set by Bynder for you instance. " + "The available derivative names for this asset are: ['mini', 'thul']" + ), + ): + self.obj.update_from_asset_data(self.asset_data) diff --git a/tests/testapp/migrations/0001_initial.py b/tests/testapp/migrations/0001_initial.py index 16d2a33..d90abbd 100644 --- a/tests/testapp/migrations/0001_initial.py +++ b/tests/testapp/migrations/0001_initial.py @@ -1,6 +1,5 @@ -# Generated by Django 5.0.1 on 2024-01-19 17:14 +# Generated by Django 5.0.4 on 2024-04-10 15:44 -import django.core.serializers.json import django.db.models.deletion import taggit.managers import wagtail.images.models @@ -16,7 +15,7 @@ class Migration(migrations.Migration): dependencies = [ ("taggit", "0005_auto_20220424_2025"), - ("wagtailcore", "0089_log_entry_data_json_null_to_object"), + ("wagtailcore", "0091_remove_revision_submitted_for_moderation"), migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] @@ -47,25 +46,33 @@ class Migration(migrations.Migration): ( "bynder_id", models.CharField( + blank=True, db_index=True, editable=False, max_length=36, - blank=True, + null=True, unique=True, verbose_name="Bynder asset ID", ), ), ( - "bynder_id_hash", - models.CharField(editable=False, max_length=100, blank=True), + "bynder_last_modified", + models.DateTimeField(db_index=True, editable=False, null=True), ), ( - "bynder_original_filename", - models.CharField(editable=False, max_length=255, blank=True), + "source_filename", + models.CharField( + blank=True, + editable=False, + max_length=255, + verbose_name="source filename", + ), ), ( - "bynder_last_modified", - models.DateTimeField(db_index=True, editable=False, null=True), + "original_filesize", + models.IntegerField( + editable=False, null=True, verbose_name="original filesize" + ), ), ( "description", @@ -93,15 +100,6 @@ class Migration(migrations.Migration): default=False, verbose_name="asset is marked as public" ), ), - ( - "metadata", - models.JSONField( - blank=True, - default=dict, - encoder=django.core.serializers.json.DjangoJSONEncoder, - verbose_name="additional metadata", - ), - ), ( "collection", models.ForeignKey( @@ -135,6 +133,8 @@ class Migration(migrations.Migration): ), ], options={ + "verbose_name": "document", + "verbose_name_plural": "documents", "abstract": False, }, bases=(wagtail.search.index.Indexed, models.Model), @@ -189,25 +189,33 @@ class Migration(migrations.Migration): ( "bynder_id", models.CharField( + blank=True, db_index=True, editable=False, max_length=36, - blank=True, + null=True, unique=True, verbose_name="Bynder asset ID", ), ), ( - "bynder_id_hash", - models.CharField(editable=False, max_length=100, blank=True), + "bynder_last_modified", + models.DateTimeField(db_index=True, editable=False, null=True), ), ( - "bynder_original_filename", - models.CharField(editable=False, max_length=255, blank=True), + "source_filename", + models.CharField( + blank=True, + editable=False, + max_length=255, + verbose_name="source filename", + ), ), ( - "bynder_last_modified", - models.DateTimeField(db_index=True, editable=False, null=True), + "original_filesize", + models.IntegerField( + editable=False, null=True, verbose_name="original filesize" + ), ), ( "description", @@ -236,12 +244,15 @@ class Migration(migrations.Migration): ), ), ( - "metadata", - models.JSONField( - blank=True, - default=dict, - encoder=django.core.serializers.json.DjangoJSONEncoder, - verbose_name="additional metadata", + "original_width", + models.IntegerField( + editable=False, null=True, verbose_name="original width" + ), + ), + ( + "original_height", + models.IntegerField( + editable=False, null=True, verbose_name="original height" ), ), ( @@ -300,25 +311,33 @@ class Migration(migrations.Migration): ( "bynder_id", models.CharField( + blank=True, db_index=True, editable=False, max_length=36, - blank=True, + null=True, unique=True, verbose_name="Bynder asset ID", ), ), ( - "bynder_id_hash", - models.CharField(editable=False, max_length=100, blank=True), + "bynder_last_modified", + models.DateTimeField(db_index=True, editable=False, null=True), ), ( - "bynder_original_filename", - models.CharField(editable=False, max_length=255, blank=True), + "source_filename", + models.CharField( + blank=True, + editable=False, + max_length=255, + verbose_name="source filename", + ), ), ( - "bynder_last_modified", - models.DateTimeField(db_index=True, editable=False, null=True), + "original_filesize", + models.IntegerField( + editable=False, null=True, verbose_name="original filesize" + ), ), ( "description", @@ -346,15 +365,6 @@ class Migration(migrations.Migration): default=False, verbose_name="asset is marked as public" ), ), - ( - "metadata", - models.JSONField( - blank=True, - default=dict, - encoder=django.core.serializers.json.DjangoJSONEncoder, - verbose_name="additional metadata", - ), - ), ("title", models.CharField(max_length=255, verbose_name="title")), ( "created_at", @@ -368,6 +378,18 @@ class Migration(migrations.Migration): auto_now=True, db_index=True, verbose_name="last updated at" ), ), + ( + "original_width", + models.IntegerField( + editable=False, null=True, verbose_name="original width" + ), + ), + ( + "original_height", + models.IntegerField( + editable=False, null=True, verbose_name="original height" + ), + ), ( "primary_source_url", models.URLField( @@ -396,6 +418,8 @@ class Migration(migrations.Migration): ), ], options={ + "verbose_name": "video", + "verbose_name_plural": "videos", "abstract": False, }, bases=(wagtail.search.index.Indexed, models.Model), diff --git a/tests/testapp/settings.py b/tests/testapp/settings.py index 7ecd9c0..19b9254 100644 --- a/tests/testapp/settings.py +++ b/tests/testapp/settings.py @@ -162,8 +162,10 @@ BYNDER_DOMAIN = env.get("BYNDER_DOMAIN", "test-org.bynder.com") BYNDER_API_TOKEN = env.get("BYNDER_API_TOKEN", None) -BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME = env.get("BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME", None) BYNDER_COMPACTVIEW_API_TOKEN = env.get("BYNDER_COMPACTVIEW_API_TOKEN", None) +BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME = env.get( + "BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME", "WagtailSource" +) BYNDER_DISABLE_WAGTAIL_EDITING_FOR_ASSETS = ( env.get("BYNDER_DISABLE_WAGTAIL_EDITING_FOR_ASSETS", "false").lower() == "true" ) @@ -176,8 +178,8 @@ # Video BYNDER_VIDEO_MODEL = "testapp.Video" BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME = env.get( - "BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME", "Derivative-1" + "BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME", "WebPrimary" ) BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME = env.get( - "BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME", "Derivative-2" + "BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME", "WebFallback" ) diff --git a/tests/utils.py b/tests/utils.py index 57185ba..6989544 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,5 +1,7 @@ from datetime import datetime +from django.utils.text import slugify + TEST_ASSET_ID = "1A7BA172-97B9-44A4-8C0AA41D9E8AE6A2" @@ -7,18 +9,46 @@ def get_test_asset_data( id: str = TEST_ASSET_ID, name: str = "Test asset", + type: str = "image", + description: str = "", copyright: str = "", - alt_text: str = "", date_modified: datetime | None = None, ): from django.conf import settings + id_hash = "3477c04a50a14650" + if date_modified: date_modified_str = date_modified.isoformat() else: date_modified_str = "2023-10-10T09:52:05Z" - return { + name_slugified = slugify(name) + + if type == "document": + extension = "pdf" + filename = name_slugified + ".pdf" + elif type == "video": + extension = "mpg" + filename = name_slugified + ".mpg" + else: + extension = "tif" + filename = name_slugified + ".tif" + + thumb_base = f"https://{settings.BYNDER_DOMAIN}/m/{id_hash}" + + thumbnails = { + "mini": f"{thumb_base}/mini-{name_slugified}.png", + "thul": f"{thumb_base}/thul-{name_slugified}.png", + "webimage": f"{thumb_base}/webimage-{name_slugified}.png", + } + if type == "image": + derivative_name = settings.BYNDER_IMAGE_SOURCE_THUMBNAIL_NAME + thumbnails[ + derivative_name + ] = f"{thumb_base}/{derivative_name}-{name_slugified}.jpg" + + data = { "activeOriginalFocusPoint": {"x": 541, "y": 550}, "archive": 0, "brandId": "04215F17-57D6-48FB-9D64DA196B6B1E33", @@ -26,32 +56,33 @@ def get_test_asset_data( "dateCreated": "2023-09-26T12:42:21Z", "dateModified": date_modified_str, "datePublished": "2005-03-11T02:08:12Z", - "description": ( - "Medieval iron battleaxe. This battleaxe has a triangular blade with a reinforced edge and eared socket. " - "The wooden handle has been added recently. This is part of a group of battleaxes and spears that were " - "found during building works at the north end of London Bridge in the 1920s. They may have been lost in " - "battle or thrown into the river by the victors in celebration." - ), - "property_Alt_text": alt_text, + "description": description, "ecsArchiveFiles": [], - "extension": ["tif"], + "extension": [extension], "fileSize": 18096064, "height": 2008, "id": id or TEST_ASSET_ID, "idHash": "3477c04a50a14650", - "isPublic": 0, + "isPublic": 1, "limited": 0, "name": name, "orientation": "landscape", - "original": f"https://{settings.BYNDER_DOMAIN}/m/3477c04a50a14650/original/Medieval-iron-battleaxe-11th-century.tif", + "original": f"{thumb_base}/original/{filename}", "property_Accession_Number": "A23339", - "thumbnails": { - "mini": f"https://{settings.BYNDER_DOMAIN}/m/3477c04a50a14650/mini-Medieval-iron-battleaxe-11th-century.png", - "thul": f"https://{settings.BYNDER_DOMAIN}/m/3477c04a50a14650/thul-Medieval-iron-battleaxe-11th-century.png", - "webimage": f"https://{settings.BYNDER_DOMAIN}/m/3477c04a50a14650/webimage-Medieval-iron-battleaxe-11th-century.png", - }, - "type": "image", + "type": type, "userCreated": "Mr Test", "watermarked": 0, "width": 3000, + "thumbnails": thumbnails, } + + if type == "video": + url_base = f"https://{settings.BYNDER_DOMAIN}/asset/{data['id'].lower()}" + primary_derivative = settings.BYNDER_VIDEO_PRIMARY_DERIVATIVE_NAME + fallback_derivative = settings.BYNDER_VIDEO_FALLBACK_DERIVATIVE_NAME + data["videoPreviewURLs"] = [ + f"{url_base}/{primary_derivative}/{primary_derivative}-{name_slugified}.webm", + f"{url_base}/{fallback_derivative}/{fallback_derivative}-{name_slugified}.mp4", + ] + + return data