Skip to content

Commit

Permalink
Support for Content-Disposition: inline
Browse files Browse the repository at this point in the history
Actually all these changes do is supporting an incoming `?preview=1` query
argument on download URLs and forward the `x-disposition = inline` extra to
Giftless when requesting the download URL.

At that point datopian/giftless#90 will kick in
and the storage backend will return the appropiate headers.

The only way to pass this parameter all the way down to the action that
requests the downloadURL was to add a new argument to the download
handlers. This would break backwards compatibility so I've added code to
support plugin hooks that don't support the new `inline` arg.
  • Loading branch information
amercader committed May 14, 2021
1 parent 29ce965 commit 72f2566
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 14 deletions.
10 changes: 8 additions & 2 deletions ckanext/blob_storage/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ def get_resource_download_spec(context, data_dict):
if k not in resource:
return {}

return get_lfs_download_spec(context, resource)
inline = toolkit.asbool(data_dict.get('inline'))
return get_lfs_download_spec(context, resource, inline=inline)


def get_lfs_download_spec(context, # type: Dict[str, Any]
resource, # type: Dict[str, Any]
sha256=None, # type: Optional[str]
size=None, # type: Optional[int]
filename=None, # type: Optional[str]
storage_prefix=None # type: Optional[str]
storage_prefix=None, # type: Optional[str]
inline=False, # type: Optional[bool]
): # type: (...) -> Dict[str, Any]
"""Get the LFS download spec (URL and headers) for a resource
Expand All @@ -55,6 +57,10 @@ def get_lfs_download_spec(context, # type: Dict[str, Any]
client = context.get('download_lfs_client', LfsClient(helpers.server_url(), authz_token))

resources = [{"oid": sha256, "size": size, "x-filename": filename}]

if inline:
resources[0]["x-disposition"] = "inline"

object_spec = _get_resource_download_lfs_objects(client, storage_prefix, resources)[0]

assert object_spec['oid'] == sha256
Expand Down
7 changes: 5 additions & 2 deletions ckanext/blob_storage/blueprints.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""ckanext-blob-storage Flask blueprints
"""
from ckan.plugins import toolkit
from flask import Blueprint
from flask import Blueprint, request

from .download_handler import call_download_handlers, call_pre_download_handlers, get_context

Expand All @@ -24,10 +24,13 @@ def download(id, resource_id, filename=None):
if id != resource['package_id']:
return toolkit.abort(404, toolkit._('Resource not found belonging to package'))

inline = toolkit.asbool(request.args.get('preview'))

package = toolkit.get_action('package_show')(context, {'id': id})

resource = call_pre_download_handlers(resource, package)
return call_download_handlers(resource, package, filename)

return call_download_handlers(resource, package, filename, inline)

except toolkit.ObjectNotFound:
return toolkit.abort(404, toolkit._('Resource not found'))
Expand Down
28 changes: 23 additions & 5 deletions ckanext/blob_storage/download_handler.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import inspect

from ckan import model, plugins
from ckan.lib import uploader
Expand Down Expand Up @@ -31,28 +32,34 @@ def call_pre_download_handlers(resource, package):
return resource


def call_download_handlers(resource, package, filename=None):
def call_download_handlers(resource, package, filename=None, inline=False):
"""Call all registered plugins download handlers
"""
for plugin in plugins.PluginImplementations(IResourceDownloadHandler):
if not hasattr(plugin, 'resource_download'):
continue
response = plugin.resource_download(resource, package, filename)

if _handler_supports_inline_arg(plugin.resource_download):
response = plugin.resource_download(resource, package, filename, inline)
else:
response = plugin.resource_download(resource, package, filename)

if response:
return response

return fallback_download_method(resource)


def download_handler(resource, _, filename=None):
def download_handler(resource, _, filename=None, inline=False):
"""Get the download URL from LFS server and redirect the user there
"""
if resource.get('url_type') != 'upload' or not resource.get('lfs_prefix'):
return None

context = get_context()
data_dict = {'resource': resource,
'filename': filename}
'filename': filename,
'inline': inline}

resource_download_spec = tk.get_action('get_resource_download_spec')(context, data_dict)
href = resource_download_spec.get('href')

Expand All @@ -76,3 +83,14 @@ def fallback_download_method(resource):
return tk.abort(404, tk._('No download is available'))

return tk.redirect_to(resource[u'url'])


def _handler_supports_inline_arg(handler_function):
try:
# Python 3
args = inspect.getfullargspec(handler_function).args
except AttributeError:
# Python 2
args = inspect.getargspec(handler_function).args

return 'inline' in args
8 changes: 5 additions & 3 deletions ckanext/blob_storage/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ def pre_resource_download(self, resource, package):
"""
pass

def resource_download(self, resource, package, filename=None):
# type: (Dict[str, Any], Dict[str, Any], Optional[str]) -> Any
def resource_download(self, resource, package, filename=None, inline=False):
# type: (Dict[str, Any], Dict[str, Any], Optional[str], Optional[bool]) -> Any
"""Download a resource
Called to download a resource, with the resource, package and filename
(if specified in the download request URL) already provided, and after
some basic authorization checks such as ``resource_show`` have been
performed.
performed. Additionally if the resource needs to be displayed inline
(ie not served as an attachment) the `inline=True` argument can be
passed.
All resource download handlers will be called in plugin load order by
the Blueprint view function responsible for file downloads. The first
Expand Down
4 changes: 2 additions & 2 deletions ckanext/blob_storage/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,5 @@ def register_authz_bindings(self, authorizer):

# IResourceDownloadHandler

def resource_download(self, resource, package, filename=None):
return download_handler(resource, package, filename)
def resource_download(self, resource, package, filename=None, inline=False):
return download_handler(resource, package, filename, inline)
67 changes: 67 additions & 0 deletions ckanext/blob_storage/tests/test_blueprint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import mock
import pytest

from ckan.plugins import toolkit
from ckan.tests import factories


@pytest.mark.usefixtures('clean_db')
def test_preview_arg(app):

org = factories.Organization()
dataset = factories.Dataset(owner_org=org['id'])
resource = factories.Resource(
package_id=dataset['id'],
url_type='upload',
sha256='cc71500070cf26cd6e8eab7c9eec3a937be957d144f445ad24003157e2bd0919',
size=12,
lfs_prefix='lfs/prefix'
)

with mock.patch('ckanext.blob_storage.blueprints.call_download_handlers') as m:

m.return_value = ''

url = toolkit.url_for(
'blob_storage.download',
id=dataset['id'],
resource_id=resource['id'],
preview=1
)

app.get(url)

args = m.call_args

assert args[0][0]['id'] == resource['id'] # resource
assert args[0][1]['id'] == dataset['id'] # dataset
assert args[0][2] is None # filename
assert args[0][3] is True # inline

url = toolkit.url_for(
'blob_storage.download',
id=dataset['id'],
resource_id=resource['id'],
filename='test.csv',
preview=1
)

app.get(url)

args = m.call_args

assert args[0][2] == 'test.csv' # filename
assert args[0][3] is True # inline

url = toolkit.url_for(
'blob_storage.download',
id=dataset['id'],
resource_id=resource['id'],
filename='test.csv',
)

app.get(url)

args = m.call_args

assert args[0][3] is False # inline

0 comments on commit 72f2566

Please sign in to comment.