Make data file names POSIX compliant #2732
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2732 +/- ##
===========================================
- Coverage 56.11% 55.86% -0.26%
===========================================
Files 398 398
Lines 25583 25423 -160
Branches 1240 1239 -1
===========================================
- Hits 14356 14202 -154
+ Misses 11227 11221 -6
Continue to review full report at Codecov.
|
| @@ -47,15 +48,11 @@ def _mkdir(path): | |||
|
|
|||
| def file_path(instance, filename): | |||
| """Construct relative file system path for new file store files relative to | |||
| FILE_STORE_BASE_DIR. | |||
| FILE_STORE_BASE_DIR | |||
| instance: FileStoreItem instance | |||
There was a problem hiding this comment.
It doesn't look like instance ever gets used here
There was a problem hiding this comment.
Correct. However, file_path is passed to upload_to as a callable.
"This callable must be able to accept two arguments"
https://docs.djangoproject.com/en/1.8/ref/models/fields/#django.db.models.FileField.upload_to
| @@ -84,6 +75,43 @@ def test_file_source_map(self): | |||
| '/example/path/test_file.dat') | |||
|
|
|||
|
|
|||
| class FilePathTest(TestCase): | |||
|
|
|||
| def test_file_path_format(self): | |||
There was a problem hiding this comment.
I don't believe file_path needs a FileStoreItem instance in any of these tests
There was a problem hiding this comment.
The signature of file_path requires a FileStoreItem instance.
| # max length: 255, allowed character set: [A-Za-z0-9._-] | ||
| # remove leading '-' characters to make file management easier | ||
| filename = re.sub('[^A-Za-z0-9._-]', '_', | ||
| string.lstrip(filename, '-'))[-255:] |
There was a problem hiding this comment.
Is the idea that the last part of the filename has the extension, and you definitely want to preserve that? Hopefully we never get files this long, but for people, the start of the name is also significant... it'd be more complicated, and probably not worth it, but I could imagine removing the part of the name just before the extension.
There was a problem hiding this comment.
I think most file systems in use today have a limit of 255 on file name length which means there should really be no files that are above that. So this is more of a last line of defense for which simple handling is sufficient.
Fix #2553