handle path separators with os.sep and add tests

Added more tests to the notebook manager to check for the correct
path separators on different operating system. Fixed the get_path
method.

IPython/html/services/notebooks/filenbmanager.py

@@ -97,17 +97,17 b' class FileNotebookManager(NotebookManager):'
         changes = data.keys()
         response = 200
         for change in changes:
-            full_path = self.get_path(notebook_name, notebook_path)
+            full_path = self.get_os_path(notebook_name, notebook_path)
             if change == "name":
-                new_path = self.get_path(data['name'], notebook_path)
+                new_path = self.get_os_path(data['name'], notebook_path)
                 if not os.path.isfile(new_path):
                     os.rename(full_path,
-                        self.get_path(data['name'], notebook_path))
+                        self.get_os_path(data['name'], notebook_path))
                     notebook_name = data['name']
                 else:
                     response = 409
             if change == "path":
-                new_path = self.get_path(data['name'], data['path'])
+                new_path = self.get_os_path(data['name'], data['path'])
                 stutil.move(full_path, new_path)
                 notebook_path = data['path']
             if change == "content":
@@ -115,21 +115,45 b' class FileNotebookManager(NotebookManager):'
         model = self.notebook_model(notebook_name, notebook_path)
         return model, response
 
-    def notebook_exists(self, notebook_path):
-        """Does a notebook exist?"""
-        return os.path.isfile(notebook_path)
-
-    def get_path(self, notebook_name, notebook_path=None):
-        """Return a full path to a notebook given its notebook_name."""
-        return self.get_path_by_name(notebook_name, notebook_path)
+    def notebook_exists(self, name, path):
+        """Returns a True if the notebook exists. Else, returns False.
+        
+        Parameters
+        ----------
+        name : string
+            The name of the notebook you are checking.
+        path : string
+            The relative path to the notebook (with '/' as separator)
+        
+        Returns
+        -------
+        bool
+        """
+        path = self.get_os_path(name, path)
+        return os.path.isfile(path)
 
-    def get_path_by_name(self, name, notebook_path=None):
-        """Return a full path to a notebook given its name."""
-        filename = name #+ self.filename_ext
-        if notebook_path == None:
-            path = os.path.join(self.notebook_dir, filename)
-        else:    
-            path = os.path.join(self.notebook_dir, notebook_path, filename)
+    def get_os_path(self, fname, path=None):
+        """Return a full path to a notebook with the os.sep as the separator.
+        
+        Parameters
+        ----------
+        fname : string
+            The name of a notebook file with the .ipynb extension
+        path : string
+            The relative path (with '/' as separator) to the named notebook.
+            
+        Returns
+        -------
+        path : 
+            A path that combines notebook_dir (location where server started),
+            the relative path, and the filename with the current operating
+            system's url.
+        """
+        if path is None:
+            path = '/'
+        parts = path.split('/')
+        parts = [p for p in parts if p != ''] # remove duplicate splits
+        path = os.sep.join([self.notebook_dir] + parts + [fname])
         return path
 
     def read_notebook_object_from_path(self, path):
@@ -148,7 +172,7 b' class FileNotebookManager(NotebookManager):'
     
     def read_notebook_object(self, notebook_name, notebook_path=None):
         """Get the Notebook representation of a notebook by notebook_name."""
-        path = self.get_path(notebook_name, notebook_path)
+        path = self.get_os_path(notebook_name, notebook_path)
         if not os.path.isfile(path):
             raise web.HTTPError(404, u'Notebook does not exist: %s' % notebook_name)
         last_modified, nb = self.read_notebook_object_from_path(path)
@@ -172,7 +196,7 b' class FileNotebookManager(NotebookManager):'
         old_name = notebook_name
         old_checkpoints = self.list_checkpoints(old_name)
         
-        path = self.get_path_by_name(new_name, new_path)
+        path = self.get_os_path(new_name, new_path)
         
         # Right before we save the notebook, we write an empty string as the
         # notebook name in the metadata. This is to prepare for removing
@@ -201,7 +225,7 b' class FileNotebookManager(NotebookManager):'
             # remove old files if the name changed
             if old_name != new_name:            
                 # remove renamed original, if it exists
-                old_path = self.get_path_by_name(old_name, notebook_path)
+                old_path = self.get_os_path(old_name, notebook_path)
                 if os.path.isfile(old_path):
                     self.log.debug("unlinking notebook %s", old_path)
                     os.unlink(old_path)
@@ -226,7 +250,7 b' class FileNotebookManager(NotebookManager):'
             
     def delete_notebook(self, notebook_name, notebook_path):
         """Delete notebook by notebook_name."""
