Code

Opened 3 years ago

Last modified 3 years ago

#15817 new Cleanup/optimization

ImageField having [width|height]_field set sytematically compute the image dimensions in ModelForm validation process

Reported by: stan <stanislas.guerra@…> Owned by: nobody
Component: Forms Version: 1.3
Severity: Normal Keywords: ImageField, dimension, height_field, width_field, slow
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This bug is directly related to the ticket #11084 and this discussion on Google Groups.

The fact is when you submit a ModelForm of an existing instance having an ImageField with the image_height / image_width attributes set, the validation process cause the image to be systematically opened to compute the dimension whathever It has changed or not.

This does not happens when these attributes are not set in the field so I guess we do not need the dimensions at this point.

This is a huge overhead in some cases as you can imagine especially with inline_formset when you have several images related to a model ; the simple fact of updating one single field of the parent take ages to open every single image with the PIL.

This take place during the form validation process (.is_valid()), in the _post_clean() method where model validation is performed.

self.instance.clean_fields(exclude=exclude) # forms.models.py (line 325 in post_clean())
setattr(self, f.attname, f.clean(raw_value, self)) # db.models.base.py (line 848 in clean_fields())

Which lead us to the ImageFileDescriptor.set() and a call to update_dimension_fields line 314 :

class ImageFileDescriptor(FileDescriptor):
    """
    Just like the FileDescriptor, but for ImageFields. The only difference is
    assigning the width/height to the width_field/height_field, if appropriate.
    """
    def __set__(self, instance, value):
        previous_file = instance.__dict__.get(self.field.name)
        super(ImageFileDescriptor, self).__set__(instance, value)

        # To prevent recalculating image dimensions when we are instantiating
        # an object from the database (bug #11084), only update dimensions if
        # the field had a value before this assignment.  Since the default
        # value for FileField subclasses is an instance of field.attr_class,
        # previous_file will only be None when we are called from
        # Model.__init__().  The ImageField.update_dimension_fields method
        # hooked up to the post_init signal handles the Model.__init__() cases.
        # Assignment happening outside of Model.__init__() will trigger the
        # update right here.
        if previous_file is not None:
            self.field.update_dimension_fields(instance, force=True)

At this point, I can not see a trivial and elegant patch that does not break anything. A simple workaround (not tested) is to disable those attributes and override the model.save() to update {height|width}_field by myself, not so funny :-)

Attachments (0)

Change History (2)

comment:1 Changed 3 years ago by emulbreh

  • Easy pickings unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.