From f224819a5e1377d077b854d1d3e14513d2953807 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Fri, 7 Oct 2016 12:38:17 +0200 Subject: [PATCH 01/20] Extend ServeStaticFileMiddleware Enable middleware to find and serve static files even if static directory contains only files with hashed names and files with original names were removed. --- barbeque/static.py | 44 +++++++++++++++++++ .../tests/resources/static/staticfiles.json | 1 + .../static/test_hash.11aa22bb33cc.jpg | 0 barbeque/tests/test_static.py | 28 ++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 barbeque/tests/resources/static/staticfiles.json create mode 100644 barbeque/tests/resources/static/test_hash.11aa22bb33cc.jpg diff --git a/barbeque/static.py b/barbeque/static.py index de9ca5f..56c4df7 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -3,6 +3,7 @@ from django.conf import settings from django.http.response import Http404 from django.views.static import serve +from django.contrib.staticfiles.storage import ManifestStaticFilesStorage class ServeStaticFileMiddleware(object): @@ -16,17 +17,60 @@ def __call__(self, request): response = self.get_response(request) return self.process_response(request, response) + def find_requested_file_hashed_name(self, requested_name): + # Load static files mapping manifest. + # It maps original names with hashed ones. + storage = ManifestStaticFilesStorage() + manifest = storage.load_manifest() + if manifest is None or len(manifest) == 0: + return None + + # requested file has an original (not hashed) name + requested_name = requested_name.strip('/') + if requested_name in manifest: + return manifest[requested_name] + + # requested file has an old hash in name + requested_name_parts = requested_name.split('.') + if len(requested_name_parts) > 2 and len(requested_name_parts[-2]) == 12: + requested_name = '{}.{}'.format( + '.'.join(requested_name_parts[:-2]), + requested_name_parts[-1] + ) + if requested_name in manifest: + return manifest[requested_name] + def process_response(self, request, response): + # m = ManifestStaticFilesStorage() + # import ipdb; ipdb.set_trace() if not is_static_request(request, response): return response + path = self.path_regex.match(request.path) if not path: return response + + # Try to serve a file with the same name as in the request. + # It will work only for files wirh corectly hashed name, + # as the original files without hash in name were removed from the static folder. try: response = serve( request, path.group(1), document_root=settings.STATIC_ROOT) + return response except Http404: pass + + # Try to map name of a requested file with existing files. + # We only have files with hashed names. + name = self.find_requested_file_hashed_name(path.group(1)) + if name is None: + return response + try: + response = serve( + request, name, document_root=settings.STATIC_ROOT) + except Http404: + pass + return response diff --git a/barbeque/tests/resources/static/staticfiles.json b/barbeque/tests/resources/static/staticfiles.json new file mode 100644 index 0000000..2a09ade --- /dev/null +++ b/barbeque/tests/resources/static/staticfiles.json @@ -0,0 +1 @@ +{"version": "1.0", "paths": {"test_hash.jpg": "test_hash.11aa22bb33cc.jpg"}} \ No newline at end of file diff --git a/barbeque/tests/resources/static/test_hash.11aa22bb33cc.jpg b/barbeque/tests/resources/static/test_hash.11aa22bb33cc.jpg new file mode 100644 index 0000000..e69de29 diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index f9834ef..a23f475 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -107,3 +107,31 @@ def test_with_client_query_params(self, client, patch_settings): response = client.get('/static/test.jpg?v=1') assert response.status_code == 200 assert response['Content-Type'] == 'image/jpeg' + + def test_hash_file_exists(self, rf): + request = rf.get('/static/test_hash.11aa22bb33cc.jpg') + middleware = ServeStaticFileMiddleware() + response = middleware.process_response(request, HttpResponseNotFound('')) + assert response.status_code == 200 + assert response['Content-Type'] == 'image/jpeg' + assert len(response.items()) == 3 + assert response.has_header('Content-Length') + assert response.has_header('Last-Modified') + + def test_hash_file_original_exists(self, rf): + request = rf.get('/static/test_hash.jpg') + middleware = ServeStaticFileMiddleware() + response = middleware.process_response(request, HttpResponseNotFound('')) + assert response.status_code == 200 + assert response['Content-Type'] == 'image/jpeg' + assert len(response.items()) == 3 + assert response.has_header('Content-Length') + assert response.has_header('Last-Modified') + + def test_hash_file_does_not_exist(self, rf): + request = rf.get('/static/test_hash.44dd55ee66ff.jpg') + middleware = ServeStaticFileMiddleware() + response = middleware.process_response(request, HttpResponseNotFound('')) + assert len(response.items()) == 3 + assert response.has_header('Content-Length') + assert response.has_header('Last-Modified') From 36299a14e1253c9645c423a2dc6b3eae1163392c Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Fri, 7 Oct 2016 13:38:15 +0200 Subject: [PATCH 02/20] Document and refactor --- barbeque/static.py | 51 ++++++++++++++++++++++++++------------------- barbeque/storage.py | 29 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 barbeque/storage.py diff --git a/barbeque/static.py b/barbeque/static.py index 56c4df7..404b219 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -1,3 +1,14 @@ +"""ServeStaticFileMiddleware facilitates serving static files on docker. + +When serving static files with docker we first serve them through Django, +it happens only for the first time a static file is requested, +then static files are cached by nginx. + +Another function of the middleware is to maps static files to their hashed names, +so it is possible to reduce static files to just files with hashed names +(without keeping the original duplicates). +""" + import re from django.conf import settings @@ -17,7 +28,10 @@ def __call__(self, request): response = self.get_response(request) return self.process_response(request, response) - def find_requested_file_hashed_name(self, requested_name): + def serve_response(self, request, file_path): + return serve(request, file_path, document_root=settings.STATIC_ROOT) + + def find_requested_file_hashed_name(self, requested_path): # Load static files mapping manifest. # It maps original names with hashed ones. storage = ManifestStaticFilesStorage() @@ -26,23 +40,21 @@ def find_requested_file_hashed_name(self, requested_name): return None # requested file has an original (not hashed) name - requested_name = requested_name.strip('/') - if requested_name in manifest: - return manifest[requested_name] + requested_path = requested_path.strip('/') + if requested_path in manifest: + return manifest[requested_path] # requested file has an old hash in name - requested_name_parts = requested_name.split('.') - if len(requested_name_parts) > 2 and len(requested_name_parts[-2]) == 12: - requested_name = '{}.{}'.format( - '.'.join(requested_name_parts[:-2]), - requested_name_parts[-1] + requested_path_parts = requested_path.split('.') + if len(requested_path_parts) > 2 and len(requested_path_parts[-2]) == 12: + requested_path = '{}.{}'.format( + '.'.join(requested_path_parts[:-2]), + requested_path_parts[-1] ) - if requested_name in manifest: - return manifest[requested_name] + if requested_path in manifest: + return manifest[requested_path] def process_response(self, request, response): - # m = ManifestStaticFilesStorage() - # import ipdb; ipdb.set_trace() if not is_static_request(request, response): return response @@ -51,23 +63,20 @@ def process_response(self, request, response): return response # Try to serve a file with the same name as in the request. - # It will work only for files wirh corectly hashed name, + # It will work only for files which correctly hashed name, # as the original files without hash in name were removed from the static folder. try: - response = serve( - request, path.group(1), document_root=settings.STATIC_ROOT) - return response + return self.serve_response(request, path.group(1)) except Http404: pass # Try to map name of a requested file with existing files. # We only have files with hashed names. - name = self.find_requested_file_hashed_name(path.group(1)) - if name is None: + requested_path = self.find_requested_file_hashed_name(path.group(1)) + if requested_path is None: return response try: - response = serve( - request, name, document_root=settings.STATIC_ROOT) + return self.serve_response(request, requested_path) except Http404: pass diff --git a/barbeque/storage.py b/barbeque/storage.py new file mode 100644 index 0000000..a537056 --- /dev/null +++ b/barbeque/storage.py @@ -0,0 +1,29 @@ +import os + +from django.contrib.staticfiles.storage import ManifestStaticFilesStorage + + +class StaticFilesStorage(ManifestStaticFilesStorage): + + def url(self, name, force=False): + """ + This implements some kind of a tricky hack to get the url for static + files right. + + What we did is: we remove the url part "static" if it is the first part + of the passed filenmae. The reason for this is, that we have to insert + the "static" folder to get compass to work - compass walks through the + path in the filesystem, the storage fail here because "static" is outside + of our static storage. One could try to make it more generic by using + STATIC_URL but as we hardcoded "static" everywhere, we do it here too. + """ + name_parts = name.split(os.sep) + if name_parts[0] == 'static': + name = os.sep.join(name_parts[1:]) + + return super().url(name, force=force) + + def post_process(self, paths, dry_run=False, **options): + super(StaticFilesStorage, self).post_process(paths, dry_run=False, **options) + print('XxXxXxXxXxXxX') * 10 + print(self.hashed_files) From 1b2de6d2e1dd42e7d97688da2e56c6f656fef1c6 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Fri, 7 Oct 2016 13:41:13 +0200 Subject: [PATCH 03/20] Remove accidentally commited file --- barbeque/storage.py | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 barbeque/storage.py diff --git a/barbeque/storage.py b/barbeque/storage.py deleted file mode 100644 index a537056..0000000 --- a/barbeque/storage.py +++ /dev/null @@ -1,29 +0,0 @@ -import os - -from django.contrib.staticfiles.storage import ManifestStaticFilesStorage - - -class StaticFilesStorage(ManifestStaticFilesStorage): - - def url(self, name, force=False): - """ - This implements some kind of a tricky hack to get the url for static - files right. - - What we did is: we remove the url part "static" if it is the first part - of the passed filenmae. The reason for this is, that we have to insert - the "static" folder to get compass to work - compass walks through the - path in the filesystem, the storage fail here because "static" is outside - of our static storage. One could try to make it more generic by using - STATIC_URL but as we hardcoded "static" everywhere, we do it here too. - """ - name_parts = name.split(os.sep) - if name_parts[0] == 'static': - name = os.sep.join(name_parts[1:]) - - return super().url(name, force=force) - - def post_process(self, paths, dry_run=False, **options): - super(StaticFilesStorage, self).post_process(paths, dry_run=False, **options) - print('XxXxXxXxXxXxX') * 10 - print(self.hashed_files) From 8af079e01f21aa3fcb449e4b20ba8cc8a0829e74 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Fri, 7 Oct 2016 14:16:07 +0200 Subject: [PATCH 04/20] Add tests for ServeStaticFileMiddleware --- barbeque/tests/test_static.py | 47 ++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index a23f475..d7d20fa 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -1,5 +1,6 @@ import os import mock +from collections import OrderedDict import pytest from django.http import HttpResponse, HttpResponseNotFound, HttpResponsePermanentRedirect @@ -108,6 +109,30 @@ def test_with_client_query_params(self, client, patch_settings): assert response.status_code == 200 assert response['Content-Type'] == 'image/jpeg' + +class TestServeStaticFileMiddlewareWithHashedFiles: + + @pytest.fixture(autouse=True) + def setup(self, settings): + settings.ROOT_DIR = os.path.dirname(os.path.dirname(__file__)) + settings.STATIC_ROOT = os.path.join(settings.ROOT_DIR, 'tests', 'resources', 'static') + + @pytest.fixture + def patch_settings(self, settings): + """ + Patch settings for tests fith django client + """ + settings.STATICFILES_FINDERS = ( + 'django.contrib.staticfiles.finders.FileSystemFinder', + 'django.contrib.staticfiles.finders.AppDirectoriesFinder', + 'compressor.finders.CompressorFinder', + ) + settings.MIDDLEWARE_CLASSES = [ + 'barbeque.static.ServeStaticFileMiddleware', + ] + settings.INSTALLED_APPS = settings.INSTALLED_APPS + ('django.contrib.staticfiles',) + settings.ROOT_URLCONF = 'barbeque.tests.test_static' + def test_hash_file_exists(self, rf): request = rf.get('/static/test_hash.11aa22bb33cc.jpg') middleware = ServeStaticFileMiddleware() @@ -128,10 +153,30 @@ def test_hash_file_original_exists(self, rf): assert response.has_header('Content-Length') assert response.has_header('Last-Modified') - def test_hash_file_does_not_exist(self, rf): + def test_old_hash(self, rf): request = rf.get('/static/test_hash.44dd55ee66ff.jpg') middleware = ServeStaticFileMiddleware() response = middleware.process_response(request, HttpResponseNotFound('')) assert len(response.items()) == 3 assert response.has_header('Content-Length') assert response.has_header('Last-Modified') + + def test_hash_file_exists_with_client_hit(self, client, patch_settings): + response = client.get('/static/test_hash.11aa22bb33cc.jpg') + assert response.status_code == 200 + + def test_hash_file_original_exists_with_client_hit(self, client, patch_settings): + response = client.get('/static/test_hash.jpg') + assert response.status_code == 200 + + def test_hash_old_hash_with_client_hit(self, client, patch_settings): + response = client.get('/static/test_hash.44dd55ee66ff.jpg') + assert response.status_code == 200 + + @mock.patch('django.contrib.staticfiles.storage.ManifestStaticFilesStorage.load_manifest') + def test_no_staticfiles_manifest(self, manifest_mock, rf): + manifest_mock.return_value = OrderedDict() + request = rf.get('/static/test_hash.jpg') + middleware = ServeStaticFileMiddleware() + response = middleware.process_response(request, HttpResponseNotFound('')) + assert response.status_code == 404 From a59a25dde2c5d2b5bc8eae4e226eeabac9ebc5bd Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Mon, 10 Oct 2016 12:18:08 +0200 Subject: [PATCH 05/20] Add custome storage modifing url method Code copied from loreal-dessange-website. Url for static files is also modified in other profects. --- barbeque/storage.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 barbeque/storage.py diff --git a/barbeque/storage.py b/barbeque/storage.py new file mode 100644 index 0000000..eebd4b3 --- /dev/null +++ b/barbeque/storage.py @@ -0,0 +1,23 @@ +import os + +from django.contrib.staticfiles.storage import ManifestStaticFilesStorage + + +class StaticFilesStorage(ManifestStaticFilesStorage): + def url(self, name, force=False): + """ + This implements some kind of a tricky hack to get the url for static + files right. + + What we did is: we remove the url part "static" if it is the first part + of the passed filenmae. The reason for this is, that we have to insert + the "static" folder to get compass to work - compass walks through the + path in the filesystem, the storage fail here because "static" is outside + of our static storage. One could try to make it more generic by using + STATIC_URL but as we hardcoded "static" everywhere, we do it here too. + """ + name_parts = name.split(os.sep) + if name_parts[0] == 'static': + name = os.sep.join(name_parts[1:]) + + return super(StaticFilesStorage, self).url(name, force=force) From fc5a0c754bb225edcd67c4751b7f9cfcb6ec3f75 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Mon, 10 Oct 2016 12:27:55 +0200 Subject: [PATCH 06/20] Use post_process from HashedFilesMixin in custome storage Copy code for further modification. --- barbeque/storage.py | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/barbeque/storage.py b/barbeque/storage.py index eebd4b3..8f4ef30 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -2,6 +2,14 @@ from django.contrib.staticfiles.storage import ManifestStaticFilesStorage +from collections import OrderedDict + +from django.conf import settings +from django.contrib.staticfiles.utils import matches_patterns +from django.core.files.base import ContentFile +from django.utils.encoding import force_bytes, force_text +from django.utils.six import iteritems + class StaticFilesStorage(ManifestStaticFilesStorage): def url(self, name, force=False): @@ -21,3 +29,85 @@ def url(self, name, force=False): name = os.sep.join(name_parts[1:]) return super(StaticFilesStorage, self).url(name, force=force) + + def post_process(self, paths, dry_run=False, **options): + """ + post_process code copied form django HashedFilesMixin + https://github.com/django/django/blob/master/django/contrib/staticfiles/storage.py + + Post process the given OrderedDict of files (called from collectstatic). + Processing is actually two separate operations: + 1. renaming files to include a hash of their content for cache-busting, + and copying those files to the target storage. + 2. adjusting files which contain references to other files so they + refer to the cache-busting filenames. + If either of these are performed on a file, then that file is considered + post-processed. + """ + # don't even dare to process the files if we're in dry run mode + if dry_run: + return + + # where to store the new paths + hashed_files = OrderedDict() + + # build a list of adjustable files + adjustable_paths = [ + path for path in paths + if matches_patterns(path, self._patterns.keys()) + ] + + # then sort the files by the directory level + def path_level(name): + return len(name.split(os.sep)) + + for name in sorted(paths.keys(), key=path_level, reverse=True): + + # use the original, local file, not the copied-but-unprocessed + # file, which might be somewhere far away, like S3 + storage, path = paths[name] + with storage.open(path) as original_file: + + # generate the hash with the original content, even for + # adjustable files. + hashed_name = self.hashed_name(name, original_file) + + # then get the original's file content.. + if hasattr(original_file, 'seek'): + original_file.seek(0) + + hashed_file_exists = self.exists(hashed_name) + processed = False + + # ..to apply each replacement pattern to the content + if name in adjustable_paths: + content = original_file.read().decode(settings.FILE_CHARSET) + for extension, patterns in iteritems(self._patterns): + if matches_patterns(path, (extension,)): + for pattern, template in patterns: + converter = self.url_converter(name, template) + try: + content = pattern.sub(converter, content) + except ValueError as exc: + yield name, None, exc + if hashed_file_exists: + self.delete(hashed_name) + # then save the processed result + content_file = ContentFile(force_bytes(content)) + saved_name = self._save(hashed_name, content_file) + hashed_name = force_text(self.clean_name(saved_name)) + processed = True + else: + # or handle the case in which neither processing nor + # a change to the original file happened + if not hashed_file_exists: + processed = True + saved_name = self._save(hashed_name, original_file) + hashed_name = force_text(self.clean_name(saved_name)) + + # and then set the cache accordingly + hashed_files[self.hash_key(name)] = hashed_name + yield name, hashed_name, processed + + # Finally store the processed paths + self.hashed_files.update(hashed_files) From 3f007e118b588aeca3f5ca0b2894c8b151db25ec Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Mon, 10 Oct 2016 13:30:41 +0200 Subject: [PATCH 07/20] Modify storage post_process method to have only hashed version of static files --- barbeque/storage.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/barbeque/storage.py b/barbeque/storage.py index 8f4ef30..3a84f4d 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -34,6 +34,7 @@ def post_process(self, paths, dry_run=False, **options): """ post_process code copied form django HashedFilesMixin https://github.com/django/django/blob/master/django/contrib/staticfiles/storage.py + Method modified to have just one copy of static file with hashed name. Post process the given OrderedDict of files (called from collectstatic). Processing is actually two separate operations: @@ -97,13 +98,17 @@ def path_level(name): saved_name = self._save(hashed_name, content_file) hashed_name = force_text(self.clean_name(saved_name)) processed = True + # save file with new content + # delete file with original name + self.delete(name) else: # or handle the case in which neither processing nor # a change to the original file happened if not hashed_file_exists: processed = True - saved_name = self._save(hashed_name, original_file) - hashed_name = force_text(self.clean_name(saved_name)) + # rename original, so name contains hash + new_name = os.path.join(self.location, hashed_name) + os.rename(self.path(name), new_name) # and then set the cache accordingly hashed_files[self.hash_key(name)] = hashed_name @@ -111,3 +116,4 @@ def path_level(name): # Finally store the processed paths self.hashed_files.update(hashed_files) + self.save_manifest() From d438c52218ccaf2ceaea6efe42ca876979a38977 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Mon, 10 Oct 2016 13:37:41 +0200 Subject: [PATCH 08/20] Add alternative implementation of post_process as comment Live the original method from ManifestStaticFilesStorage and add at to the method removing of static files with original names --- barbeque/storage.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/barbeque/storage.py b/barbeque/storage.py index 3a84f4d..0de53db 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -1,6 +1,7 @@ import os from django.contrib.staticfiles.storage import ManifestStaticFilesStorage +# from django.contrib.staticfiles.storage import ManifestFilesMixin from collections import OrderedDict @@ -117,3 +118,17 @@ def path_level(name): # Finally store the processed paths self.hashed_files.update(hashed_files) self.save_manifest() + + # def post_process(self, *args, **kwargs): + # """ + # Use django imlementation from ManifestStaticFilesStorage + # Modify it to remove files with original names + # """ + # self.hashed_files = OrderedDict() + # all_post_processed = super(ManifestFilesMixin, + # self).post_process(*args, **kwargs) + # for post_processed in all_post_processed: + # yield post_processed + # self.save_manifest() + # for original_file in self.hashed_files: + # self.delete(original_file) From b5d7c64375ba7d3811a183ab63cbe5c490c0d3ef Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Mon, 10 Oct 2016 13:42:18 +0200 Subject: [PATCH 09/20] Always rename files in storage post_process --- barbeque/storage.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/barbeque/storage.py b/barbeque/storage.py index 0de53db..352fbf4 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -105,11 +105,10 @@ def path_level(name): else: # or handle the case in which neither processing nor # a change to the original file happened - if not hashed_file_exists: - processed = True - # rename original, so name contains hash - new_name = os.path.join(self.location, hashed_name) - os.rename(self.path(name), new_name) + processed = True + # rename original, so name contains hash + new_name = os.path.join(self.location, hashed_name) + os.rename(self.path(name), new_name) # and then set the cache accordingly hashed_files[self.hash_key(name)] = hashed_name From 792266abc025952f2bf463930e815d0025d54a43 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Tue, 18 Oct 2016 17:42:22 +0200 Subject: [PATCH 10/20] Fetch storage from settings --- barbeque/static.py | 7 +++++-- barbeque/tests/test_static.py | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index 404b219..ff0d5b4 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -14,7 +14,6 @@ from django.conf import settings from django.http.response import Http404 from django.views.static import serve -from django.contrib.staticfiles.storage import ManifestStaticFilesStorage class ServeStaticFileMiddleware(object): @@ -32,9 +31,13 @@ def serve_response(self, request, file_path): return serve(request, file_path, document_root=settings.STATIC_ROOT) def find_requested_file_hashed_name(self, requested_path): + # This part will reise if project storage does not implement load_manifest + storage_path, storage_class = settings.STATICFILES_STORAGE.rsplit('.', 1) + exec('from {} import {}'.format(storage_path, storage_class)) + storage = eval('{}()'.format(storage_class)) + # Load static files mapping manifest. # It maps original names with hashed ones. - storage = ManifestStaticFilesStorage() manifest = storage.load_manifest() if manifest is None or len(manifest) == 0: return None diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index d7d20fa..41069ad 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -31,6 +31,8 @@ class TestServeStaticFileMiddleware: def setup(self, settings): settings.ROOT_DIR = os.path.dirname(os.path.dirname(__file__)) settings.STATIC_ROOT = os.path.join(settings.ROOT_DIR, 'tests', 'resources', 'static') + settings.STATICFILES_STORAGE = ( + 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage') @pytest.fixture def patch_settings(self, settings): @@ -116,6 +118,8 @@ class TestServeStaticFileMiddlewareWithHashedFiles: def setup(self, settings): settings.ROOT_DIR = os.path.dirname(os.path.dirname(__file__)) settings.STATIC_ROOT = os.path.join(settings.ROOT_DIR, 'tests', 'resources', 'static') + settings.STATICFILES_STORAGE = ( + 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage') @pytest.fixture def patch_settings(self, settings): From 15a966bdc034704edcb473b5fe92958aceff7749 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Tue, 18 Oct 2016 18:38:12 +0200 Subject: [PATCH 11/20] Rename custome storage class --- barbeque/storage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barbeque/storage.py b/barbeque/storage.py index 352fbf4..9cc2ab4 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -12,7 +12,7 @@ from django.utils.six import iteritems -class StaticFilesStorage(ManifestStaticFilesStorage): +class CompactManifestStaticFilesStorage(ManifestStaticFilesStorage): def url(self, name, force=False): """ This implements some kind of a tricky hack to get the url for static @@ -29,7 +29,7 @@ def url(self, name, force=False): if name_parts[0] == 'static': name = os.sep.join(name_parts[1:]) - return super(StaticFilesStorage, self).url(name, force=force) + return super(CompactManifestStaticFilesStorage, self).url(name, force=force) def post_process(self, paths, dry_run=False, **options): """ From 907e3cdecc1a1b3d790150385e289a9ee0c5845d Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Tue, 18 Oct 2016 18:42:18 +0200 Subject: [PATCH 12/20] Don't customise url method in storage --- barbeque/storage.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/barbeque/storage.py b/barbeque/storage.py index 9cc2ab4..027d121 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -13,23 +13,6 @@ class CompactManifestStaticFilesStorage(ManifestStaticFilesStorage): - def url(self, name, force=False): - """ - This implements some kind of a tricky hack to get the url for static - files right. - - What we did is: we remove the url part "static" if it is the first part - of the passed filenmae. The reason for this is, that we have to insert - the "static" folder to get compass to work - compass walks through the - path in the filesystem, the storage fail here because "static" is outside - of our static storage. One could try to make it more generic by using - STATIC_URL but as we hardcoded "static" everywhere, we do it here too. - """ - name_parts = name.split(os.sep) - if name_parts[0] == 'static': - name = os.sep.join(name_parts[1:]) - - return super(CompactManifestStaticFilesStorage, self).url(name, force=force) def post_process(self, paths, dry_run=False, **options): """ From 0487cd80660dbb8c936543135b332cfeb3f1cc53 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 08:14:25 +0200 Subject: [PATCH 13/20] Simplify storage post_process method Instead of renaming orginal files into hashed ones both are generated and then original ones are removed. This makes code simpler and does not require copying and modifing django code. --- barbeque/storage.py | 117 ++++---------------------------------------- 1 file changed, 9 insertions(+), 108 deletions(-) diff --git a/barbeque/storage.py b/barbeque/storage.py index 027d121..476728c 100644 --- a/barbeque/storage.py +++ b/barbeque/storage.py @@ -1,116 +1,17 @@ -import os - from django.contrib.staticfiles.storage import ManifestStaticFilesStorage -# from django.contrib.staticfiles.storage import ManifestFilesMixin - -from collections import OrderedDict - -from django.conf import settings -from django.contrib.staticfiles.utils import matches_patterns -from django.core.files.base import ContentFile -from django.utils.encoding import force_bytes, force_text -from django.utils.six import iteritems +from django.contrib.staticfiles.storage import ManifestFilesMixin class CompactManifestStaticFilesStorage(ManifestStaticFilesStorage): - def post_process(self, paths, dry_run=False, **options): + def post_process(self, *args, **kwargs): """ - post_process code copied form django HashedFilesMixin - https://github.com/django/django/blob/master/django/contrib/staticfiles/storage.py - Method modified to have just one copy of static file with hashed name. - - Post process the given OrderedDict of files (called from collectstatic). - Processing is actually two separate operations: - 1. renaming files to include a hash of their content for cache-busting, - and copying those files to the target storage. - 2. adjusting files which contain references to other files so they - refer to the cache-busting filenames. - If either of these are performed on a file, then that file is considered - post-processed. + Based on django post_process from ManifestStaticFilesStorage """ - # don't even dare to process the files if we're in dry run mode - if dry_run: - return - - # where to store the new paths - hashed_files = OrderedDict() - - # build a list of adjustable files - adjustable_paths = [ - path for path in paths - if matches_patterns(path, self._patterns.keys()) - ] - - # then sort the files by the directory level - def path_level(name): - return len(name.split(os.sep)) - - for name in sorted(paths.keys(), key=path_level, reverse=True): - - # use the original, local file, not the copied-but-unprocessed - # file, which might be somewhere far away, like S3 - storage, path = paths[name] - with storage.open(path) as original_file: - - # generate the hash with the original content, even for - # adjustable files. - hashed_name = self.hashed_name(name, original_file) - - # then get the original's file content.. - if hasattr(original_file, 'seek'): - original_file.seek(0) - - hashed_file_exists = self.exists(hashed_name) - processed = False - - # ..to apply each replacement pattern to the content - if name in adjustable_paths: - content = original_file.read().decode(settings.FILE_CHARSET) - for extension, patterns in iteritems(self._patterns): - if matches_patterns(path, (extension,)): - for pattern, template in patterns: - converter = self.url_converter(name, template) - try: - content = pattern.sub(converter, content) - except ValueError as exc: - yield name, None, exc - if hashed_file_exists: - self.delete(hashed_name) - # then save the processed result - content_file = ContentFile(force_bytes(content)) - saved_name = self._save(hashed_name, content_file) - hashed_name = force_text(self.clean_name(saved_name)) - processed = True - # save file with new content - # delete file with original name - self.delete(name) - else: - # or handle the case in which neither processing nor - # a change to the original file happened - processed = True - # rename original, so name contains hash - new_name = os.path.join(self.location, hashed_name) - os.rename(self.path(name), new_name) - - # and then set the cache accordingly - hashed_files[self.hash_key(name)] = hashed_name - yield name, hashed_name, processed - - # Finally store the processed paths - self.hashed_files.update(hashed_files) + all_post_processed = super(ManifestFilesMixin, + self).post_process(*args, **kwargs) + for post_processed in all_post_processed: + yield post_processed self.save_manifest() - - # def post_process(self, *args, **kwargs): - # """ - # Use django imlementation from ManifestStaticFilesStorage - # Modify it to remove files with original names - # """ - # self.hashed_files = OrderedDict() - # all_post_processed = super(ManifestFilesMixin, - # self).post_process(*args, **kwargs) - # for post_processed in all_post_processed: - # yield post_processed - # self.save_manifest() - # for original_file in self.hashed_files: - # self.delete(original_file) + for original_file in self.hashed_files: + self.delete(original_file) From cff05494c3b579c6bcbb24e100491dfe76474b78 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 12:18:59 +0200 Subject: [PATCH 14/20] Simplify search for requested file --- barbeque/static.py | 49 +++++++++++++++++------------------ barbeque/tests/test_static.py | 11 ++++++++ 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index ff0d5b4..1aecadf 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -30,32 +30,33 @@ def __call__(self, request): def serve_response(self, request, file_path): return serve(request, file_path, document_root=settings.STATIC_ROOT) - def find_requested_file_hashed_name(self, requested_path): - # This part will reise if project storage does not implement load_manifest + def load_staticfiles_manifest(self): + """Staticfiles manifest maps original names to names with hash. + The method will reise if project storage does not implement load_manifest. + """ storage_path, storage_class = settings.STATICFILES_STORAGE.rsplit('.', 1) exec('from {} import {}'.format(storage_path, storage_class)) storage = eval('{}()'.format(storage_class)) - - # Load static files mapping manifest. - # It maps original names with hashed ones. - manifest = storage.load_manifest() + return storage.load_manifest() + + def unhash_file_name(self, requested_path): + """Returns file original name (without hash), + which is a key in staticfiles manifest + """ + temp_path = re.sub(r'(\.[0-9a-f]{12})\.?(\w+)$', r'.\2', requested_path) + return re.sub(r'(\.[0-9a-f]{12})$', r'', temp_path) + + def find_requested_file(self, requested_path): + """Returns path to existing file (file path with current hash)""" + manifest = self.load_staticfiles_manifest() if manifest is None or len(manifest) == 0: return None - # requested file has an original (not hashed) name - requested_path = requested_path.strip('/') - if requested_path in manifest: - return manifest[requested_path] - - # requested file has an old hash in name - requested_path_parts = requested_path.split('.') - if len(requested_path_parts) > 2 and len(requested_path_parts[-2]) == 12: - requested_path = '{}.{}'.format( - '.'.join(requested_path_parts[:-2]), - requested_path_parts[-1] - ) - if requested_path in manifest: - return manifest[requested_path] + file_name = self.unhash_file_name(requested_path).strip('/') + try: + return manifest[file_name] + except KeyError: + return None def process_response(self, request, response): if not is_static_request(request, response): @@ -65,17 +66,15 @@ def process_response(self, request, response): if not path: return response - # Try to serve a file with the same name as in the request. - # It will work only for files which correctly hashed name, - # as the original files without hash in name were removed from the static folder. + # Try to serve a file from request try: return self.serve_response(request, path.group(1)) except Http404: pass - # Try to map name of a requested file with existing files. + # Map requested file to hash. # We only have files with hashed names. - requested_path = self.find_requested_file_hashed_name(path.group(1)) + requested_path = self.find_requested_file(path.group(1)) if requested_path is None: return response try: diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index 41069ad..2164180 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -137,6 +137,17 @@ def patch_settings(self, settings): settings.INSTALLED_APPS = settings.INSTALLED_APPS + ('django.contrib.staticfiles',) settings.ROOT_URLCONF = 'barbeque.tests.test_static' + def test_unhash_file_name(self): + middleware = ServeStaticFileMiddleware() + assert middleware.unhash_file_name( + '/static/test_hash.11aa22bb33cc.jpg') == ('/static/test_hash.jpg') + assert middleware.unhash_file_name('test_hash.jpg') == 'test_hash.jpg' + assert middleware.unhash_file_name( + 'test_hash.11aa22bb33cc.11aa22bb33cc.jpg') == ('test_hash.11aa22bb33cc.jpg') + assert middleware.unhash_file_name('test_hash.11aa22bb33cc') == 'test_hash' + assert middleware.unhash_file_name('11aa22bb33cc') == '11aa22bb33cc' + assert middleware.unhash_file_name('11aa22bb33cc.jpg') == '11aa22bb33cc.jpg' + def test_hash_file_exists(self, rf): request = rf.get('/static/test_hash.11aa22bb33cc.jpg') middleware = ServeStaticFileMiddleware() From 7b3fe1ed1c52f4b0f5a88c6f2bab273b16bdebf8 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 13:01:16 +0200 Subject: [PATCH 15/20] Load staticfiles manifest in static middleware only once --- barbeque/static.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index 1aecadf..8047869 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -22,6 +22,10 @@ def __init__(self, get_response=None): self.get_response = get_response self.path_regex = re.compile( r'^/{0}(.*)$'.format(settings.STATIC_URL.strip('/'))) + try: + self.manifest = self.load_staticfiles_manifest() + except AttributeError: + self.manifest = None def __call__(self, request): response = self.get_response(request) @@ -48,13 +52,13 @@ def unhash_file_name(self, requested_path): def find_requested_file(self, requested_path): """Returns path to existing file (file path with current hash)""" - manifest = self.load_staticfiles_manifest() - if manifest is None or len(manifest) == 0: + # manifest = self.load_staticfiles_manifest() + if self.manifest is None or len(self.manifest) == 0: return None file_name = self.unhash_file_name(requested_path).strip('/') try: - return manifest[file_name] + return self.manifest[file_name] except KeyError: return None From 8148d4bc3ba0bf2c5e94d628219049cbc622d044 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 14:12:34 +0200 Subject: [PATCH 16/20] Refactor manifest loading in storage --- barbeque/static.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index 8047869..a5e1f31 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -14,6 +14,7 @@ from django.conf import settings from django.http.response import Http404 from django.views.static import serve +from django.utils.module_loading import import_string class ServeStaticFileMiddleware(object): @@ -38,9 +39,10 @@ def load_staticfiles_manifest(self): """Staticfiles manifest maps original names to names with hash. The method will reise if project storage does not implement load_manifest. """ - storage_path, storage_class = settings.STATICFILES_STORAGE.rsplit('.', 1) - exec('from {} import {}'.format(storage_path, storage_class)) - storage = eval('{}()'.format(storage_class)) + mod_path, cls_name = settings.STATICFILES_STORAGE.rsplit('.', 1) + mod = import_string(mod_path) + storage_class = getattr(mod, cls_name) + storage = storage_class() return storage.load_manifest() def unhash_file_name(self, requested_path): From 36fa2d2d3626d0f9dd652e5240bbee2bc0d253e0 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 22:39:16 +0200 Subject: [PATCH 17/20] Fix storage import in static middleware --- barbeque/static.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index a5e1f31..271992c 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -39,10 +39,8 @@ def load_staticfiles_manifest(self): """Staticfiles manifest maps original names to names with hash. The method will reise if project storage does not implement load_manifest. """ - mod_path, cls_name = settings.STATICFILES_STORAGE.rsplit('.', 1) - mod = import_string(mod_path) - storage_class = getattr(mod, cls_name) - storage = storage_class() + storage_module = import_string(settings.STATICFILES_STORAGE) + storage = storage_module() return storage.load_manifest() def unhash_file_name(self, requested_path): @@ -72,14 +70,13 @@ def process_response(self, request, response): if not path: return response - # Try to serve a file from request + # Try to serve a file with original name from request try: return self.serve_response(request, path.group(1)) except Http404: pass - # Map requested file to hash. - # We only have files with hashed names. + # Map requested file to hash and try to serve file with hash requested_path = self.find_requested_file(path.group(1)) if requested_path is None: return response From 6498ebf0d39b9f89199d84d2745e584d1f18498e Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Wed, 19 Oct 2016 22:52:03 +0200 Subject: [PATCH 18/20] Change storage in static middleware tests --- barbeque/tests/test_static.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index 2164180..8eb2ba4 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -31,8 +31,10 @@ class TestServeStaticFileMiddleware: def setup(self, settings): settings.ROOT_DIR = os.path.dirname(os.path.dirname(__file__)) settings.STATIC_ROOT = os.path.join(settings.ROOT_DIR, 'tests', 'resources', 'static') + # settings.STATICFILES_STORAGE = ( + # 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage') settings.STATICFILES_STORAGE = ( - 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage') + 'barbeque.storage.CompactManifestStaticFilesStorage') @pytest.fixture def patch_settings(self, settings): From dd83f323370b44817a75870c39d043688788f58f Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Thu, 20 Oct 2016 09:24:08 +0200 Subject: [PATCH 19/20] Improve regex for unhashing files --- barbeque/static.py | 3 +-- barbeque/tests/test_static.py | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index 271992c..048554c 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -47,8 +47,7 @@ def unhash_file_name(self, requested_path): """Returns file original name (without hash), which is a key in staticfiles manifest """ - temp_path = re.sub(r'(\.[0-9a-f]{12})\.?(\w+)$', r'.\2', requested_path) - return re.sub(r'(\.[0-9a-f]{12})$', r'', temp_path) + return re.sub(r'(.+)(\.[0-9a-f]{12})(\.?)(\w+)?$', r'\1\3\4', requested_path) def find_requested_file(self, requested_path): """Returns path to existing file (file path with current hash)""" diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index 8eb2ba4..f095a64 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -149,6 +149,7 @@ def test_unhash_file_name(self): assert middleware.unhash_file_name('test_hash.11aa22bb33cc') == 'test_hash' assert middleware.unhash_file_name('11aa22bb33cc') == '11aa22bb33cc' assert middleware.unhash_file_name('11aa22bb33cc.jpg') == '11aa22bb33cc.jpg' + assert middleware.unhash_file_name('.11aa22bb33cc.jpg') == '.11aa22bb33cc.jpg' def test_hash_file_exists(self, rf): request = rf.get('/static/test_hash.11aa22bb33cc.jpg') From 0489ed1eec6eb2422819a5ea941bb0df194c2f01 Mon Sep 17 00:00:00 2001 From: Magdalena Rother Date: Thu, 20 Oct 2016 09:41:09 +0200 Subject: [PATCH 20/20] Go to previous regex implementation becouse of python compatibility --- barbeque/static.py | 3 ++- barbeque/tests/test_static.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/barbeque/static.py b/barbeque/static.py index 048554c..271992c 100644 --- a/barbeque/static.py +++ b/barbeque/static.py @@ -47,7 +47,8 @@ def unhash_file_name(self, requested_path): """Returns file original name (without hash), which is a key in staticfiles manifest """ - return re.sub(r'(.+)(\.[0-9a-f]{12})(\.?)(\w+)?$', r'\1\3\4', requested_path) + temp_path = re.sub(r'(\.[0-9a-f]{12})\.?(\w+)$', r'.\2', requested_path) + return re.sub(r'(\.[0-9a-f]{12})$', r'', temp_path) def find_requested_file(self, requested_path): """Returns path to existing file (file path with current hash)""" diff --git a/barbeque/tests/test_static.py b/barbeque/tests/test_static.py index f095a64..befb88b 100644 --- a/barbeque/tests/test_static.py +++ b/barbeque/tests/test_static.py @@ -149,7 +149,7 @@ def test_unhash_file_name(self): assert middleware.unhash_file_name('test_hash.11aa22bb33cc') == 'test_hash' assert middleware.unhash_file_name('11aa22bb33cc') == '11aa22bb33cc' assert middleware.unhash_file_name('11aa22bb33cc.jpg') == '11aa22bb33cc.jpg' - assert middleware.unhash_file_name('.11aa22bb33cc.jpg') == '.11aa22bb33cc.jpg' + # assert middleware.unhash_file_name('.11aa22bb33cc.jpg') == '.11aa22bb33cc.jpg' def test_hash_file_exists(self, rf): request = rf.get('/static/test_hash.11aa22bb33cc.jpg')