Merge pull request #6900 from takluyver/contents-api-get-as-type

Contents API get as type

IPython/html/tree/tests/test_tree_handler.py

@@ -0,0 +1,32
+"""Test the /tree handlers"""
+import os
+import io
+from IPython.html.utils import url_path_join
+from IPython.nbformat import write
+from IPython.nbformat.v4 import new_notebook
+
+import requests
+
+from IPython.html.tests.launchnotebook import NotebookTestBase
+
+class TreeTest(NotebookTestBase):
+    def setUp(self):
+        nbdir = self.notebook_dir.name
+        d = os.path.join(nbdir, 'foo')
+        os.mkdir(d)
+
+        with io.open(os.path.join(d, 'bar.ipynb'), 'w', encoding='utf-8') as f:
+            nb = new_notebook()
+            write(nb, f, version=4)
+
+        with io.open(os.path.join(d, 'baz.txt'), 'w', encoding='utf-8') as f:
+            f.write(u'flamingo')
+
+        self.base_url()
+
+    def test_redirect(self):
+        r = requests.get(url_path_join(self.base_url(), 'tree/foo/bar.ipynb'))
+        self.assertEqual(r.url, self.base_url() + 'notebooks/foo/bar.ipynb')
+
+        r = requests.get(url_path_join(self.base_url(), 'tree/foo/baz.txt'))
+        self.assertEqual(r.url, url_path_join(self.base_url(), 'files/foo/baz.txt'))

IPython/html/files/handlers.py

@@ -28,7 +28,7 class FilesHandler(IPythonHandler):
         else:
             name = path
         
-        model = cm.get_model(path)
+        model = cm.get(path)
         
         if self.get_argument("download", False):
             self.set_header('Content-Disposition','attachment; filename="%s"' % name)

IPython/html/nbconvert/handlers.py

@@ -81,7 +81,7 class NbconvertFileHandler(IPythonHandler):
         exporter = get_exporter(format, config=self.config, log=self.log)
         
         path = path.strip('/')
-        model = self.contents_manager.get_model(path=path)
+        model = self.contents_manager.get(path=path)
         name = model['name']
 
         self.set_header('Last-Modified', model['last_modified'])

IPython/html/services/contents/filemanager.py

@@ -200,7 +200,7 class FileContentsManager(ContentsManager):
                     self.log.debug("%s not a regular file", os_path)
                     continue
                 if self.should_list(name) and not is_hidden(os_path, self.root_dir):
