Merge pull request #7278 from minrk/nb-file-ext

don't enforce .ipynb extension in URLs

IPython/html/base/handlers.py

@@ -464,7 +464,13 @@ def get(self):
 
 class FilesRedirectHandler(IPythonHandler):
     """Handler for redirecting relative URLs to the /files/ handler"""
-    def get(self, path=''):
+
+    @staticmethod
+    def redirect_to_files(self, path):
+        """make redirect logic a reusable static method
+        
+        so it can be called from other handlers.
+        """
         cm = self.contents_manager
         if cm.dir_exists(path):
             # it's a *directory*, redirect to /tree
@@ -488,6 +494,9 @@ def get(self, path=''):
         url = url_escape(url)
         self.log.debug("Redirecting %s to %s", self.request.path, url)
         self.redirect(url)
+
+    def get(self, path=''):
+        return self.redirect_to_files(self, path)
 
 
 #-----------------------------------------------------------------------------
@@ -496,7 +505,6 @@ def get(self, path=''):
 
 # path matches any number of `/foo[/bar...]` or just `/` or ''
 path_regex = r"(?P<path>(?:(?:/[^/]+)+|/?))"
-notebook_path_regex = r"(?P<path>(?:/[^/]+)+\.ipynb)"
 
 #-----------------------------------------------------------------------------
 # URL to handler mappings

IPython/html/nbconvert/handlers.py

@@ -11,7 +11,7 @@
 
 from ..base.handlers import (
     IPythonHandler, FilesRedirectHandler,
-    notebook_path_regex, path_regex,
+    path_regex,
 )
 from IPython.nbformat import from_dict
 
@@ -83,6 +83,8 @@ def get(self, format, path):
         path = path.strip('/')
         model = self.contents_manager.get(path=path)
         name = model['name']
+        if model['type'] != 'notebook':
+            raise web.HTTPError(400, "Not a notebook: %s" % path)
 
         self.set_header('Last-Modified', model['last_modified'])
 
@@ -142,8 +144,8 @@ def post(self, format):
 
 
 default_handlers = [
-    (r"/nbconvert/%s%s" % (_format_regex, notebook_path_regex),
-         NbconvertFileHandler),
     (r"/nbconvert/%s" % _format_regex, NbconvertPostHandler),
+    (r"/nbconvert/%s%s" % (_format_regex, path_regex),
+         NbconvertFileHandler),
     (r"/nbconvert/html%s" % path_regex, FilesRedirectHandler),
 ]

IPython/html/notebook/handlers.py

@@ -8,8 +8,7 @@
 HTTPError = web.HTTPError
 
 from ..base.handlers import (
-    IPythonHandler, FilesRedirectHandler,
-    notebook_path_regex, path_regex,
+    IPythonHandler, FilesRedirectHandler, path_regex,
 )
 from ..utils import url_escape
 
@@ -23,9 +22,18 @@ def get(self, path):
         path = path.strip('/')
         cm = self.contents_manager
 
-        # a .ipynb filename was given
-        if not cm.file_exists(path):
-            raise web.HTTPError(404, u'Notebook does not exist: %s' % path)
+        # will raise 404 on not found
+        try:
+            model = cm.get(path, content=False)
+        except web.HTTPError as e:
+            if e.status_code == 404 and 'files' in path.split('/'):
+                # 404, but '/files/' in URL, let FilesRedirect take care of it
+                return FilesRedirectHandler.redirect_to_files(self, path)
+            else:
+                raise
+        if model['type'] != 'notebook':
+            # not a notebook, redirect to files
+            return FilesRedirectHandler.redirect_to_files(self, path)
         name = url_escape(path.rsplit('/', 1)[-1])
         path = url_escape(path)
         self.write(self.render_template('notebook.html',
@@ -43,7 +51,6 @@ def get(self, path):
 
 
 default_handlers = [
-    (r"/notebooks%s" % notebook_path_regex, NotebookHandler),
-    (r"/notebooks%s" % path_regex, FilesRedirectHandler),
+    (r"/notebooks%s" % path_regex, NotebookHandler),
 ]
 

IPython/html/static/notebook/js/notebook.js

@@ -2031,16 +2031,25 @@ define([
             }
         );
     };
+
+    /**
+     * Ensure a filename has the right extension
+     * Returns the filename with the appropriate extension, appending if necessary.
+     */
+    Notebook.prototype.ensure_extension = function (name) {
+        if (!name.match(/\.ipynb$/)) {
+            name = name + ".ipynb";
+        }
+        return name;
+    };
 
     /**
      * Rename the notebook.
      * @param  {string} new_name
      * @return {Promise} promise that resolves when the notebook is renamed.
      */
     Notebook.prototype.rename = function (new_name) {
-        if (!new_name.match(/\.ipynb$/)) {
-            new_name = new_name + ".ipynb";
-        }
+        new_name = this.ensure_extension(new_name);
 
         var that = this;
         var parent = utils.url_path_split(this.notebook_path)[0];

IPython/html/tests/test_paths.py

@@ -2,7 +2,7 @@
 import re
 import nose.tools as nt
 
-from IPython.html.base.handlers import path_regex, notebook_path_regex
+from IPython.html.base.handlers import path_regex
 
 try: # py3
     assert_regex = nt.assert_regex
@@ -14,7 +14,6 @@
 
 # build regexps that tornado uses:
 path_pat = re.compile('^' + '/x%s' % path_regex + '$')
-nb_path_pat = re.compile('^' + '/y%s' % notebook_path_regex + '$')
 
 def test_path_regex():
     for path in (
@@ -39,23 +38,3 @@ def test_path_regex_bad():
         '/y/x/foo',
     ):
         assert_not_regex(path, path_pat)
-
-def test_notebook_path_regex():
-    for path in (
-        '/y/asdf.ipynb',
-        '/y/foo/bar.ipynb',
-        '/y/a/b/c/d/e.ipynb',
-    ):
-        assert_regex(path, nb_path_pat)
-
-def test_notebook_path_regex_bad():
-    for path in (
-        '/y',
-        '/y/',
-        '/y/.ipynb',
-        '/y/foo/.ipynb',
-        '/y/foo/bar',
-        '/yfoo.ipynb',
-        '/yfoo/bar.ipynb',
-    ):
-        assert_not_regex(path, nb_path_pat)