Merge pull request #845 from krinsman/step5

Step 5

Author: parente

nbviewer/providers/base.py

@@ -57,6 +57,8 @@ class BaseHandler(web.RequestHandler):
     """Base Handler class with common utilities"""
 
     def initialize(self, format=None, format_prefix="", **handler_settings):
+        # format: str, optional
+        #     Rendering format (e.g. script, slides, html)
         self.format = format or self.default_format
         self.format_prefix = format_prefix
         self.http_client = httpclient.AsyncHTTPClient()
@@ -263,13 +265,13 @@ def render_template(self, name, **namespace):
         namespace.update(self.template_namespace)
         template = self.get_template(name)
         return template.render(**namespace)
-    
+
     # Wrappers to facilitate custom rendering in subclasses without having to rewrite entire GET methods
     # This would seem to mostly involve creating different template namespaces to enable custom logic in
     # extended templates, but there might be other possibilities
     def render_status_code_template(self, status_code, **namespace):
         return self.render_template('%d.html' % status_code, **namespace)
-    
+
     def render_error_template(self, **namespace):
         return self.render_template('error.html', **namespace)
 
@@ -630,6 +632,31 @@ def filter_formats(self, nb, raw):
             except Exception as err:
                 app_log.info("failed to test %s: %s", self.request.uri, name)
 
+    # empty methods to be implemented by subclasses to make GET requests more modular
+    def get_notebook_data(self, **kwargs):
+        """
+        Pass as kwargs variables needed to define those variables which will be necessary for 
+        the provider to find the notebook. (E.g. path for LocalHandler, user and repo for GitHub.) 
+        Return variables the provider needs to find and load the notebook. Then run custom logic
+        in GET or pass the output of get_notebook_data immediately to deliver_notebook.
+
+        First part of any provider's GET method. 
+
+        Custom logic, if applicable, is middle part of any provider's GET method, and usually
+        is implemented or overwritten in subclasses, while get_notebook_data and deliver_notebook
+        will often remain unchanged from the parent class (e.g. for a custom GitHub provider).
+        """
+        pass
+
+    def deliver_notebook(self, **kwargs):
+        """
+        Pass as kwargs the return values of get_notebook_data to this method. Get the JSON data
+        from the provider to render the notebook. Finish with a call to self.finish_notebook.
+
+        Last part of any provider's GET method.
+        """
+        pass 
+
     # Wrappers to facilitate custom rendering in subclasses without having to rewrite entire GET methods
     # This would seem to mostly involve creating different template namespaces to enable custom logic in
     # extended templates, but there might be other possibilities
@@ -658,30 +685,10 @@ def finish_notebook(self, json_notebook, download_url, msg=None,
             Notebook document in JSON format
         download_url: str
             URL to download the notebook document
-        provider_url: str, optional
-            URL to the notebook document upstream at the provider (e.g., GitHub)
-        provider_icon: str, optional
-            CSS classname to apply to the navbar icon linking to the provider
-        provider_label: str, optional
-            Text to to apply to the navbar icon linking to the provider
         msg: str, optional
             Extra information to log when rendering fails
-        breadcrumbs: list of dict, optional
-            Breadcrumb 'name' and 'url' to render as links at the top of the notebook page
         public: bool, optional
             True if the notebook is public and its access indexed, False if not
-        format: str, optional
-            Rendering format (e.g., script, slides, html)
-        request: tornado.httputil.HTTPServerRequest, optional
-            HTTP request that triggered notebook rendering
-        title: str, optional
-            Title to use as the HTML page title (i.e., text on the browser tab)
-        executor_url: str, optional
-            URL to execute the notebook document (e.g., Binder)
-        executor_label: str, optional
-            Text to apply to the navbar icon linking to the execution service
-        executor_icon: str, optional
-            CSS classname to apply to the navbar icon linking to the execution service
         """
 
         if msg is None:

nbviewer/providers/gist/handlers.py

@@ -23,18 +23,29 @@
 
 from .. import _load_handler_from_location
 
-
 class GistClientMixin(GithubClientMixin):
+    
+    # PROVIDER_CTX is a dictionary whose entries are passed as keyword arguments
+    # to the render_template method of the GistHandler. The following describe
+    # the information contained in each of these keyword arguments:
+    # provider_label: str
+    #     Text to to apply to the navbar icon linking to the provider
+    # provider_icon: str
+    #     CSS classname to apply to the navbar icon linking to the provider
+    # executor_label: str, optional
+    #     Text to apply to the navbar icon linking to the execution service
+    # executor_icon: str, optional
+    #     CSS classname to apply to the navbar icon linking to the execution service
     PROVIDER_CTX = {
         'provider_label': 'Gist',
         'provider_icon': 'github-square',
         'executor_label': 'Binder',
         'executor_icon': 'icon-binder',
     }
-    
+
     BINDER_TMPL = '{binder_base_url}/gist/{user}/{gist_id}/master'
     BINDER_PATH_TMPL = BINDER_TMPL+'?filepath={path}'
-    
+
     def client_error_message(self, exc, url, body, msg=None):
         if exc.code == 403 and 'too big' in body.lower():
             return 400, "GitHub will not serve raw gists larger than 10MB"
@@ -50,12 +61,18 @@ class UserGistsHandler(GistClientMixin, BaseHandler):
     .ipynb file extension is required for listing (not for rendering).
     """
     def render_usergists_template(self, entries, user, provider_url, prev_url,
-                                 next_url, **namespace):
+                                  next_url, **namespace):
+        """
+        provider_url: str
+            URL to the notebook document upstream at the provider (e.g., GitHub)
+        executor_url: str, optional (kwarg passed into `namespace`)
+            URL to execute the notebook document (e.g., Binder)
+        """
         return self.render_template("usergists.html", entries=entries, user=user,
-                                   provider_url=provider_url, prev_url=prev_url,
-                                   next_url=next_url, **self.PROVIDER_CTX,
-                                   **namespace)
-    
+                                    provider_url=provider_url, prev_url=prev_url,
+                                    next_url=next_url, **self.PROVIDER_CTX, 
+                                    **namespace)
+
     @cached
     @gen.coroutine
     def get(self, user, **namespace):
