Refs #34 - Introduced unit tests for DownloadMixin.was_modified_since(). Refactored DownloadMixin: easier to test, better docstrings.

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:

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):

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

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))