From 31c0a0f4b98a9e7795a561c08965e1dbfc703871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Bryon?= Date: Tue, 14 May 2013 00:25:30 +0200 Subject: [PATCH] Refs #34 - Introduced unit tests for DownloadMixin.was_modified_since(). Refactored DownloadMixin: easier to test, better docstrings. --- Makefile | 7 +++- django_downloadview/views.py | 70 +++++++++++++++++++++++++++--------- etc/buildout.cfg | 2 ++ tests/views.py | 50 ++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 17 deletions(-) create mode 100644 tests/views.py diff --git a/Makefile b/Makefile index 864772c..2398327 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,12 @@ maintainer-clean: distclean rm -rf $(ROOT_DIR)/lib/ -test: test-demo test-documentation +test: test-app test-demo test-documentation + + +test-app: + $(NOSE) -c $(ROOT_DIR)/etc/nose.cfg --with-coverage --cover-package=django_downloadview django_downloadview tests + mv $(ROOT_DIR)/.coverage $(ROOT_DIR)/var/test/app.coverage test-demo: diff --git a/django_downloadview/views.py b/django_downloadview/views.py index 36d401d..8a23dc7 100644 --- a/django_downloadview/views.py +++ b/django_downloadview/views.py @@ -1,3 +1,4 @@ +# coding=utf-8 """Views.""" from django.core.files import File from django.core.files.storage import DefaultStorage @@ -42,28 +43,65 @@ class DownloadMixin(object): def get_basename(self): return self.basename - def render_to_response(self, *args, **kwargs): - """Returns a response with a file as attachment.""" - # Respect the If-Modified-Since header. - file_instance = self.get_file() - if_modified_since = self.request.META.get('HTTP_IF_MODIFIED_SINCE', - None) - if if_modified_since is not None and \ - hasattr(file_instance, 'modified_time'): - modification_time = file_instance.modified_time - size = file_instance.size - if not was_modified_since(if_modified_since, modification_time, - size): - content_type = file_instance.content_type - return HttpResponseNotModified(content_type=content_type) - # Return download response. - response_kwargs = {'file_instance': file_instance, + def was_modified_since(self, file_instance, since): + """Return True if ``file_instance`` was modified after ``since``. + + Uses file wrapper's ``was_modified_since`` if available, with value of + ``since`` as positional argument. + + Else, fallbacks to default implementation, which uses + :py:func:`django.views.static.was_modified_since`. + + Django's ``was_modified_since`` function needs a datetime and a size. + It is passed ``modified_time`` and ``size`` attributes from file + wrapper. If file wrapper does not support these attributes + (``AttributeError`` or ``NotImplementedError`` is raised), then + the file is considered as modified and ``True`` is returned. + + """ + try: + return file_instance.was_modified_since(since) + except (AttributeError, NotImplementedError): + try: + modification_time = file_instance.modified_time + size = file_instance.size + except (AttributeError, NotImplementedError): + return True + else: + return was_modified_since(since, modification_time, size) + + def not_modified_response(self, *args, **kwargs): + """Return :py:class:`django.http.HttpResponseNotModified` instance.""" + content_type = self.file_instance.content_type + return HttpResponseNotModified(content_type=content_type) + + def download_response(self, *args, **kwargs): + """Return :py:class:`DownloadResponse` instance.""" + response_kwargs = {'file_instance': self.file_instance, 'attachment': self.attachment, 'basename': self.get_basename()} response_kwargs.update(kwargs) response = self.response_class(**response_kwargs) return response + def render_to_response(self, *args, **kwargs): + """Return a download response. + + Respects the "HTTP_IF_MODIFIED_SINCE" header if any. In that case, uses + :py:meth:`was_modified_since` and :py:meth:`not_modified_response`. + + Else, uses :py:meth:`download_response` to return a download response. + + """ + self.file_instance = self.get_file() + # Respect the If-Modified-Since header. + since = self.request.META.get('HTTP_IF_MODIFIED_SINCE', None) + if since is not None: + if not self.was_modified_since(self.file_instance, since): + return self.not_modified_response(*args, **kwargs) + # Return download response. + return self.download_response(*args, **kwargs) + class BaseDownloadView(DownloadMixin, View): def get(self, request, *args, **kwargs): diff --git a/etc/buildout.cfg b/etc/buildout.cfg index 80b8743..d11fed7 100644 --- a/etc/buildout.cfg +++ b/etc/buildout.cfg @@ -31,6 +31,7 @@ parts = recipe = z3c.recipe.scripts eggs = django-downloadview-demo + mock bpython nose rednose @@ -65,6 +66,7 @@ django-nose = 1.1 docutils = 0.10 evg.recipe.activate = 0.5 Jinja2 = 2.6 +mock = 1.0.1 nose = 1.2.1 Pygments = 1.6 python-termstyle = 0.1.9 diff --git a/tests/views.py b/tests/views.py new file mode 100644 index 0000000..77ebca4 --- /dev/null +++ b/tests/views.py @@ -0,0 +1,50 @@ +# coding=utf-8 +"""Tests around :py:mod:`django_downloadview.views`.""" +import unittest +try: + from unittest import mock +except ImportError: + import mock + +from django_downloadview import views + + +def setup_view(view, request, *args, **kwargs): + """Mimic as_view() returned callable, but returns view instance. + + ``*args`` and ``**kwargs`` are the same you would pass to ``reverse()`` + + """ + view.request = request + view.args = args + view.kwargs = kwargs + return view + + +class DownloadMixinTestCase(unittest.TestCase): + """Test suite around :py:class:`django_downloadview.views.DownloadMixin`. + """ + def test_was_modified_since_specific(self): + """DownloadMixin.was_modified_since() delegates to file wrapper.""" + file_wrapper = mock.Mock() + file_wrapper.was_modified_since = mock.Mock( + return_value=mock.sentinel.return_value) + mixin = views.DownloadMixin() + since = mock.sentinel.since + return_value = mixin.was_modified_since(file_wrapper, since) + self.assertEqual(return_value, mock.sentinel.return_value) + file_wrapper.was_modified_since.assert_called_once_with(since) + + def test_was_modified_since_not_implemented(self): + """DownloadMixin.was_modified_since() returns True if file wrapper + does not support ``modified_time`` or ``size`` attributes.""" + fields = ['modified_time', 'size'] + side_effects = [AttributeError('fake'), NotImplementedError('fake')] + for field in fields: + for side_effect in side_effects: + file_wrapper = mock.Mock() + setattr(file_wrapper, field, mock.Mock( + side_effect=AttributeError('fake'))) + mixin = views.DownloadMixin() + since = mock.sentinel.since + self.assertTrue(mixin.was_modified_since(file_wrapper, since))