-        nb_path = self.get_path(notebook_name, notebook_path)
+        nb_path = self.get_os_path(notebook_name, notebook_path)
         if not os.path.isfile(nb_path):
             raise web.HTTPError(404, u'Notebook does not exist: %s' % notebook_name)
         
@@ -252,7 +276,7 b' class FileNotebookManager(NotebookManager):'
         i = 0
         while True:
             name = u'%s%i.ipynb' % (basename,i)
-            path = self.get_path_by_name(name, notebook_path)
+            path = self.get_os_path(name, notebook_path)
             if not os.path.isfile(path):
                 break
             else:
@@ -295,7 +319,7 b' class FileNotebookManager(NotebookManager):'
     
     def create_checkpoint(self, notebook_name, notebook_path=None):
         """Create a checkpoint from the current state of a notebook"""
-        nb_path = self.get_path(notebook_name, notebook_path)
+        nb_path = self.get_os_path(notebook_name, notebook_path)
         # only the one checkpoint ID:
         checkpoint_id = u"checkpoint"
         cp_path = self.get_checkpoint_path(notebook_name, checkpoint_id, notebook_path)
@@ -323,7 +347,7 b' class FileNotebookManager(NotebookManager):'
     def restore_checkpoint(self, notebook_name, checkpoint_id, notebook_path=None):
         """restore a notebook to a checkpointed state"""
         self.log.info("restoring Notebook %s from checkpoint %s", notebook_name, checkpoint_id)
-        nb_path = self.get_path(notebook_name, notebook_path)
+        nb_path = self.get_os_path(notebook_name, notebook_path)
         cp_path = self.get_checkpoint_path(notebook_name, checkpoint_id, notebook_path)
         if not os.path.isfile(cp_path):
             self.log.debug("checkpoint file does not exist: %s", cp_path)

IPython/html/services/notebooks/nbmanager.py

@@ -74,12 +74,10 b' class NotebookManager(LoggingConfigurable):'
 
     def url_encode(self, path):
         parts = os.path.split(path)
-        #parts = path.split('/')
         return os.path.join(*[quote(p) for p in parts])
 
     def url_decode(self, path):
         parts = os.path.split(path)
-        #parts = path.split('/')
         return os.path.join(*[unquote(p) for p in parts])
 
     def _notebook_dir_changed(self, new):

IPython/html/services/notebooks/tests/test_nbmanager.py

@@ -32,6 +32,28 b' class TestFileNotebookManager(TestCase):'
         with NamedTemporaryFile() as tf:
             self.assertRaises(TraitError, FileNotebookManager, notebook_dir=tf.name)
 
+    def test_get_os_path(self):
+        # full filesystem path should be returned with correct operating system
+        # separators.
+        with TemporaryDirectory() as td:
+            nbdir = os.path.join(td, 'notebooks')
+            km = FileNotebookManager(notebook_dir=nbdir)
+            path = km.get_os_path('test.ipynb', '/path/to/notebook/')
+            self.assertEqual(path, km.notebook_dir+os.sep+'path'+os.sep+'to'+os.sep+
+                'notebook'+os.sep+'test.ipynb')
+
+        with TemporaryDirectory() as td:
+            nbdir = os.path.join(td, 'notebooks')
+            km = FileNotebookManager(notebook_dir=nbdir)
+            path = km.get_os_path('test.ipynb', None)
+            self.assertEqual(path, km.notebook_dir+os.sep+'test.ipynb')
+
+        with TemporaryDirectory() as td:
+            nbdir = os.path.join(td, 'notebooks')
+            km = FileNotebookManager(notebook_dir=nbdir)
+            path = km.get_os_path('test.ipynb', '////')
+            self.assertEqual(path, km.notebook_dir+os.sep+'test.ipynb')
+
 class TestNotebookManager(TestCase):
     def test_named_notebook_path(self):
         nm = NotebookManager()

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

@@ -50,7 +50,6 b' var IPython = (function (IPython) {'
         // single worksheet for now
         this.worksheet_metadata = {};
         this.control_key_active = false;
-        this.notebook_name = null;
         this.notebook_name_blacklist_re = /[\/\\:]/;
         this.nbformat = 3 // Increment this when changing the nbformat
         this.nbformat_minor = 0 // Increment this when changing the nbformat
