Opened 3 weeks ago

Closed 3 days ago

#36622 closed Bug (fixed)

FileField.__init__ is triggering storage LazyObject resolution at boot time

Reported by: Fabien MICHEL Owned by:
Component: Database layer (models, ORM) Version: 5.2
Severity: Normal Keywords:
Cc: Fabien MICHEL Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Fabien MICHEL)

There is this code in FileField.__init__ :

self.storage = storage or default_storage

The problem is that if we want to have a LazyObject as storage for the field, it is still resolved at boot time, and it seems unecessary to trigger resolution at this moment. This seems not expected as the documentation itself suggest to use LazyObject in case we require a callable to be overridable during tests : https://docs.djangoproject.com/en/5.2/topics/files/#using-a-callable

Using storage or default_storage is calling storage.__bool__ which trigger resolution on default LazyObject.

A solution would be to change this line by :

self.storage = storage if storage is not None else default_storage

A workaround for the case specified in the documentation is to override __bool__ on LazyObject so it always returns True.

class OtherStorage(LazyObject):
    def _setup(self):
        self._wrapped = storages["mystorage"]

    def __bool__(self):
        return True

Change History (9)

comment:1 by Fabien MICHEL, 3 weeks ago

Type: UncategorizedBug

comment:2 by Fabien MICHEL, 3 weeks ago

Description: modified (diff)

comment:3 by Sarah Boyce, 3 weeks ago

Related #36504

comment:4 by Sarah Boyce, 3 weeks ago

Resolution: duplicate
Status: newclosed

Thank you for the ticket! I have tested and I agree with this approach.
Given there were some efforts started in #36504, I have added a comment there and marking this as a duplicate to try and prevent contradictory/overlapping effort

comment:5 by Clifford Gama, 9 days ago

Resolution: duplicate
Status: closednew

Reopening because this issue can be fixed independently of #36504--it neither contradicts nor, strictly speaking, overlap. The reason being that for override_settings to work on custom storages (#36504), we still need to write a custom signal receiver. The solution proposed here only seems to solve #36504 because we're resolving the lazy object for the first and only time after calling override_settings. However, whatever value we get with the first access will persist and cannot be overridden again.

The following modification to the docs example should demonstrate:

def test_storage(self):
    model = MyModel()
    self.assertIsInstance(model.upload.storage, FileSystemStorage)  # Assuming this is in settings
    with override_settings(
        STORAGES={
            "mystorage": {
                "BACKEND": "django.core.files.storage.InMemoryStorage",
            }
        }
     ):
         self.assertIsInstance(model.upload.storage, InMemoryStorage)
Last edited 9 days ago by Clifford Gama (previous) (diff)

comment:6 by Clifford Gama, 9 days ago

Has patch: set

comment:7 by Natalia Bidart, 4 days ago

Triage Stage: UnreviewedAccepted

Thank you Clifford for your detailed explanation.

comment:8 by Natalia Bidart, 3 days ago

Triage Stage: AcceptedReady for checkin

comment:9 by Natalia Bidart, 3 days ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top