Encode download url, fix tests and style updates.

Author: cslzchen

mfr/core/exceptions.py

@@ -147,9 +147,8 @@ def __init__(self, message, *args, metadata_url: str='', response: str='', **kwa
 
 
 class QueryParameterError(ProviderError):
- """The MFR related errors raised from a :class:`mfr.core.provider` and relating to query parameters
- should inherit from MetadataError
- This error is thrown when a query parameter is used missused
+ """The MFR related errors raised from a :class:`mfr.core.provider`and relating to query
+ parameters. This error is thrown when the query has an invalid value.
  """
 
  __TYPE = 'query_parameter'

mfr/core/provider.py

@@ -60,8 +60,8 @@ def serialize(self):
  return {
  'name': self.name,
  'ext': self.ext,
- 'is_public': self.is_public,
  'content_type': self.content_type,
  'unique_key': str(self.unique_key),
  'download_url': str(self.download_url),
+ 'is_public': self.is_public,
  }

mfr/core/utils.py

@@ -78,25 +78,27 @@ def make_renderer(name, metadata, file_path, url, assets_url, export_url):
  normalized_name = (name and name.lower()) or 'none'
  if metadata.is_public:
  try:
+ # Use the public renderer if exist
  return driver.DriverManager(
  namespace='mfr.public_renderers',
  name=normalized_name,
  invoke_on_load=True,
  invoke_args=(metadata, file_path, url, assets_url, export_url),
  ).driver
  except:
- # Check for a public renderer, if one doesn't exist, use a regular one
- # Real exceptions handled by main driver.DriverManager
+ # If public render does not exist, use default renderer by MFR
+ # If public render exists but exceptions occurs, delay the exception handling
  pass
 
  try:
+ # Use the default MFR handler
  return driver.DriverManager(
  namespace='mfr.renderers',
  name=normalized_name,
  invoke_on_load=True,
  invoke_args=(metadata, file_path, url, assets_url, export_url),
  ).driver
- except RuntimeError:
+ except:
  raise exceptions.MakeRendererError(
  namespace='mfr.renderers',
  name=normalized_name,
@@ -110,6 +112,7 @@ def make_renderer(name, metadata, file_path, url, assets_url, export_url):
  }
  )
 
+
 def sizeof_fmt(num, suffix='B'):
  if abs(num) < 1000:
  return '%3.0f%s' % (num, suffix)

mfr/extensions/office365/render.py

@@ -1,19 +1,22 @@
 import os
+from urllib import parse
+
 import furl
+from mako.lookup import TemplateLookup
 
 from mfr.core import extension
-from mako.lookup import TemplateLookup
-from mfr.extensions.office365 import settings
+from mfr.extensions.office365 import settings as office365_settings
 
 
 class Office365Renderer(extension.BaseRenderer):
- """A renderer for use with public .docx files.
+ """A renderer for .docx files that are publicly available.
+
+ Office online can render `.docx` files to `.pdf` for us. This renderer will only be made
+ if a query param with `public_file=true` is present. It then generates and embeds an
+ office online URL into an `iframe` and returns the template. The file it is trying to
+ render MUST be public.
 
- Office online can render .docx files to pdf for us.
- This renderer will only ever be made if a query param with `public_file=1` is sent.
- It then generates and embeds an office online url into an
- iframe and returns the template. The file it is trying to render MUST
- be available publically online. This renderer will not work if testing locally.
+ Note: this renderer DOES NOT work locally.
 
  """
 
@@ -23,9 +26,10 @@ class Office365Renderer(extension.BaseRenderer):
  ]).get_template('viewer.mako')
 
  def render(self):
- download_url = furl.furl(self.metadata.download_url).set(query='')
- url = settings.OFFICE_BASE_URL + download_url.url
- return self.TEMPLATE.render(base=self.assets_url, url=url)
+ download_url = furl.furl(self.metadata.download_url).set(query='').url
+ encoded_download_url = parse.quote(download_url)
+ office_render_url = office365_settings.OFFICE_BASE_URL + encoded_download_url
+ return self.TEMPLATE.render(base=self.assets_url, url=office_render_url)
 
  @property
  def file_required(self):

mfr/providers/osf/provider.py

@@ -121,21 +121,18 @@ async def metadata(self):
  unique_key = hashlib.sha256((metadata['data']['etag'] + cleaned_url.url).encode('utf-8')).hexdigest()
 
  is_public = False
-
- if 'public_file' in cleaned_url.args:
- if cleaned_url.args['public_file'] not in ['0', '1']:
+ public_file = cleaned_url.args.get('public_file', None)
+ if public_file:
+ if public_file not in ['True', 'False']:
  raise exceptions.QueryParameterError(
- 'The `public_file` query paramter should either `0`, `1`, or unused. Instead '
- 'got {}'.format(cleaned_url.args['public_file']),
+ 'Invalid value for query parameter `public_file`: {}'.format(cleaned_url.args['public_file']),
  url=download_url,
  provider=self.NAME,
  code=400,
  )
+ is_public = public_file == 'True'
 
- is_public = cleaned_url.args['public_file'] == '1'
-
- return provider.ProviderMetadata(name, ext, content_type,
- unique_key, download_url, is_public=is_public)
+ return provider.ProviderMetadata(name, ext, content_type, unique_key, download_url, is_public=is_public)
 
  async def download(self):
  """Download file from WaterButler, returning stream."""

tests/extensions/office365/test_renderer.py

@@ -1,16 +1,23 @@
+from urllib import parse
+
 import furl
 import pytest
 
-from mfr.extensions.office365 import settings
 from mfr.core.provider import ProviderMetadata
 from mfr.extensions.office365 import Office365Renderer
+from mfr.extensions.office365 import settings as office365_settings
 
 
 @pytest.fixture
 def metadata():
- return ProviderMetadata('test', '.pdf', 'text/plain', '1234',
+ return ProviderMetadata(
+ 'test',
+ '.pdf',
+ 'text/plain',
+ '1234',
  'http://wb.osf.io/file/test.pdf?token=1234&public_file=1',
- is_public=True)
+ is_public=True
+ )
 
 
 @pytest.fixture
@@ -20,7 +27,7 @@ def file_path():
 
 @pytest.fixture
 def url():
- return 'http://osf.io/file/test.pdf'
+ return parse.quote('http://osf.io/file/test.pdf')
 
 
 @pytest.fixture
@@ -40,8 +47,8 @@ def renderer(metadata, file_path, url, assets_url, export_url):
 
 class TestOffice365Renderer:
 
- def test_render_pdf(self, renderer, metadata, assets_url):
+ def test_render_pdf(self, renderer, metadata):
  download_url = furl.furl(metadata.download_url).set(query='')
- body_url = settings.OFFICE_BASE_URL + download_url.url
+ office_render_url = office365_settings.OFFICE_BASE_URL + parse.quote(download_url.url)
  body = renderer.render()
- assert '<iframe src={} frameborder=\'0\'></iframe>'.format(body_url) in body
+ assert '<iframe src={} frameborder=\'0\'></iframe>'.format(office_render_url) in body

tests/server/handlers/test_query_params.py

@@ -7,7 +7,7 @@
 from tests import utils
 
 
-class TestRenderHandler(utils.HandlerTestCase):
+class TestQueryParamsHandler(utils.HandlerTestCase):
 
  @testing.gen_test
  def test_format_url(self):