@@ -88,14 +105,17 @@ def get(self, user, **namespace):
 
 class GistHandler(GistClientMixin, RenderingHandler):
     """render a gist notebook, or list files if a multifile gist"""
-    @cached
+
     @gen.coroutine
-    def get(self, user, gist_id, filename=''):
+    def parse_gist(self, user, gist_id, filename=''):
+
         with self.catch_client_error():
             response = yield self.github_client.get_gist(gist_id)
 
         gist = json.loads(response_text(response))
+
         gist_id=gist['id']
+
         if user is None:
             # redirect to /gist/user/gist_id if no user given
             owner_dict = gist.get('owner', {})
@@ -111,94 +131,151 @@ def get(self, user, gist_id, filename=''):
             return
 
         files = gist['files']
+
         many_files_gist = (len(files) > 1)
 
-        if not many_files_gist and not filename:
-            filename = list(files.keys())[0]
+        # user and gist_id get modified
+        return user, gist_id, gist, files, many_files_gist
 
-        if filename and filename in files:
-            file = files[filename]
-            if (file['type'] or '').startswith('image/'):
-                app_log.debug("Fetching raw image (%s) %s/%s: %s", file['type'], gist_id, filename, file['raw_url'])
-                response = yield self.fetch(file['raw_url'])
-                # use raw bytes for images:
-                content = response.body
-            elif file['truncated']:
-                app_log.debug("Gist %s/%s truncated, fetching %s", gist_id, filename, file['raw_url'])
-                response = yield self.fetch(file['raw_url'])
-                content = response_text(response, encoding='utf-8')
+   # Analogous to GitHubTreeHandler
+    @gen.coroutine
+    def tree_get(self, user, gist_id, gist, files):
+        """
+        user, gist_id, gist, and files are (most) of the values returned by parse_gist
+        """
+        entries = []
+        ipynbs = []
+        others = []
+
+        for file in files.values():
+            e = {}
+            e['name'] = file['filename']
+            if file['filename'].endswith('.ipynb'):
+                e['url'] = quote('/%s/%s' % (gist_id, file['filename']))
+                e['class'] = 'fa-book'
+                ipynbs.append(e)
             else:
-                content = file['content']
-
-            # Enable a binder navbar icon if a binder base URL is configured
-            executor_url = self.BINDER_PATH_TMPL.format(
-                binder_base_url=self.binder_base_url,
-                user=user.rstrip('/'),
-                gist_id=gist_id,
-                path=quote(filename)
-            ) if self.binder_base_url else None
-
-            if not many_files_gist or filename.endswith('.ipynb'):
-                yield self.finish_notebook(
-                    content,
-                    file['raw_url'],
-                    provider_url=gist['html_url'],
-                    executor_url=executor_url,
-                    msg="gist: %s" % gist_id,
-                    public=gist['public'],
-                    **self.PROVIDER_CTX
+                provider_url = u"https://gist.github.com/{user}/{gist_id}#file-{clean_name}".format(
+                    user=user,
+                    gist_id=gist_id,
+                    clean_name=clean_filename(file['filename']),
                 )
-            else:
-                self.set_header('Content-Type', file.get('type') or 'text/plain')
-                # cannot redirect because of X-Frame-Content
-                self.finish(content)
-                return
+                e['url'] = provider_url
+                e['class'] = 'fa-share'
+                others.append(e)
+
+        entries.extend(ipynbs)
+        entries.extend(others)
+
+        # Enable a binder navbar icon if a binder base URL is configured
+        executor_url = self.BINDER_TMPL.format(
+            binder_base_url=self.binder_base_url,
+            user=user.rstrip('/'),
+            gist_id=gist_id
+        ) if self.binder_base_url else None
+
+        # provider_url:
+        #     URL to the notebook document upstream at the provider (e.g., GitHub)
+        # executor_url: str, optional
+        #     URL to execute the notebook document (e.g., Binder)
+        html = self.render_template(
+            'treelist.html',
+            entries=entries,
+            tree_type='gist',
+            tree_label='gists',
+            user=user.rstrip('/'),
+            provider_url=gist['html_url'],
+            executor_url=executor_url,
+            **self.PROVIDER_CTX
+        )
+        yield self.cache_and_finish(html)
 
-        elif filename:
-            raise web.HTTPError(404, "No such file in gist: %s (%s)", filename, list(files.keys()))
+    # Analogous to GitHubBlobHandler
+    @gen.coroutine
+    def file_get(self, user, gist_id, filename, gist, many_files_gist, file):
+        content = yield self.get_notebook_data(gist_id, filename, many_files_gist, file)
+
+        if not content:
+            return
+        
+        yield self.deliver_notebook(user, gist_id, filename, gist, file, content)
+
+    # Only called by file_get
+    @gen.coroutine
+    def get_notebook_data(self, gist_id, filename, many_files_gist, file):
+        """
+        gist_id, filename, many_files_gist, file are all passed to file_get
+        """
+        if (file['type'] or '').startswith('image/'):
+            app_log.debug("Fetching raw image (%s) %s/%s: %s", file['type'], gist_id, filename, file['raw_url'])
+            response = yield self.fetch(file['raw_url'])
+            # use raw bytes for images:
+            content = response.body
+        elif file['truncated']:
+            app_log.debug("Gist %s/%s truncated, fetching %s", gist_id, filename, file['raw_url'])
+            response = yield self.fetch(file['raw_url'])
+            content = response_text(response, encoding='utf-8')
         else:
-            entries = []
-            ipynbs = []
-            others = []
-
-            for file in files.values():
-                e = {}
-                e['name'] = file['filename']
-                if file['filename'].endswith('.ipynb'):
-                    e['url'] = quote('/%s/%s' % (gist_id, file['filename']))
-                    e['class'] = 'fa-book'
-                    ipynbs.append(e)
-                else:
-                    provider_url = u"https://gist.github.com/{user}/{gist_id}#file-{clean_name}".format(
-                        user=user,
-                        gist_id=gist_id,
-                        clean_name=clean_filename(file['filename']),
-                    )
-                    e['url'] = provider_url
-                    e['class'] = 'fa-share'
-                    others.append(e)
-
-            entries.extend(ipynbs)
-            entries.extend(others)
-
-            # Enable a binder navbar icon if a binder base URL is configured
-            executor_url = self.BINDER_TMPL.format(
-                binder_base_url=self.binder_base_url,
-                user=user.rstrip('/'),
-                gist_id=gist_id
-            ) if self.binder_base_url else None
-
-            html = self.render_template(
-                'treelist.html',
-                entries=entries,
-                tree_type='gist',
-                tree_label='gists',
-                user=user.rstrip('/'),
-                provider_url=gist['html_url'],
-                executor_url=executor_url,
-                **self.PROVIDER_CTX
-            )
-            yield self.cache_and_finish(html)
+            content = file['content']
+
+        if many_files_gist and not filename.endswith('.ipynb'):
+            self.set_header('Content-Type', file.get('type') or 'text/plain')
+            # cannot redirect because of X-Frame-Content
+            self.finish(content)
+            return
+
+        else:
+            return content
+
+    # Only called by file_get
+    @gen.coroutine
+    def deliver_notebook(self, user, gist_id, filename, gist, file, content):
+        """
+        user, gist_id, filename, gist, file, are the same values as those 
+        passed into file_get, whereas content is returned from 
+        get_notebook_data using user, gist_id, filename, gist, and file.
+        """
+        # Enable a binder navbar icon if a binder base URL is configured
+        executor_url = self.BINDER_PATH_TMPL.format(
+            binder_base_url=self.binder_base_url,
+            user=user.rstrip('/'),
+            gist_id=gist_id,
+            path=quote(filename)
+        ) if self.binder_base_url else None
+
+        # provider_url: str, optional
+        #     URL to the notebook document upstream at the provider (e.g., GitHub)
+        yield self.finish_notebook(
+            content,
+            file['raw_url'],
+            msg="gist: %s" % gist_id,
+            public=gist['public'],
+            provider_url=gist['html_url'],
+            executor_url=executor_url,
+            **self.PROVIDER_CTX)
+
+    @cached
+    @gen.coroutine
+    def get(self, user, gist_id, filename=''):
+        """
+        Encompasses both the case of a single file gist, handled by
+        `file_get`, as well as a many-file gist, handled by `tree_get`.
+        """
+        user, gist_id, gist, files, many_files_gist = yield self.parse_gist(user, gist_id, filename)
+
+        if many_files_gist and not filename:
+            yield self.tree_get(user, gist_id, gist, files)
+
+        else:
+            if not many_files_gist and not filename:
+                filename = list(files.keys())[0]
+
+            if filename not in files:
+                raise web.HTTPError(404, "No such file in gist: %s (%s)", filename, list(files.keys()))
+
+            file = files[filename]
+
+            yield self.file_get(user, gist_id, filename, gist, many_files_gist, file)
 
 
 class GistRedirectHandler(BaseHandler):