Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass args and kwargs around so that the get_file() method can respond to URL parameters. #121

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion django_downloadview/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,10 @@ class DownloadResponse(StreamingHttpResponse):
attributes (size, name, ...).

"""

def __init__(self, file_instance, attachment=True, basename=None,
status=200, content_type=None, file_mimetype=None,
file_encoding=None):
file_encoding=None, extra_headers=None, **kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does **kwargs come from #109 ?
Is it really related to the use of args and kwargs in get_file()?

About extra_headers, I don't see the relationship with args and kwargs in get_file(). If it is another feature, can you create another pull-request for this particular change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, those are unrelated. **kwargs did come in from #109 and isn't necessary. And extra_headers came from some preliminary work on #122 that I didn't intend to include. I'll remove those unrelated changes.

"""Constructor.

:param content_type: Value for ``Content-Type`` header.
Expand Down Expand Up @@ -161,6 +162,11 @@ def __init__(self, file_instance, attachment=True, basename=None,
if header not in self:
self[header] = value # Does self support setdefault?

# Apply any extra headers.
if extra_headers:
for header, value in extra_headers:
self[header] = value

@property
def default_headers(self):
"""Return dictionary of automatically-computed headers.
Expand Down
16 changes: 11 additions & 5 deletions django_downloadview/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class DownloadMixin(object):
#: :mod:`mimetypes`.
encoding = None

def get_file(self):
def get_file(self, *args, **kwargs):
"""Return a file wrapper instance.

Raises :class:`~django_downloadview.exceptions.FileNotFound` if file
Expand Down Expand Up @@ -122,8 +122,13 @@ def was_modified_since(self, file_instance, since):
return was_modified_since(since, modification_time, size)

def not_modified_response(self, *response_args, **response_kwargs):
"""Return :class:`django.http.HttpResponseNotModified` instance."""
return HttpResponseNotModified(*response_args, **response_kwargs)
"""Return :class:`django.http.HttpResponseNotModified` instance.
Only `django.http.HttpResponseBase.__init__()` kwargs will be used."""
allowed_kwargs = {}
for k in ('content_type', 'status', 'reason', 'charset'):
if k in response_kwargs:
allowed_kwargs[k] = response_kwargs[k]
return HttpResponseNotModified(*response_args, **allowed_kwargs)

def download_response(self, *response_args, **response_kwargs):
"""Return :class:`~django_downloadview.response.DownloadResponse`."""
Expand Down Expand Up @@ -151,7 +156,7 @@ def render_to_response(self, *response_args, **response_kwargs):

"""
try:
self.file_instance = self.get_file()
self.file_instance = self.get_file(*response_args, **response_kwargs)
except exceptions.FileNotFound:
return self.file_not_found_response()
# Respect the If-Modified-Since header.
Expand All @@ -165,6 +170,7 @@ def render_to_response(self, *response_args, **response_kwargs):

class BaseDownloadView(DownloadMixin, View):
"""A base :class:`DownloadMixin` that implements :meth:`get`."""

def get(self, request, *args, **kwargs):
"""Handle GET requests: stream a file."""
return self.render_to_response()
return self.render_to_response(*args, **kwargs)
11 changes: 8 additions & 3 deletions tests/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

class DownloadMixinTestCase(unittest.TestCase):
"""Test suite around :class:`django_downloadview.views.DownloadMixin`."""

def test_get_file(self):
"""DownloadMixin.get_file() raise NotImplementedError.

Expand Down Expand Up @@ -218,21 +219,23 @@ def test_file_not_found_response(self):

class BaseDownloadViewTestCase(unittest.TestCase):
"Tests around :class:`django_downloadviews.views.base.BaseDownloadView`."

def test_get(self):
"""BaseDownloadView.get() calls render_to_response()."""
request = django.test.RequestFactory().get('/dummy-url')
args = ['dummy-arg']
kwargs = {'dummy': 'kwarg'}
args = []
kwargs = {'content_type': 'application/pdf'}
view = setup_view(views.BaseDownloadView(), request, *args, **kwargs)
view.render_to_response = mock.Mock(
return_value=mock.sentinel.response)
response = view.get(request, *args, **kwargs)
self.assertIs(response, mock.sentinel.response)
view.render_to_response.assert_called_once_with()
view.render_to_response.assert_called_once_with(*args, **kwargs)


class PathDownloadViewTestCase(unittest.TestCase):
"Tests for :class:`django_downloadviews.views.path.PathDownloadView`."

def test_get_file_ok(self):
"PathDownloadView.get_file() returns ``File`` instance."
view = setup_view(views.PathDownloadView(path=__file__),
Expand Down Expand Up @@ -262,6 +265,7 @@ def test_get_file_is_directory(self):

class ObjectDownloadViewTestCase(unittest.TestCase):
"Tests for :class:`django_downloadviews.views.object.ObjectDownloadView`."

def test_get_file_ok(self):
"ObjectDownloadView.get_file() returns ``file`` field by default."
view = setup_view(views.ObjectDownloadView(), 'fake request')
Expand Down Expand Up @@ -296,6 +300,7 @@ def test_get_file_empty_field(self):
class VirtualDownloadViewTestCase(unittest.TestCase):
"""Test suite around
:py:class:`django_downloadview.views.VirtualDownloadView`."""

def test_was_modified_since_specific(self):
"""VirtualDownloadView.was_modified_since() delegates to file wrapper.

Expand Down