Opened 9 years ago
Closed 9 years ago
#26038 closed Bug (fixed)
FileSystemStorage size() method does not use current MEDIA_ROOT setting value
Reported by: | Dave Voutila | Owned by: | nobody |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | Keywords: | testing fieldfile |
Cc: | 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
While writing some functional tests and using @override_settings
to point to a different MEDIA_ROOT
location on my filesystem containing test media files, getting aFieldFile
's size
property resulted in OSError
exceptions because of an attempt to read the file in the app's default (i.e. not overridden) MEDIA_ROOT
location.
For instance, if I decorate my test class with like:
@override_settings( MEDIA_ROOT = os.path.join(settings.INSTALL_ROOT, 'cl/assets/media/test/') ) class FeedsFunctionalTest(StaticLiveServerTestCase): ...
My test fails during setUp()
due to logic in the test class that attempts to read size
on test model's FieldFile
instance. (It's persisting the value in an index for downline use.) It appears the underlying FileSystemStorage
class is getting and keeping the original MEDIA_ROOT
before the @override_settings
decorator gets to change my setting. It then uses that old value to build the path to call os.path.getsize(self.path(name))
.
Right now the workaround I'm using is to override the size()
method (not path()
at the moment) like so:
class MyDifferentFileSystemStorage(FileSystemStorage): def size(self, name): """ Override the size method of FileSystemStorage to work around bug in Django 1.8 where MEDIA_ROOT is not used dynamically when building the underlying absolute path. """ return os.path.getsize(os.path.join(settings.MEDIA_ROOT, name))
This looks like a bug to me as this negatively impacts using some test utils (namely override_settings
) and functional testing in general.
I'm experiencing this in v1.8, but looking at the code it appears to be the same logic in Django's master branch as well.
Change History (8)
follow-up: 2 comment:1 by , 9 years ago
comment:2 by , 9 years ago
Replying to timgraham:
Could you provide a failing test that demonstrates the problem? There is some usage of
@override_settings(MEDIA_ROOT=...)
intests/file_uploads
.
Will do, hopefully today.
comment:3 by , 9 years ago
I've created a simple Django project that recreates the issue and I think have identified some more information related to the problem. When using the built-in FileSystemStorage
class, the @override_settings()
works fine against MEDIA_ROOT
. When creating a new class based on FileSystemStorage
, even when doing nothing (see below), it causes the failure.
storage.py
from django.core.files.storage import FileSystemStorage class SubclassedFileSystemStorage(FileSystemStorage): pass
models.py:
from django.db import models from issue26038.storage import SubclassedFileSystemStorage class Media(models.Model): binary_file = models.FileField(upload_to='prod/') diff_storage_binary_file = models.FileField( upload_to='prod/', storage=SubclassedFileSystemStorage() )
I've got a project located in GitHub that quickly recreates the issue: https://github.com/voutilad/django-issue26038
comment:4 by , 9 years ago
I suspect you'll need to implement a signal listener similar to what's done in django.test.signals.
If that works, maybe we can adapt this ticket into a documentation addition unless there's an idea of how Django could take care of this automatically.
comment:5 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 1.8 → master |
I think django.core.files.storage.FileSystemStorage
should take care of recomputing its settings based properties (MEDIAL_URL
, MEDIA_ROOT
, FILE_UPLOAD_PERMISSIONS
and FILE_UPLOAD_DIRECTORY_PERMISSIONS
) by itself by registering a weak setting_changed
receiver during its initialization phase.
The default_storage
should only be reset if DEFAULT_FILE_STORAGE
is changed.
comment:7 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Could you provide a failing test that demonstrates the problem? There is some usage of
@override_settings(MEDIA_ROOT=...)
intests/file_uploads
.