diff --git a/IPython/html/notebook/handlers.py b/IPython/html/notebook/handlers.py index 49b1ea3f5..f6e101209 100644 --- a/IPython/html/notebook/handlers.py +++ b/IPython/html/notebook/handlers.py @@ -17,7 +17,7 @@ from ..utils import url_escape class NotebookHandler(IPythonHandler): @web.authenticated - def get(self, path=''): + def get(self, path): """get renders the notebook template if a name is given, or redirects to the '/files/' handler if the name is not given.""" path = path.strip('/') diff --git a/IPython/html/notebookapp.py b/IPython/html/notebookapp.py index 1c4718401..0086ce7c3 100644 --- a/IPython/html/notebookapp.py +++ b/IPython/html/notebookapp.py @@ -187,7 +187,9 @@ class NotebookWebApplication(web.Application): return settings def init_handlers(self, settings): - # Load the (URL pattern, handler) tuples for each component. + """Load the (URL pattern, handler) tuples for each component.""" + + # Order matters. The first handler to match the URL will handle the request. handlers = [] handlers.extend(load_handlers('tree.handlers')) handlers.extend(load_handlers('auth.login')) @@ -205,6 +207,7 @@ class NotebookWebApplication(web.Application): handlers.append( (r"/nbextensions/(.*)", FileFindHandler, {'path' : settings['nbextensions_path']}), ) + # register base handlers last handlers.extend(load_handlers('base.handlers')) # set the URL that will be redirected from `/` handlers.append( diff --git a/IPython/html/services/contents/filemanager.py b/IPython/html/services/contents/filemanager.py index d2e2b7046..e99482908 100644 --- a/IPython/html/services/contents/filemanager.py +++ b/IPython/html/services/contents/filemanager.py @@ -61,9 +61,8 @@ class FileContentsManager(ContentsManager): except OSError as e: self.log.debug("copystat on %s failed", dest, exc_info=True) - def _get_os_path(self, path=''): - """Given a filename and API path, return its file system - path. + def _get_os_path(self, path): + """Given an API path, return its file system path. Parameters ---------- @@ -131,8 +130,8 @@ class FileContentsManager(ContentsManager): Whether the file exists. """ path = path.strip('/') - nbpath = self._get_os_path(path) - return os.path.isfile(nbpath) + os_path = self._get_os_path(path) + return os.path.isfile(os_path) def exists(self, path): """Returns True if the path exists, else returns False. @@ -167,7 +166,6 @@ class FileContentsManager(ContentsManager): model['created'] = created model['content'] = None model['format'] = None - model['message'] = None return model def _dir_model(self, path, content=True): @@ -191,12 +189,16 @@ class FileContentsManager(ContentsManager): model['type'] = 'directory' if content: model['content'] = contents = [] - for os_path in glob.glob(self._get_os_path('%s/*' % path)): - name = os.path.basename(os_path) + os_dir = self._get_os_path(path) + for name in os.listdir(os_dir): + os_path = os.path.join(os_dir, name) # skip over broken symlinks in listing if not os.path.exists(os_path): self.log.warn("%s doesn't exist", os_path) continue + elif not os.path.isfile(os_path) and not os.path.isdir(os_path): + 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( path='%s/%s' % (path, name), @@ -217,6 +219,9 @@ class FileContentsManager(ContentsManager): model['type'] = 'file' if content: os_path = self._get_os_path(path) + if not os.path.isfile(os_path): + # could be FIFO + 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: @@ -407,14 +412,14 @@ class FileContentsManager(ContentsManager): old_os_path = self._get_os_path(old_path) # Should we proceed with the move? - if os.path.isfile(new_os_path): - raise web.HTTPError(409, u'File already exists: %s' % new_os_path) + if os.path.exists(new_os_path): + raise web.HTTPError(409, u'File already exists: %s' % new_path) # Move the file try: shutil.move(old_os_path, new_os_path) except Exception as e: - raise web.HTTPError(500, u'Unknown error renaming file: %s %s' % (old_os_path, e)) + raise web.HTTPError(500, u'Unknown error renaming file: %s %s' % (old_path, e)) # Move the checkpoints old_checkpoints = self.list_checkpoints(old_path) @@ -462,6 +467,8 @@ class FileContentsManager(ContentsManager): def create_checkpoint(self, path): """Create a checkpoint from the current state of a file""" path = path.strip('/') + if not self.file_exists(path): + raise web.HTTPError(404) src_path = self._get_os_path(path) # only the one checkpoint ID: checkpoint_id = u"checkpoint" @@ -521,7 +528,7 @@ class FileContentsManager(ContentsManager): def get_kernel_path(self, path, model=None): """Return the initial working dir a kernel associated with a given notebook""" if '/' in path: - os_dir = path.rsplit('/', 1)[0] + parent_dir = path.rsplit('/', 1)[0] else: - os_dir = '' - return self._get_os_path(os_dir) + parent_dir = '' + return self._get_os_path(parent_dir) diff --git a/IPython/html/services/contents/handlers.py b/IPython/html/services/contents/handlers.py index 0ffb2f2d3..c724d7ad4 100644 --- a/IPython/html/services/contents/handlers.py +++ b/IPython/html/services/contents/handlers.py @@ -73,14 +73,11 @@ class ContentsHandler(IPythonHandler): model = self.get_json_body() if model is None: raise web.HTTPError(400, u'JSON body missing') - print('before', model) model = cm.update(model, path) - print('after', model) self._finish_model(model) def _copy(self, copy_from, copy_to=None): - """Copy a file, optionally specifying the new path. - """ + """Copy a file, optionally specifying a target directory.""" self.log.info(u"Copying {copy_from} to {copy_to}".format( copy_from=copy_from, copy_to=copy_to or '', @@ -96,13 +93,17 @@ class ContentsHandler(IPythonHandler): self.set_status(201) self._finish_model(model) - def _create_empty_file(self, path, ext='.ipynb'): - """Create an empty file in path - - If name specified, create it in path. + def _new(self, path, type='notebook', ext=''): + """Create an empty file or directory in directory specified by path + + ContentsManager picks the filename. """ - self.log.info(u"Creating new file in %s", path) - model = self.contents_manager.new(path=path, ext=ext) + self.log.info(u"Creating new %s in %s", type or 'file', path) + if type: + model = {'type': type} + else: + model = None + model = self.contents_manager.new(model, path=path, ext=ext) self.set_status(201) self._finish_model(model) @@ -115,9 +116,9 @@ class ContentsHandler(IPythonHandler): @web.authenticated @json_errors def post(self, path=''): - """Create a new file or directory in the specified path. + """Create a new file in the specified path. - POST creates new files or directories. The server always decides on the name. + POST creates new files. The server always decides on the name. POST /api/contents/path New untitled, empty file or directory. @@ -129,7 +130,7 @@ class ContentsHandler(IPythonHandler): cm = self.contents_manager if cm.file_exists(path): - raise web.HTTPError(400, "Cannot POST to existing files, use PUT instead.") + raise web.HTTPError(400, "Cannot POST to files, use PUT instead.") if not cm.dir_exists(path): raise web.HTTPError(404, "No such directory: %s" % path) @@ -138,13 +139,14 @@ class ContentsHandler(IPythonHandler): if model is not None: copy_from = model.get('copy_from') - ext = model.get('ext', '.ipynb') + ext = model.get('ext', '') + type = model.get('type') if copy_from: self._copy(copy_from, path) else: - self._create_empty_file(path, ext=ext) + self._new(path, type=type, ext=ext) else: - self._create_empty_file(path) + self._new(path) @web.authenticated @json_errors @@ -168,7 +170,7 @@ class ContentsHandler(IPythonHandler): else: self._upload(model, path) else: - self._create_empty_file(path) + self._new(path) @web.authenticated @json_errors diff --git a/IPython/html/services/contents/manager.py b/IPython/html/services/contents/manager.py index 786062b85..c4f7b96b4 100644 --- a/IPython/html/services/contents/manager.py +++ b/IPython/html/services/contents/manager.py @@ -119,7 +119,7 @@ class ContentsManager(LoggingConfigurable): raise NotImplementedError('must be implemented in a subclass') def exists(self, path): - """Does a file or directory exist at the given name and path? + """Does a file or directory exist at the given path? Like os.path.exists @@ -181,7 +181,11 @@ class ContentsManager(LoggingConfigurable): return "Serving contents" def get_kernel_path(self, path, model=None): - """ Return the path to start kernel in """ + """Return the API path for the kernel + + KernelManagers can turn this value into a filesystem path, + or ignore it altogether. + """ return path def increment_filename(self, filename, path=''): @@ -204,7 +208,7 @@ class ContentsManager(LoggingConfigurable): for i in itertools.count(): name = u'{basename}{i}{ext}'.format(basename=basename, i=i, ext=ext) - if not self.file_exists(u'{}/{}'.format(path, name)): + if not self.exists(u'{}/{}'.format(path, name)): break return name @@ -218,22 +222,35 @@ class ContentsManager(LoggingConfigurable): ) return model - def new(self, model=None, path='', ext='.ipynb'): - """Create a new file or directory and return its model with no content.""" + def new(self, model=None, path='', ext=''): + """Create a new file or directory and return its model with no content. + + If path is a directory, a new untitled file/directory is created in path. + Otherwise, a new file/directory is created exactly at path. + """ path = path.strip('/') if model is None: model = {} else: model.pop('path', None) - if 'content' not in model and model.get('type', None) != 'directory': - if ext == '.ipynb': + + if ext and ext != '.ipynb': + model.setdefault('type', 'file') + else: + model.setdefault('type', 'notebook') + + # no content, not a directory, so fill out new-file model + if 'content' not in model and model['type'] != 'directory': + if model['type'] == 'notebook': + ext = '.ipynb' model['content'] = new_notebook() - model['type'] = 'notebook' model['format'] = 'json' else: model['content'] = '' model['type'] = 'file' model['format'] = 'text' + + # if path is a directory, create an untitled file or directory if self.dir_exists(path): if model['type'] == 'directory': untitled = self.untitled_directory @@ -252,10 +269,10 @@ class ContentsManager(LoggingConfigurable): def copy(self, from_path, to_path=None): """Copy an existing file and return its new model. - If to_name not specified, increment `from_name-Copy#.ext`. + If to_path not specified, it will be the parent directory of from_path. + If to_path is a directory, filename will increment `from_path-Copy#.ext`. - copy_from can be a full path to a file, - or just a base name. If a base name, `path` is used. + from_path must be a full path to a file. """ path = from_path.strip('/') if '/' in path: @@ -325,7 +342,7 @@ class ContentsManager(LoggingConfigurable): nb : dict The notebook object (in current nbformat) path : string - The notebook's directory (for logging) + The notebook's path (for logging) """ trusted = self.notary.check_signature(nb) if not trusted: diff --git a/IPython/html/services/contents/tests/test_contents_api.py b/IPython/html/services/contents/tests/test_contents_api.py index 698f471eb..a55895b59 100644 --- a/IPython/html/services/contents/tests/test_contents_api.py +++ b/IPython/html/services/contents/tests/test_contents_api.py @@ -55,6 +55,9 @@ class API(object): body = json.dumps({'ext': ext}) return self._req('POST', path, body) + def mkdir_untitled(self, path='/'): + return self._req('POST', path, json.dumps({'type': 'directory'})) + def copy(self, copy_from, path='/'): body = json.dumps({'copy_from':copy_from}) return self._req('POST', path, body) @@ -65,6 +68,9 @@ class API(object): def upload(self, path, body): return self._req('PUT', path, body) + def mkdir_untitled(self, path='/'): + return self._req('POST', path, json.dumps({'type': 'directory'})) + def mkdir(self, path='/'): return self._req('PUT', path, json.dumps({'type': 'directory'})) @@ -193,8 +199,11 @@ class APITest(NotebookTestBase): self.assertEqual(nbnames, expected) def test_list_dirs(self): + print(self.api.list().json()) dirs = dirs_only(self.api.list().json()) dir_names = {normalize('NFC', d['name']) for d in dirs} + print(dir_names) + print(self.top_level_dirs) self.assertEqual(dir_names, self.top_level_dirs) # Excluding hidden dirs def test_list_nonexistant_dir(self): @@ -293,6 +302,18 @@ class APITest(NotebookTestBase): resp = self.api.upload(path, body=json.dumps(nbmodel)) self._check_created(resp, path) + def test_mkdir_untitled(self): + resp = self.api.mkdir_untitled(path=u'å b') + self._check_created(resp, u'å b/Untitled Folder0', type='directory') + + # Second time + resp = self.api.mkdir_untitled(path=u'å b') + self._check_created(resp, u'å b/Untitled Folder1', type='directory') + + # And two directories down + resp = self.api.mkdir_untitled(path='foo/bar') + self._check_created(resp, 'foo/bar/Untitled Folder0', type='directory') + def test_mkdir(self): path = u'å b/New ∂ir' resp = self.api.mkdir(path) diff --git a/IPython/html/static/notebook/js/menubar.js b/IPython/html/static/notebook/js/menubar.js index 3c367a5a9..b96f18f33 100644 --- a/IPython/html/static/notebook/js/menubar.js +++ b/IPython/html/static/notebook/js/menubar.js @@ -90,9 +90,9 @@ define([ this.element.find('#new_notebook').click(function () { // Create a new notebook in the same path as the current // notebook's path. - var parent = utils.url_path_split(this.notebook_path)[0]; + var parent = utils.url_path_split(that.notebook.notebook_path)[0]; that.contents.new(parent, { - ext: ".ipynb", + type: "notebook", extra_settings: {async: false}, // So we can open a new window afterwards success: function (data) { window.open( diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index 041a7af6b..97e0839a4 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -98,9 +98,13 @@ define([ * @param {String} path The path to create the new notebook at * @param {Object} options: * ext: file extension to use + * type: model type to create ('notebook', 'file', or 'directory') */ Contents.prototype.new = function(path, options) { - var data = JSON.stringify({ext: options.ext || ".ipynb"}); + var data = JSON.stringify({ + ext: options.ext, + type: options.type + }); var settings = { processData : false, diff --git a/IPython/html/static/tree/js/main.js b/IPython/html/static/tree/js/main.js index 8d849172a..ea820d492 100644 --- a/IPython/html/static/tree/js/main.js +++ b/IPython/html/static/tree/js/main.js @@ -65,7 +65,7 @@ require([ $('#new_notebook').click(function (e) { contents.new(common_options.notebook_path, { - ext: ".ipynb", + type: "notebook", extra_settings: {async: false}, // So we can open a new window afterwards success: function (data) { window.open( diff --git a/IPython/html/tree/handlers.py b/IPython/html/tree/handlers.py index 894a173e8..9bd3d95bd 100644 --- a/IPython/html/tree/handlers.py +++ b/IPython/html/tree/handlers.py @@ -37,9 +37,12 @@ class TreeHandler(IPythonHandler): path = path.strip('/') cm = self.contents_manager if cm.file_exists(path): - # is a notebook, redirect to notebook handler + # it's not a directory, we have redirecting to do + model = cm.get(path, content=False) + # redirect to /api/notebooks if it's a notebook, otherwise /api/files + service = 'notebooks' if model['type'] == 'notebook' else 'files' url = url_escape(url_path_join( - self.base_url, 'notebooks', path, + self.base_url, service, path, )) self.log.debug("Redirecting %s to %s", self.request.path, url) self.redirect(url)