Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#36269 closed Cleanup/optimization (fixed)

Document overriding STORAGES for tests when using callable storage in a FileField

Reported by: emillumine Owned by: Ahmed Nassar
Component: File uploads/storage Version: 5.1
Severity: Normal Keywords: override_settings, storages
Cc: emillumine 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

It seems that the default storage and other user defined storages dont have the same behaviour in tests.
When overriding the settings.STORAGES in tests we can easily replace the default storage whereas it doesn't work for another user defined storage.
I tried to reproduce my problem with a minimal code below.

It may seems successful at first because the first assertion assert storages["other_storage"]._location == "other_location" is green. But the last one assert test.other_file.storage._location == "other_location" is not. Its value is the one defined in settings.STORAGES.

I would expect a similar behaviour in both cases.

# settings.py
STORAGES = {
    "default": {
        "BACKEND": "django.core.files.storage.FileSystemStorage",
    },
    "staticfiles": {
        "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
    },
    "other_storage": {
        "BACKEND": "django.core.files.storage.FileSystemStorage",
    }
}
# test.py

from django.test import override_settings
from django.core.files.storage import default_storage
from django.core.files.storage import storages
from django.core.files.storage import InMemoryStorage
from django.db import models
from django.core.files.base import ContentFile


def select_other_storage():
    return storages["other_storage"]

def select_default_storage():
    return default_storage


class TestModel(models.Model):
    other_file = models.FileField(storage=select_other_storage)
    default_file = models.FileField(storage=select_default_storage)

    class Meta:
        app_label = "test"

@override_settings(
    STORAGES={
        "default": {
            "BACKEND": "django.core.files.storage.InMemoryStorage",
            "OPTIONS": {
                "location": "default_location",
                },
            },
        "other_storage": {
            "BACKEND": "django.core.files.storage.InMemoryStorage",
            "OPTIONS": {
                "location": "other_location",
                },
        }
    }
)
def test_override_settings():
    assert default_storage._location == "default_location"
    assert storages["other_storage"]._location == "other_location"

    test = TestModel(
        other_file=ContentFile("test data", name="test.pdf"),
        default_file=ContentFile("test data", name="test.pdf"),
    )
    assert isinstance(test.default_file.storage, InMemoryStorage)
    assert test.default_file.storage._location == "default_location"
    # following assertions fail
    assert isinstance(test.other_file.storage, InMemoryStorage)
    assert test.other_file.storage._location == "other_location"

Change History (12)

comment:1 by emillumine, 6 months ago

Version: 5.25.1

comment:2 by Sarah Boyce, 6 months ago

Summary: Cant override storage used in a FileField other than the default storageDocument overriding STORAGES for tests when using callable storage in a FileField
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

To me, this is expected based off the documentation:

https://docs.djangoproject.com/en/5.1/topics/files/#using-a-callable

Your callable will be evaluated when your models classes are loaded...

However, I think it might be nice to update the docs with an example similar to the default storage in order to make this easier to test.

We would need to check if this works but maybe something like:

from django.core.files.storage import storages
from django.db import models
from django.utils.functional import LazyObject


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


select_storage = MyStorage()

class MyModel(models.Model):
    upload = models.FileField(storage=select_storage)

With a comment around testing and why you might want to do this

comment:3 by Ahmed Nassar, 5 months ago

Owner: set to Ahmed Nassar
Status: newassigned

comment:4 by Ahmed Nassar, 5 months ago

Has patch: set

comment:5 by Ahmed Nassar, 5 months ago

I've submitted the PR addresses this ticket: https://github.com/django/django/pull/19349/

Last edited 5 months ago by Ahmed Nassar (previous) (diff)

comment:6 by Sarah Boyce, 5 months ago

Patch needs improvement: set

comment:7 by Ahmed Nassar, 5 months ago

Patch needs improvement: unset

comment:8 by Ahmed Nassar, 5 months ago

Triage Stage: AcceptedReady for checkin

comment:9 by Sarah Boyce, 5 months ago

(You shouldn't mark you're own ticket as ready for checkin, however, I was now about to do so so it ok. In future, you shouldn't do this)

in reply to:  9 comment:10 by Ahmed Nassar, 5 months ago

Okay. I'll don't make it again.

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

Resolution: fixed
Status: assignedclosed

In 8bca33f6:

Fixed #36269 -- Documented how to test callable storage in FileField.

comment:12 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

In cbdb1be:

[5.2.x] Fixed #36269 -- Documented how to test callable storage in FileField.

Backport of 8bca33f68acc4fc881146c4b9cf4101a8bfab437 from main.

Note: See TracTickets for help on using tickets.
Back to Top