Skip to content

Commit

Permalink
Clear renditions for image if import changed image's file
Browse files Browse the repository at this point in the history
  • Loading branch information
@jams2 @gasman
jams2 authored and gasman committed Nov 2, 2023
1 parent 9eb8dd9 commit b47ea84
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 3 deletions.
157 changes: 156 additions & 1 deletion tests/tests/test_import.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from django.core.files.images import ImageFile
from django.test import TestCase, override_settings
from wagtail.images.models import Image
from wagtail.models import Collection, Comment, Page
from wagtail.models import Collection, Page

from tests.models import (Advert, Author, Avatar, Category, LongAdvert,
ModelWithManyToMany, PageWithParentalManyToMany,
Expand Down Expand Up @@ -1068,6 +1068,161 @@ def test_replace_image(self, get):
self.assertEqual(image.title, "A lovely wagtail")
self.assertEqual(image.file.read(), b'my test image file contents')

@mock.patch('requests.get')
def test_updated_image_renditions_cleared(self, get):
"""
If we update an Image file, we should clear any renditions that were generated from
the older version of the file.
"""

get.return_value.status_code = 200
get.return_value.content = b'my test image file contents'

with open(os.path.join(FIXTURES_DIR, 'wagtail.jpg'), 'rb') as f:
image = Image.objects.create(
title="Wagtail",
file=ImageFile(f, name='wagtail.jpg')
)

rendition = image.get_rendition("fill-165x165")
self.assertIn(rendition, image.renditions.all())

IDMapping.objects.get_or_create(
uid="f91debc6-1751-11ea-8001-0800278dc04d",
defaults={
'content_type': ContentType.objects.get_for_model(Image),
'local_id': image.id,
}
)

data = """{
"ids_for_import": [
["wagtailimages.image", 53]
],
"mappings": [
["wagtailcore.collection", 3, "f91cb31c-1751-11ea-8000-0800278dc04d"],
["wagtailimages.image", 53, "f91debc6-1751-11ea-8001-0800278dc04d"]
],
"objects": [
{
"model": "wagtailcore.collection",
"pk": 3,
"fields": {
"name": "root"
},
"parent_id": null
},
{
"model": "wagtailimages.image",
"pk": 53,
"fields": {
"collection": 3,
"title": "A lovely wagtail",
"file": {
"download_url": "https://wagtail.io/media/original_images/wagtail.jpg",
"size": 27,
"hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada"
},
"width": 32,
"height": 40,
"created_at": "2019-04-01T07:31:21.251Z",
"uploaded_by_user": null,
"focal_point_x": null,
"focal_point_y": null,
"focal_point_width": null,
"focal_point_height": null,
"file_size": 27,
"file_hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada",
"tags": "[]",
"tagged_items": []
}
}
]
}"""

importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
importer.add_json(data)
importer.run()

get.assert_called()
image = Image.objects.get()
self.assertNotIn(rendition, image.renditions.all())

def test_renditions_not_cleared_if_file_unchanged(self):
"""
If we update an Image, but not the file, we shouldn't clear the renditions
"""

with open(os.path.join(FIXTURES_DIR, 'wagtail.jpg'), 'rb') as f:
image = Image.objects.create(
title="Wagtail",
file=ImageFile(f, name='wagtail.jpg')
)

original_image_hash = image.get_file_hash()
rendition = image.get_rendition("fill-165x165")
self.assertIn(rendition, image.renditions.all())

IDMapping.objects.get_or_create(
uid="f91debc6-1751-11ea-8001-0800278dc04d",
defaults={
'content_type': ContentType.objects.get_for_model(Image),
'local_id': image.id,
}
)

data = f"""{{
"ids_for_import": [
["wagtailimages.image", 53]
],
"mappings": [
["wagtailcore.collection", 3, "f91cb31c-1751-11ea-8000-0800278dc04d"],
["wagtailimages.image", 53, "f91debc6-1751-11ea-8001-0800278dc04d"]
],
"objects": [
{{
"model": "wagtailcore.collection",
"pk": 3,
"fields": {{
"name": "root"
}},
"parent_id": null
}},
{{
"model": "wagtailimages.image",
"pk": 53,
"fields": {{
"collection": 3,
"title": "A lovely wagtail",
"file": {{
"download_url": "https://wagtail.io/media/original_images/wagtail.jpg",
"size": 27,
"hash": "{original_image_hash}"
}},
"width": 32,
"height": 40,
"created_at": "2019-04-01T07:31:21.251Z",
"uploaded_by_user": null,
"focal_point_x": null,
"focal_point_y": null,
"focal_point_width": null,
"focal_point_height": null,
"file_size": 27,
"file_hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada",
"tags": "[]",
"tagged_items": []
}}
}}
]
}}"""

importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
importer.add_json(data)
importer.run()

image = Image.objects.get()
self.assertIn(rendition, image.renditions.all())

def test_import_collection(self):
root_collection = Collection.objects.get()

Expand Down
32 changes: 30 additions & 2 deletions wagtail_transfer/operations.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.utils.functional import cached_property
from modelcluster.models import ClusterableModel, get_all_child_relations
from treebeard.mp_tree import MP_Node
from wagtail.images import get_image_model
from wagtail.models import Page

from .field_adapters import adapter_registry
Expand Down Expand Up @@ -198,7 +199,7 @@ def for_model(cls, model):
def add_json(self, json_data):
"""
Add JSON data to the import plan. The data is a dict consisting of:
'ids_for_import': a list of [source_id, model_classname] pairs for the set of objects
'ids_for_import': a list of [model_classname, source_id] pairs for the set of objects
explicitly requested to be imported. (For example, in a page import, this is the set of
descendant pages of the selected root page.)
'mappings': a list of mappings between UIDs and the object IDs that exist on the source
Expand Down Expand Up @@ -383,7 +384,10 @@ def _handle_task(self, task):
else: # action == 'update'
destination_id = self.context.destination_ids_by_source[(model, source_id)]
obj = specific_model.objects.get(pk=destination_id)
operation = UpdateModel(obj, object_data)
if specific_model is get_image_model():
operation = UpdateImage(obj, object_data)
else:
operation = UpdateModel(obj, object_data)

if issubclass(specific_model, ClusterableModel):
# Process child object relations for this item
Expand Down Expand Up @@ -772,6 +776,30 @@ def run(self, context):
self._populate_many_to_many_fields(context)


class UpdateImage(UpdateModel):
"""
Operation class to handle clearing existing renditions when an image is updated.
If an image's file changes, and we don't clear renditions generated from the old
file, outdated renditions may be shown to users.
"""

def run(self, context):
super().run(context)
self._clear_renditions(context)

def _clear_renditions(self, context):
instance_file_hash = self.instance.get_file_hash()
for imported_file in context.imported_files_by_source_url.values():
if (
imported_file.file.name == self.instance.file.name
and imported_file.hash == instance_file_hash
):
# This will cause Wagtail to purge the renditions cache also.
self.instance.renditions.all().delete()
break


class DeleteModel(Operation):
def __init__(self, instance):
self.instance = instance
Expand Down

0 comments on commit b47ea84

Please sign in to comment.