-                    contents.append(self.get_model(
+                    contents.append(self.get(
                         path='%s/%s' % (path, name),
                         content=False)
                     )
@@ -209,11 +209,15 class FileContentsManager(ContentsManager):
 
         return model
 
-    def _file_model(self, path, content=True):
+    def _file_model(self, path, content=True, format=None):
         """Build a model for a file
 
         if content is requested, include the file contents.
-        UTF-8 text files will be unicode, binary files will be base64-encoded.
+
+        format:
+          If 'text', the contents will be decoded as UTF-8.
+          If 'base64', the raw bytes contents will be encoded as base64.
+          If not specified, try to decode as UTF-8, and fall back to base64
         """
         model = self._base_model(path)
         model['type'] = 'file'
@@ -224,13 +228,20 class FileContentsManager(ContentsManager):
                 raise web.HTTPError(400, "Cannot get content of non-file %s" % os_path)
             with io.open(os_path, 'rb') as f:
                 bcontent = f.read()
-            try:
-                model['content'] = bcontent.decode('utf8')
-            except UnicodeError as e:
+
+            if format != 'base64':
+                try:
+                    model['content'] = bcontent.decode('utf8')
+                except UnicodeError as e:
+                    if format == 'text':
+                        raise web.HTTPError(400, "%s is not UTF-8 encoded" % path)
+                else:
+                    model['format'] = 'text'
+
+            if model['content'] is None:
                 model['content'] = base64.encodestring(bcontent).decode('ascii')
                 model['format'] = 'base64'
-            else:
-                model['format'] = 'text'
+
         return model
 
 
@@ -255,13 +266,21 class FileContentsManager(ContentsManager):
             self.validate_notebook_model(model)
         return model
 
-    def get_model(self, path, content=True):
+    def get(self, path, content=True, type_=None, format=None):
         """ Takes a path for an entity and returns its model
 
         Parameters
         ----------
         path : str
             the API path that describes the relative path for the target
+        content : bool
+            Whether to include the contents in the reply
+        type_ : str, optional
+            The requested type - 'file', 'notebook', or 'directory'.
+            Will raise HTTPError 400 if the content doesn't match.
+        format : str, optional
+            The requested format for file contents. 'text' or 'base64'.
+            Ignored if this returns a notebook or directory model.
 
         Returns
         -------
@@ -276,11 +295,17 class FileContentsManager(ContentsManager):
 
         os_path = self._get_os_path(path)
         if os.path.isdir(os_path):
+            if type_ not in (None, 'directory'):
+                raise web.HTTPError(400,
+                                u'%s is a directory, not a %s' % (path, type_))
             model = self._dir_model(path, content=content)
-        elif path.endswith('.ipynb'):
+        elif type_ == 'notebook' or (type_ is None and path.endswith('.ipynb')):
             model = self._notebook_model(path, content=content)
         else:
-            model = self._file_model(path, content=content)
+            if type_ == 'directory':
+                raise web.HTTPError(400,
+                                u'%s is not a directory')
+            model = self._file_model(path, content=content, format=format)
         return model
 
     def _save_notebook(self, os_path, model, path=''):
@@ -355,7 +380,7 class FileContentsManager(ContentsManager):
             self.validate_notebook_model(model)
             validation_message = model.get('message', None)
 
-        model = self.get_model(path, content=False)
+        model = self.get(path, content=False)
         if validation_message:
             model['message'] = validation_message
         return model
@@ -370,7 +395,7 class FileContentsManager(ContentsManager):
         new_path = model.get('path', path).strip('/')
         if path != new_path:
             self.rename(path, new_path)
-        model = self.get_model(new_path, content=False)
+        model = self.get(new_path, content=False)
         return model
 
     def delete(self, path):

IPython/html/services/contents/handlers.py

@@ -58,7 +58,15 class ContentsHandler(IPythonHandler):
         of the files and directories it contains.
         """
         path = path or ''
-        model = self.contents_manager.get_model(path=path)
+        type_ = self.get_query_argument('type', default=None)
+        if type_ not in {None, 'directory', 'file', 'notebook'}:
+            raise web.HTTPError(400, u'Type %r is invalid' % type_)
+
+        format = self.get_query_argument('format', default=None)#
+        if format not in {None, 'text', 'base64'}:
+            raise web.HTTPError(400, u'Format %r is invalid' % format)
+
+        model = self.contents_manager.get(path=path, type_=type_, format=format)
         if model['type'] == 'directory':
             # group listing by type, then by name (case-insensitive)
             # FIXME: sorting should be done in the frontends

IPython/html/services/contents/manager.py

@@ -135,7 +135,7 class ContentsManager(LoggingConfigurable):
         """
         return self.file_exists(path) or self.dir_exists(path)
 
-    def get_model(self, path, content=True):
+    def get(self, path, content=True, type_=None, format=None):
         """Get the model of a file or directory with or without content."""
         raise NotImplementedError('must be implemented in a subclass')
 
@@ -300,7 +300,7 class ContentsManager(LoggingConfigurable):
             from_dir = ''
             from_name = path
         
-        model = self.get_model(path)
+        model = self.get(path)
         model.pop('path', None)
         model.pop('name', None)
         if model['type'] == 'directory':
@@ -328,7 +328,7 class ContentsManager(LoggingConfigurable):
         path : string
             The path of a notebook
         """
-        model = self.get_model(path)
+        model = self.get(path)
         nb = model['content']
         self.log.warn("Trusting notebook %s", path)
         self.notary.mark_cells(nb, True)

IPython/html/services/contents/tests/test_contents_api.py

@@ -35,10 +35,10 class API(object):
     def __init__(self, base_url):
         self.base_url = base_url
 
-    def _req(self, verb, path, body=None):
+    def _req(self, verb, path, body=None, params=None):
         response = requests.request(verb,
                 url_path_join(self.base_url, 'api/contents', path),
-                data=body,
+                data=body, params=params,
         )
         response.raise_for_status()
         return response
@@ -46,8 +46,13 class API(object):
     def list(self, path='/'):
         return self._req('GET', path)
 
-    def read(self, path):
-        return self._req('GET', path)
+    def read(self, path, type_=None, format=None):
+        params = {}
+        if type_ is not None:
+            params['type'] = type_
+        if format is not None:
+            params['format'] = format
+        return self._req('GET', path, params=params)
 
     def create_untitled(self, path='/', ext='.ipynb'):
         body = None
@@ -243,6 +248,10 class APITest(NotebookTestBase):
         with assert_http_error(404):
             self.api.read('foo/q.txt')
 
+        # Specifying format=text should fail on a non-UTF-8 file
+        with assert_http_error(400):
+            self.api.read('foo/bar/baz.blob', type_='file', format='text')
+
     def test_get_binary_file_contents(self):
         for d, name in self.dirs_nbs:
             path = url_path_join(d, name + '.blob')
@@ -259,6 +268,13 class APITest(NotebookTestBase):
         with assert_http_error(404):
             self.api.read('foo/q.txt')
 
+    def test_get_bad_type(self):
+        with assert_http_error(400):
+            self.api.read(u'unicodé', type_='file')  # this is a directory
+
+        with assert_http_error(400):
+            self.api.read(u'unicodé/innonascii.ipynb', type_='directory')
+
     def _check_created(self, resp, path, type='notebook'):
         self.assertEqual(resp.status_code, 201)
         location_header = py3compat.str_to_unicode(resp.headers['Location'])

IPython/html/services/contents/tests/test_manager.py

@@ -105,7 +105,7 class TestContentsManager(TestCase):
         name = model['name']
         path = model['path']
 
-        full_model = cm.get_model(path)
+        full_model = cm.get(path)
         nb = full_model['content']
         self.add_code_cell(nb)
 
@@ -152,24 +152,41 class TestContentsManager(TestCase):
         path = model['path']
 
         # Check that we 'get' on the notebook we just created
-        model2 = cm.get_model(path)
+        model2 = cm.get(path)
         assert isinstance(model2, dict)
         self.assertIn('name', model2)
         self.assertIn('path', model2)
         self.assertEqual(model['name'], name)
         self.assertEqual(model['path'], path)
 
+        nb_as_file = cm.get(path, content=True, type_='file')
+        self.assertEqual(nb_as_file['path'], path)
+        self.assertEqual(nb_as_file['type'], 'file')
+        self.assertEqual(nb_as_file['format'], 'text')
+        self.assertNotIsInstance(nb_as_file['content'], dict)
+
+        nb_as_bin_file = cm.get(path, content=True, type_='file', format='base64')
+        self.assertEqual(nb_as_bin_file['format'], 'base64')
+
         # Test in sub-directory
         sub_dir = '/foo/'
         self.make_dir(cm.root_dir, 'foo')
         model = cm.new_untitled(path=sub_dir, ext='.ipynb')
-        model2 = cm.get_model(sub_dir + name)
+        model2 = cm.get(sub_dir + name)
         assert isinstance(model2, dict)
         self.assertIn('name', model2)
         self.assertIn('path', model2)
         self.assertIn('content', model2)
         self.assertEqual(model2['name'], 'Untitled0.ipynb')
         self.assertEqual(model2['path'], '{0}/{1}'.format(sub_dir.strip('/'), name))
+
+        # Test getting directory model
+        dirmodel = cm.get('foo')
+        self.assertEqual(dirmodel['type'], 'directory')
+
+        with self.assertRaises(HTTPError):
+            cm.get('foo', type_='file')
+
     
     @dec.skip_win32
     def test_bad_symlink(self):
@@ -181,7 +198,7 class TestContentsManager(TestCase):
         
         # create a broken symlink
         os.symlink("target", os.path.join(os_path, "bad symlink"))
-        model = cm.get_model(path)
+        model = cm.get(path)
         self.assertEqual(model['content'], [file_model])
     
     @dec.skip_win32
@@ -196,8 +213,8 class TestContentsManager(TestCase):
         
         # create a good symlink
         os.symlink(file_model['name'], os.path.join(os_path, name))
-        symlink_model = cm.get_model(path, content=False)
-        dir_model = cm.get_model(parent)
+        symlink_model = cm.get(path, content=False)
+        dir_model = cm.get(parent)
         self.assertEqual(
             sorted(dir_model['content'], key=lambda x: x['name']),
             [symlink_model, file_model],
@@ -219,7 +236,7 class TestContentsManager(TestCase):
         self.assertEqual(model['name'], 'test.ipynb')
 
         # Make sure the old name is gone
-        self.assertRaises(HTTPError, cm.get_model, path)
+        self.assertRaises(HTTPError, cm.get, path)
 
         # Test in sub-directory
         # Create a directory and notebook in that directory
@@ -240,7 +257,7 class TestContentsManager(TestCase):
         self.assertEqual(model['path'], new_path)
 
         # Make sure the old name is gone
-        self.assertRaises(HTTPError, cm.get_model, path)
+        self.assertRaises(HTTPError, cm.get, path)
 
     def test_save(self):
         cm = self.contents_manager
@@ -250,7 +267,7 class TestContentsManager(TestCase):
         path = model['path']
 
         # Get the model with 'content'
-        full_model = cm.get_model(path)
+        full_model = cm.get(path)
 
         # Save the notebook
         model = cm.save(full_model, path)
@@ -267,7 +284,7 class TestContentsManager(TestCase):
         model = cm.new_untitled(path=sub_dir, type='notebook')
         name = model['name']
         path = model['path']
-        model = cm.get_model(path)
+        model = cm.get(path)
 
         # Change the name in the model for rename
         model = cm.save(model, path)
@@ -286,7 +303,7 class TestContentsManager(TestCase):
         cm.delete(path)
 
         # Check that a 'get' on the deleted notebook raises and error
-        self.assertRaises(HTTPError, cm.get_model, path)
+        self.assertRaises(HTTPError, cm.get, path)
 
     def test_copy(self):
         cm = self.contents_manager
@@ -309,12 +326,12 class TestContentsManager(TestCase):
         cm = self.contents_manager
         nb, name, path = self.new_notebook()
 
-        untrusted = cm.get_model(path)['content']
+        untrusted = cm.get(path)['content']
         assert not cm.notary.check_cells(untrusted)
 
         # print(untrusted)
         cm.trust_notebook(path)
-        trusted = cm.get_model(path)['content']
+        trusted = cm.get(path)['content']
         # print(trusted)
         assert cm.notary.check_cells(trusted)
 
@@ -328,7 +345,7 class TestContentsManager(TestCase):
                 assert not cell.metadata.trusted
 
         cm.trust_notebook(path)
-        nb = cm.get_model(path)['content']
+        nb = cm.get(path)['content']
         for cell in nb.cells:
             if cell.cell_type == 'code':
                 assert cell.metadata.trusted
@@ -342,7 +359,7 class TestContentsManager(TestCase):
         assert not cm.notary.check_signature(nb)
 
         cm.trust_notebook(path)
-        nb = cm.get_model(path)['content']
+        nb = cm.get(path)['content']
         cm.mark_trusted_cells(nb, path)
         cm.check_and_sign(nb, path)
         assert cm.notary.check_signature(nb)

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

@@ -110,11 +110,8 define([
                 });
         });
         this.element.find('#open_notebook').click(function () {
-            window.open(utils.url_join_encode(
-                that.notebook.base_url,
-                'tree',
-                that.notebook.notebook_path
-            ));
+            var parent = utils.url_path_split(that.notebook.notebook_path)[0];
+            window.open(utils.url_join_encode(that.base_url, 'tree', parent));
         });
         this.element.find('#copy_notebook').click(function () {
             that.notebook.copy_notebook();

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

@@ -2104,6 +2104,7 define([
         this.notebook_name = utils.url_path_split(this.notebook_path)[1];
         this.events.trigger('notebook_loading.Notebook');
         this.contents.get(notebook_path, {
+            type: 'notebook',
             success: $.proxy(this.load_notebook_success, this),
             error: $.proxy(this.load_notebook_error, this)
         });

IPython/html/static/services/contents.js

@@ -87,7 +87,10 define([
             error : this.create_basic_error_handler(options.error)
         };
         var url = this.api_url(path);
-        $.ajax(url, settings);
+        params = {};
+        if (options.type) { params.type = options.type; }
+        if (options.format) { params.format = options.format; }
+        $.ajax(url + '?' + $.param(params), settings);
     };
 
 
@@ -241,20 +244,11 define([
      *     last_modified: last modified dat
      * @method list_notebooks
      * @param {String} path The path to list notebooks in
-     * @param {Function} load_callback called with list of notebooks on success
-     * @param {Function} error called with ajax results on error
+     * @param {Object} options including success and error callbacks
      */
     Contents.prototype.list_contents = function(path, options) {
-        var settings = {
-            processData : false,
-            cache : false,
-            type : "GET",
-            dataType : "json",
-            success : options.success,
-            error : this.create_basic_error_handler(options.error)
-        };
-
-        $.ajax(this.api_url(path), settings);
+        options.type = 'directory';
+        this.get(path, options);
     };