Skip to content

Commit

Permalink
Fixed #8307 -- Prevented a read operation when saving ImageFields wit…
Browse files Browse the repository at this point in the history
…h width_field and height_field.
  • Loading branch information
john-parton committed Apr 29, 2024
1 parent 85c154d commit b21b364
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 2 deletions.
9 changes: 8 additions & 1 deletion django/db/models/fields/files.py
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions docs/ref/models/fields.txt
Expand Up @@ -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``
----------------

Expand Down
5 changes: 5 additions & 0 deletions docs/releases/5.1.txt
Expand Up @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions tests/model_fields/models.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
10 changes: 10 additions & 0 deletions 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.")
30 changes: 29 additions & 1 deletion tests/model_fields/test_imagefield.py
Expand Up @@ -18,6 +18,7 @@
from .models import (
Person,
PersonDimensionsFirst,
PersonNoReadImage,
PersonTwoImages,
PersonWithHeight,
PersonWithHeightAndWidth,
Expand All @@ -30,7 +31,7 @@ class Person:
pass

PersonWithHeight = PersonWithHeightAndWidth = PersonDimensionsFirst = Person
PersonTwoImages = Person
PersonTwoImages = PersonNoReadImage = Person


class ImageFieldTestMixin(SerializeMixin):
Expand Down Expand Up @@ -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)

0 comments on commit b21b364

Please sign in to comment.