@@ -87,14 +86,7 b' var IPython = (function (IPython) {'
     Notebook.prototype.notebookPath = function() {
         var path = $('body').data('notebookPath');
         path = decodeURIComponent(path);
-        if (path != 'None') {
-            if (path[path.length-1] != '/') {
-                path = path.substring(0,path.length);
-            }; 
-            return path;
-        } else {
-            return '';
-        }
+        return path
     };
     
     /**
@@ -1368,7 +1360,7 b' var IPython = (function (IPython) {'
         var cells = this.get_cells();
         for (var i=0; i<ncells; i++) {
             if (cells[i] instanceof IPython.CodeCell) {
-                cells[i].clear_output();
+                cells[i].clear_output(true,true,true);
                 // Make all In[] prompts blank, as well
                 // TODO: make this configurable (via checkbox?)
                 cells[i].set_input_prompt();
@@ -1397,8 +1389,7 b' var IPython = (function (IPython) {'
      * @method start_session
      */
     Notebook.prototype.start_session = function () {
-        var notebook_info = this.notebookPath() + this.notebook_name;
-        this.session = new IPython.Session(notebook_info, this);
+        this.session = new IPython.Session(this.notebook_name, this.notebook_path, this);
         this.session.start();
         this.link_cells_to_session();
     };
@@ -1527,7 +1518,7 b' var IPython = (function (IPython) {'
      * @return {String} This notebook's name
      */
     Notebook.prototype.get_notebook_name = function () {
-        nbname = this.notebook_name.substring(0,this.notebook_name.length-6);
+        var nbname = this.notebook_name.substring(0,this.notebook_name.length-6);
         return nbname;
     };
 
@@ -1566,7 +1557,7 b' var IPython = (function (IPython) {'
      * @param {Object} data JSON representation of a notebook
      */
     Notebook.prototype.fromJSON = function (data) {
-        data = data.content;
+        var content = data.content;
         var ncells = this.ncells();
         var i;
         for (i=0; i<ncells; i++) {
@@ -1574,10 +1565,10 b' var IPython = (function (IPython) {'
             this.delete_cell(0);
         };
         // Save the metadata and name.
-        this.metadata = data.metadata;
-        this.notebook_name = data.metadata.name +'.ipynb';
+        this.metadata = content.metadata;
+        this.notebook_name = data.name;
         // Only handle 1 worksheet for now.
-        var worksheet = data.worksheets[0];
+        var worksheet = content.worksheets[0];
         if (worksheet !== undefined) {
             if (worksheet.metadata) {
                 this.worksheet_metadata = worksheet.metadata;
@@ -1598,7 +1589,7 b' var IPython = (function (IPython) {'
                 new_cell.fromJSON(cell_data);
             };
         };
-        if (data.worksheets.length > 1) {
+        if (content.worksheets.length > 1) {
             IPython.dialog.modal({
                 title : "Multiple worksheets",
                 body : "This notebook has " + data.worksheets.length + " worksheets, " +
@@ -1672,10 +1663,13 b' var IPython = (function (IPython) {'
     Notebook.prototype.save_notebook = function () {
         // We may want to move the name/id/nbformat logic inside toJSON?
         var data = this.toJSON();
-        data.metadata.name = this.notebook_name;
-        data.nbformat = this.nbformat;
-        data.nbformat_minor = this.nbformat_minor;
-        
+        var model = {};
+        // Create a JSON model to be sent to the server.
+        model['name'] = this.notebook_name;
+        model['path'] = this.notebook_path;
+        model['content'] = data
+        model.content.nbformat = this.nbformat;
+        model.content.nbformat_minor = this.nbformat_minor;
         // time the ajax call for autosave tuning purposes.
         var start =  new Date().getTime();
         // We do the call with settings so we can set cache to false.
@@ -1683,13 +1677,13 b' var IPython = (function (IPython) {'
             processData : false,
             cache : false,
             type : "PUT",
-            data : JSON.stringify(data),
+            data : JSON.stringify(model),
             headers : {'Content-Type': 'application/json'},
             success : $.proxy(this.save_notebook_success, this, start),
             error : $.proxy(this.save_notebook_error, this)
         };
         $([IPython.events]).trigger('notebook_saving.Notebook');
-        var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath()+ this.notebook_name;
+        var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath()+ this.notebook_name;
         $.ajax(url, settings);
     };
     
@@ -1752,14 +1746,15 b' var IPython = (function (IPython) {'
             type : "POST",
             dataType : "json",
             success:$.proxy(function (data, status, xhr){
-                notebook_name = data.name;
-                window.open(this._baseProjectUrl +'notebooks/' + this.notebookPath()+ notebook_name);
+                var notebook_name = data.name;
+                window.open(this.baseProjectUrl() +'notebooks' + this.notebookPath()+ notebook_name, '_blank');
             }, this)
         };
-        var url = this._baseProjectUrl + 'notebooks/' + path;
+        var url = this.baseProjectUrl() + 'api/notebooks' + path;
         $.ajax(url,settings);
     };
 
+
     Notebook.prototype.copy_notebook = function(){
         var path = this.notebookPath();
         var name = {'name': this.notebook_name}
@@ -1771,10 +1766,10 b' var IPython = (function (IPython) {'
             dataType : "json",
             success:$.proxy(function (data, status, xhr){
                 notebook_name = data.name;
-                window.open(this._baseProjectUrl +'notebooks/' + this.notebookPath()+ notebook_name);
+                window.open(this._baseProjectUrl +'notebooks' + this.notebookPath()+ notebook_name);
             }, this)
         };
-        var url = this._baseProjectUrl + 'notebooks/' + path;
+        var url = this._baseProjectUrl + 'notebooks' + path;
         $.ajax(url,settings);
     };
 
@@ -1793,15 +1788,16 b' var IPython = (function (IPython) {'
             error : $.proxy(that.rename_error, this)
         };
         $([IPython.events]).trigger('notebook_rename.Notebook');
