From cffbc1e875bff0819c2b489e148ec913b0d98a91 Mon Sep 17 00:00:00 2001 From: Andy McKay Date: Sun, 5 Jun 2011 19:57:45 +0100 Subject: [PATCH] put file and fragment on seperate urls to keep all the caching happy (bug 661710) --- apps/files/decorators.py | 4 ++-- apps/files/helpers.py | 8 +++++--- apps/files/tests/test_helpers.py | 2 +- apps/files/tests/test_views.py | 4 ++-- apps/files/urls.py | 6 ++++-- apps/files/views.py | 14 ++++++-------- media/js/zamboni/files.js | 2 +- 7 files changed, 21 insertions(+), 19 deletions(-) diff --git a/apps/files/decorators.py b/apps/files/decorators.py index 1df9c2978f..53a376ba06 100644 --- a/apps/files/decorators.py +++ b/apps/files/decorators.py @@ -46,11 +46,11 @@ def _get_value(obj, key, value, cast=None): return cast(value) if cast else value -def last_modified(request, obj, key=None): +def last_modified(request, obj, key=None, **kw): return _get_value(obj, key, 'modified', datetime.fromtimestamp) -def etag(request, obj, key=None): +def etag(request, obj, key=None, **kw): return _get_value(obj, key, 'md5') diff --git a/apps/files/helpers.py b/apps/files/helpers.py index c0c41a4fa9..b5f8ba6c9b 100644 --- a/apps/files/helpers.py +++ b/apps/files/helpers.py @@ -236,7 +236,6 @@ class FileViewer: short = smart_unicode(path[len(self.dest) + 1:]) mime, encoding = mimetypes.guess_type(filename) directory = os.path.isdir(path) - args = [self.file.id, short] res[short] = {'binary': self._is_binary(mime, path), 'depth': short.count(os.sep), 'directory': directory, @@ -249,8 +248,10 @@ class FileViewer: 'short': short, 'size': os.stat(path)[stat.ST_SIZE], 'truncated': self.truncate(filename), - 'url': reverse('files.list', args=args), - 'url_serve': reverse('files.redirect', args=args), + 'url': reverse('files.list', + args=[self.file.id, 'file', short]), + 'url_serve': reverse('files.redirect', + args=[self.file.id, short]), 'version': self.file.version.version} return res @@ -278,6 +279,7 @@ class DiffHelper: def get_url(self, short): return reverse('files.compare', args=[self.left.file.id, self.right.file.id, + 'file', short]) @memoize(prefix='file-viewer-get-files', time=60 * 60) diff --git a/apps/files/tests/test_helpers.py b/apps/files/tests/test_helpers.py index fdf2dcd8f4..884adc5e6f 100644 --- a/apps/files/tests/test_helpers.py +++ b/apps/files/tests/test_helpers.py @@ -130,7 +130,7 @@ class TestFileHelper(test_utils.TestCase): self.viewer.extract() files = self.viewer.get_files() url = reverse('files.list', args=[self.viewer.file.id, - 'install.js']) + 'file', 'install.js']) assert files['install.js']['url'].endswith(url) def test_get_files_depth(self): diff --git a/apps/files/tests/test_views.py b/apps/files/tests/test_views.py index b08fc66390..f0d0fae204 100644 --- a/apps/files/tests/test_views.py +++ b/apps/files/tests/test_views.py @@ -256,7 +256,7 @@ class TestFileViewer(FilesBase, test_utils.TestCase): def file_url(self, file=None): args = [self.file.pk] if file: - args.append(file) + args.extend(['file', file]) return reverse('files.list', args=args) def check_urls(self, status): @@ -396,7 +396,7 @@ class TestDiffViewer(FilesBase, test_utils.TestCase): def file_url(self, file=None): args = [self.file.pk, self.file_two.pk] if file: - args.append(file) + args.extend(['file', file]) return reverse('files.compare', args=args) def check_urls(self, status): diff --git a/apps/files/urls.py b/apps/files/urls.py index ec42500a9f..c45d92ce81 100644 --- a/apps/files/urls.py +++ b/apps/files/urls.py @@ -4,7 +4,8 @@ from files import views file_patterns = patterns('', url(r'^$', views.browse, name='files.list'), - url(r'file/(?P.*)$', views.browse, name='files.list'), + url(r'^(?Pfragment|file)/(?P.*)$', views.browse, + name='files.list'), url(r'file-redirect/(?P.*)$', views.redirect, name='files.redirect'), url(r'file-serve/(?P.*)$', views.serve, name='files.serve'), @@ -13,7 +14,8 @@ file_patterns = patterns('', compare_patterns = patterns('', url(r'^$', views.compare, name='files.compare'), - url(r'file/(?P.*)$', views.compare, name='files.compare'), + url(r'(?Pfragment|file)/(?P.*)$', views.compare, + name='files.compare'), url(r'status$', views.compare_poll, name='files.compare.poll'), ) diff --git a/apps/files/views.py b/apps/files/views.py index 80bc766edc..bf5e8a08de 100644 --- a/apps/files/views.py +++ b/apps/files/views.py @@ -49,7 +49,7 @@ def poll(request, viewer): @file_view @condition(etag_func=etag, last_modified_func=last_modified) -def browse(request, viewer, key=None): +def browse(request, viewer, key=None, type='file'): data = setup_viewer(request, viewer.file) data['viewer'] = viewer data['poll_url'] = reverse('files.poll', args=[viewer.file.id]) @@ -72,9 +72,8 @@ def browse(request, viewer, key=None): else: extract_file.delay(viewer) - tmpl = ('files/content.html' if not request.GET.get('full') - and request.is_ajax() - else 'files/viewer.html') + tmpl = ('files/content.html' if type == 'fragment' + else 'files/viewer.html') return jingo.render(request, tmpl, data) @@ -92,7 +91,7 @@ def compare_poll(request, diff): @compare_file_view @condition(etag_func=etag, last_modified_func=last_modified) -def compare(request, diff, key=None): +def compare(request, diff, key=None, type='file'): data = setup_viewer(request, diff.left.file) data['diff'] = diff data['poll_url'] = reverse('files.compare.poll', @@ -121,9 +120,8 @@ def compare(request, diff, key=None): extract_file.delay(diff.left) extract_file.delay(diff.right) - tmpl = ('files/content.html' if not request.GET.get('full') - and request.is_ajax() - else 'files/viewer.html') + tmpl = ('files/content.html' if type == 'fragment' + else 'files/viewer.html') return jingo.render(request, tmpl, data) diff --git a/media/js/zamboni/files.js b/media/js/zamboni/files.js index cec452bc96..cb453014f5 100644 --- a/media/js/zamboni/files.js +++ b/media/js/zamboni/files.js @@ -199,7 +199,7 @@ function bind_viewer(nodes) { history.pushState({ path: $link.text() }, '', $link.attr('href') + '#top'); } } - $old_wrapper.load($link.attr('href') + ' #content-wrapper', function() { + $old_wrapper.load($link.attr('href').replace('/file/', '/fragment/') + ' #content-wrapper', function() { $(this).children().unwrap(); var $new_wrapper = $('#content-wrapper'); self.compute($new_wrapper);