From 4b70750c143a337a72c2487f53ba21339f63e20e Mon Sep 17 00:00:00 2001 From: John Parton Date: Wed, 24 Jan 2024 16:22:07 -0600 Subject: [PATCH] Fixed #8307 -- Prevented a read operation when saving ImageFields with width_field and height_field. --- django/db/models/fields/files.py | 9 +++++++- docs/ref/models/fields.txt | 7 +++++++ docs/releases/5.1.txt | 5 +++++ tests/model_fields/models.py | 19 +++++++++++++++++ tests/model_fields/storage.py | 10 +++++++++ tests/model_fields/test_imagefield.py | 30 ++++++++++++++++++++++++++- 6 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 tests/model_fields/storage.py diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py index 95985684ee125..5826573551b0b 100644 --- a/django/db/models/fields/files.py +++ b/django/db/models/fields/files.py @@ -91,7 +91,14 @@ def open(self, mode="rb"): def save(self, name, content, save=True): name = self.field.generate_filename(self.instance, name) self.name = self.storage.save(name, content, max_length=self.field.max_length) - setattr(self.instance, self.field.attname, self.name) + + # Set the file-like content using the descriptors __set__ method. + setattr(self.instance, self.field.attname, content) + + # Update the name in case generate_filename or the storage backend + # changed the name. Avoids re-running the descriptor logic. + self.instance.__dict__[self.field.attname] = self.name + self._committed = True # Save the object because it has changed, unless save is False diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index c6052123fc823..51dd0b7f7e30d 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -1361,6 +1361,13 @@ can change the maximum length using the :attr:`~CharField.max_length` argument. The default form widget for this field is a :class:`~django.forms.ClearableFileInput`. +.. versionchanged:: 5.1 + + In older versions, auto-populating ``height_field`` and ``width_field`` + would always result in re-reading the file from the storage backend. If + your storage backend resizes images, the actual image's width and height + will not match the auto-populated fields. + ``IntegerField`` ---------------- diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt index b461524c70943..014f657abb234 100644 --- a/docs/releases/5.1.txt +++ b/docs/releases/5.1.txt @@ -419,6 +419,11 @@ Miscellaneous * The undocumented ``django.urls.converters.get_converter()`` function is removed. +* :class:`~django.db.models.ImageField` no longer re-reads the file from the + storage backend after ``width_field`` and ``height_field`` are set. If + your storage backend resizes images, the actual image's width and height will + not match the auto-populated fields. + * The minimum supported version of SQLite is increased from 3.27.0 to 3.31.0. .. _deprecated-features-5.1: diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py index e34f3c8947aa5..1a1e775e865ae 100644 --- a/tests/model_fields/models.py +++ b/tests/model_fields/models.py @@ -13,6 +13,8 @@ from django.utils.functional import SimpleLazyObject from django.utils.translation import gettext_lazy as _ +from .storage import NoReadFileSystemStorage + try: from PIL import Image except ImportError: @@ -373,6 +375,23 @@ class PersonTwoImages(models.Model): width_field="headshot_width", ) + class PersonNoReadImage(models.Model): + """ + Model that defines an ImageField with a storage backend that does not + support reading. + + This supports a test to avoid a double-read performance problem. + """ + + mugshot = models.ImageField( + upload_to="tests", + storage=NoReadFileSystemStorage(), + width_field="mugshot_width", + height_field="mugshot_height", + ) + mugshot_width = models.IntegerField() + mugshot_height = models.IntegerField() + class CustomJSONDecoder(json.JSONDecoder): def __init__(self, object_hook=None, *args, **kwargs): diff --git a/tests/model_fields/storage.py b/tests/model_fields/storage.py new file mode 100644 index 0000000000000..1b58b4d46a9ac --- /dev/null +++ b/tests/model_fields/storage.py @@ -0,0 +1,10 @@ +from django.core.files.storage.filesystem import FileSystemStorage + + +class NoReadException(Exception): + pass + + +class NoReadFileSystemStorage(FileSystemStorage): + def open(self, *args, **kwargs): + raise NoReadException("This storage class does not support reading.") diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py index 8c93ed1bdbeb7..78df8b377e19d 100644 --- a/tests/model_fields/test_imagefield.py +++ b/tests/model_fields/test_imagefield.py @@ -16,6 +16,7 @@ if Image: from .models import ( + PersonNoReadImage, Person, PersonDimensionsFirst, PersonTwoImages, @@ -30,7 +31,7 @@ class Person: pass PersonWithHeight = PersonWithHeightAndWidth = PersonDimensionsFirst = Person - PersonTwoImages = Person + PersonTwoImages = PersonNoReadImage = Person class ImageFieldTestMixin(SerializeMixin): @@ -469,3 +470,30 @@ def test_dimensions(self): # Dimensions were recalculated, and hence file should have opened. self.assertIs(p.mugshot.was_opened, True) self.assertIs(p.headshot.was_opened, True) + + +@skipIf(Image is None, "Pillow is required to test ImageField") +class NoReadTests(ImageFieldTestMixin, TestCase): + PersonNoReadImage = PersonNoReadImage + + def test_width_height_correct_name_mangling_correct(self): + instance1 = self.PersonNoReadImage() + + instance1.mugshot.save("mug", self.file1) + + self.assertEqual(instance1.mugshot_width, 4) + self.assertEqual(instance1.mugshot_height, 8) + + instance1.save() + + self.assertEqual(instance1.mugshot_width, 4) + self.assertEqual(instance1.mugshot_height, 8) + + instance2 = self.PersonNoReadImage() + instance2.mugshot.save("mug", self.file1) + instance2.save() + + self.assertNotEqual(instance1.mugshot.name, instance2.mugshot.name) + + self.assertEqual(instance1.mugshot_width, instance2.mugshot_width) + self.assertEqual(instance1.mugshot_height, instance2.mugshot_height)