ManifestFilesMixin.file_hash() returning None get's included in hashed filename as 'None'.

[Richard Campen]

When returning a string from a custom ManifestFilesMixin.file_hash() implementation, the resulting file name is <file_path>.<custom_hash>.<ext> as expected, whereas returning None results in <file_path>None.<ext>.

​Discussion on django-developers supports this behaviour being unintended.

Behavior appears to have been introduced with #17896 which split the file hashing into a separate method.

The following test, when included in the test_storage.TestCollectionManifestStorage test class demonstrates the bug:

def test_hashed_name_unchanged_when_file_hash_is_None(self):
    with mock.patch('django.contrib.staticfiles.storage.ManifestStaticFilesStorage.file_hash', return_value=None):
        self.assertEqual(storage.staticfiles_storage.hashed_name('test/file.txt'), 'test/file.txt')

As suggested by the name of my test, my opinion is that the correct behaviour should be that if file_hash returns None, then no hash is inserted into the filename and it therefore remains unchanged.

With that in mind, a possible solution is to change the following lines in the hashed_name() method (~line 100 in staticfiles.storage):

if file_hash is not None:
    file_hash = ".%s" % file_hash
hashed_name = os.path.join(path, "%s%s%s" % (root, file_hash, ext))

to

if file_hash is None:
    file_hash = ""
else:
    file_hash = ".%s" % file_hash
hashed_name = os.path.join(path, "%s%s%s" % (root, file_hash, ext))

[Mariusz Felisiak]

Thanks for this ticket.

[Richard Campen]

Thanks for the feedback. I've update the PR as per your recommendations.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 67b334f:

Fixed #31517 -- Fixed HashedFilesMixin.hashed_name() if hash of the file is None.