-        var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath()+ this.notebook_name;
+        var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath()+ this.notebook_name;
         $.ajax(url, settings);
     };
     
     
     Notebook.prototype.rename_success = function (json, status, xhr) {
         this.notebook_name = json.name
-        var notebook_path = this.notebookPath() + this.notebook_name;
-        this.session.notebook_rename(notebook_path);
+        var name = this.notebook_name
+        var path = json.path
+        this.session.notebook_rename(name, path);
         $([IPython.events]).trigger('notebook_renamed.Notebook');
     }
 
@@ -1855,7 +1851,7 b' var IPython = (function (IPython) {'
              error : $.proxy(this.load_notebook_error,this),
          };
          $([IPython.events]).trigger('notebook_loading.Notebook');
-         var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath() + this.notebook_name;
+         var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath() + this.notebook_name;
          $.ajax(url, settings);
      };
 
@@ -1916,7 +1912,7 b' var IPython = (function (IPython) {'
         // Create the session after the notebook is completely loaded to prevent
         // code execution upon loading, which is a security risk.
         if (this.session == null) {
-            this.start_session(this.notebook_path);
+            this.start_session();
         }
         // load our checkpoint list
         IPython.notebook.list_checkpoints();
@@ -1988,7 +1984,7 b' var IPython = (function (IPython) {'
      * @method list_checkpoints
      */
      Notebook.prototype.list_checkpoints = function () {
-         var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath() + this.notebook_name + '/checkpoints';
+         var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath() + this.notebook_name + '/checkpoints';
          $.get(url).done(
              $.proxy(this.list_checkpoints_success, this)
          ).fail(
@@ -2033,7 +2029,7 b' var IPython = (function (IPython) {'
      * @method create_checkpoint
      */
      Notebook.prototype.create_checkpoint = function () {
-         var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath() + this.notebook_name + '/checkpoints';
+         var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath() + this.notebook_name + '/checkpoints';
          $.post(url).done(
              $.proxy(this.create_checkpoint_success, this)
          ).fail(
@@ -2113,18 +2109,8 b' var IPython = (function (IPython) {'
      * @param {String} checkpoint ID
      */
     Notebook.prototype.restore_checkpoint = function (checkpoint) {
-<<<<<<< HEAD
-        $([IPython.events]).trigger('checkpoint_restoring.Notebook', checkpoint);
-        if (this.notebook_path != "") {
-            var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebook_path + this.notebook_name + '/checkpoints/' + checkpoint;
-        }
-        else {
-            var url = this.baseProjectUrl() + 'api/notebooks/' +this.notebook_name + '/checkpoints/' + checkpoint;
-        }
-=======
         $([IPython.events]).trigger('notebook_restoring.Notebook', checkpoint);
-        var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath() + this.notebook_name + '/checkpoints/' + checkpoint;
->>>>>>> fixing path redirects, cleaning path logic
+        var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath() + this.notebook_name + '/checkpoints/' + checkpoint;
         $.post(url).done(
             $.proxy(this.restore_checkpoint_success, this)
         ).fail(
@@ -2165,7 +2151,7 b' var IPython = (function (IPython) {'
      */
     Notebook.prototype.delete_checkpoint = function (checkpoint) {
         $([IPython.events]).trigger('notebook_restoring.Notebook', checkpoint);
-        var url = this.baseProjectUrl() + 'api/notebooks/' + this.notebookPath() + this.notebook_name + '/checkpoints/' + checkpoint;
+        var url = this.baseProjectUrl() + 'api/notebooks' + this.notebookPath() + this.notebook_name + '/checkpoints/' + checkpoint;
         $.ajax(url, {
             type: 'DELETE',
             success: $.proxy(this.delete_checkpoint_success, this),
@@ -2205,4 +2191,3 b' var IPython = (function (IPython) {'
     return IPython;
 
 }(IPython));
-