ImageField: height_field and width_field option sometimes doesn't work

[Lehych]

I had a problem: height_field and width_field always = 0 in some models, after uploading any image file.
Problem was in fields order: if height_field and width_field goes before image field - this thing happends.

I appears in models save method.
/django/db/models/base.py : 378 (actual for revision: 9962)

values = [(f, None, f.get_db_prep_save(raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]

here all fields values are calculated (get_db_prep_save) and copy to values. So, if in models description ImageField goes before height and width fields, everything is fine. But if ImageField goes after this fields, all pic size calculation still in fields but not in values list.

[Travis Cline]

The attached test patch would expose this problem, it doesn't. Marking worksforme.

[Karen Tracey]

I haven't looked in detail but somehow that existing test with the order reversed doesn't trigger this bug. I, however, can recreate the described problem manually using admin, so I do believe there is a problem here.

[Karen Tracey]

Also, this looks to be another r9766 side-effect. While the line of code identified in the description hasn't changed, r9766 effectively moved the setting of these fields from early on (when form data was saved) to much later in the processing. Using r9765 I cannot recreate the error with admin even if the width and height fields are declared before the image field itself.

[Karen Tracey]

The reason the patch to the file_storage tests doesn't trigger the error is because those tests call save() on the model image field directly. The problem introduced by r9766 is that the under-the-covers save() for the file/image fields got moved to much later in the processing, so you need a test that doesn't call the field save() directly but rather just attempts to call save() on the model (or form). That's when it becomes apparent that the save() on the image/file field is now happening too late in some cases. I'll attach a patch to the model_forms test that do trigger the error.

[anonymous]

The attach patch does fix it (test suite runs through fine). Don't ask me if that's the best way to do it :þ

[Armin Ronacher]

The actual fix for this is in ​http://github.com/mitsuhiko/django/tree/ticket-10404

It fixes it by using a descriptor in the image field, rather than pre saving multiple times.

[Jacob]

(In [10737]) Fixed #10404: ImageField height_field and width_field options no longer depend on putting the image field after the height/width fields as they did after r9766.

This bug actually exposed a related handful of inconsistancies in the underlying file handling and wraping, so a few related changes are in here as well:

• Dimensions are also now calculated the moment the image is assigned to the field instead of upon save.
• The base File object now when possible delegates its closed attribute down to the os-level file it wrapps.
• In-memory files' close() now is a no-op. Without this certain APIs that should be able to handle in-memory files were failing.
• Accessing FieldFile.closed used to open the file. That's silly, and it doesn't any more.
• Some over-eager error handling was squishing some errors that would normally be raised. One unit test was incorrectly depending on this behavior, so the test was removed.

Thanks to Armin Ronacher for much of this work.

[Jacob]

Milestone 1.1 deleted