From 9b2dac3fc174f7c6ba8eb72052b32fa0dd908145 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 11:42:37 -0800 Subject: [PATCH 1/9] Infrastructure for AJAX requests returning ES6 promises --- IPython/html/static/base/js/utils.js | 19 ++++++++++++++++++- IPython/html/templates/page.html | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/IPython/html/static/base/js/utils.js b/IPython/html/static/base/js/utils.js index a974166a0..d782ebff6 100644 --- a/IPython/html/static/base/js/utils.js +++ b/IPython/html/static/base/js/utils.js @@ -5,6 +5,7 @@ define([ 'base/js/namespace', 'jquery', 'codemirror/lib/codemirror', + 'es6promise', ], function(IPython, $, CodeMirror){ "use strict"; @@ -591,6 +592,21 @@ define([ return wrapped_error; }; + var promising_ajax = function(url, settings) { + // Like $.ajax, but returning an ES6 promise. success and error settings + // will be ignored. + return new Promise(function(resolve, reject) { + settings.success = function(data, status, jqXHR) { + resolve(data); + } + settings.error = function(jqXHR, status, error) { + log_ajax_error(jqXHR, status, error); + reject(wrap_ajax_error(jqXHR, status, error)); + } + $.ajax(url, settings); + }); + }; + var utils = { regex_split : regex_split, uuid : uuid, @@ -618,7 +634,8 @@ define([ log_ajax_error : log_ajax_error, requireCodeMirrorMode : requireCodeMirrorMode, XHR_ERROR : XHR_ERROR, - wrap_ajax_error : wrap_ajax_error + wrap_ajax_error : wrap_ajax_error, + promising_ajax : promising_ajax, }; // Backwards compatability. diff --git a/IPython/html/templates/page.html b/IPython/html/templates/page.html index e047424db..8c1650bd3 100644 --- a/IPython/html/templates/page.html +++ b/IPython/html/templates/page.html @@ -30,6 +30,7 @@ moment: "components/moment/moment", codemirror: 'components/codemirror', termjs: "components/term.js/src/term", + es6promise: 'components/es6-promise/promise.min', contents: '{{ contents_js_source }}', }, shim: { From 11cfcc40d4de7a7afd37ec28986bad35755c0fd1 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 11:42:52 -0800 Subject: [PATCH 2/9] Use promises for GET requests --- IPython/html/static/notebook/js/notebook.js | 9 ++++----- IPython/html/static/services/contents.js | 9 +++------ IPython/html/static/tree/js/notebooklist.js | 8 ++++---- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index e71f45157..f722997cf 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -2103,11 +2103,10 @@ define([ this.notebook_path = notebook_path; 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) - }); + this.contents.get(notebook_path, {type: 'notebook'}).then( + $.proxy(this.load_notebook_success, this), + $.proxy(this.load_notebook_error, this) + ); }; /** diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index c52bd0d7b..2528b7d7b 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -83,14 +83,12 @@ define([ cache : false, type : "GET", dataType : "json", - success : options.success, - error : this.create_basic_error_handler(options.error) }; var url = this.api_url(path); params = {}; if (options.type) { params.type = options.type; } if (options.format) { params.format = options.format; } - $.ajax(url + '?' + $.param(params), settings); + return utils.promising_ajax(url + '?' + $.param(params), settings); }; @@ -246,9 +244,8 @@ define([ * @param {String} path The path to list notebooks in * @param {Object} options including success and error callbacks */ - Contents.prototype.list_contents = function(path, options) { - options.type = 'directory'; - this.get(path, options); + Contents.prototype.list_contents = function(path) { + return this.get(path, {type: 'directory'}); }; diff --git a/IPython/html/static/tree/js/notebooklist.js b/IPython/html/static/tree/js/notebooklist.js index fbc0091a3..58ffc4d73 100644 --- a/IPython/html/static/tree/js/notebooklist.js +++ b/IPython/html/static/tree/js/notebooklist.js @@ -142,12 +142,12 @@ define([ NotebookList.prototype.load_list = function () { var that = this; - this.contents.list_contents(that.notebook_path, { - success: $.proxy(this.draw_notebook_list, this), - error: function(error) { + this.contents.list_contents(that.notebook_path).then( + $.proxy(this.draw_notebook_list, this), + function(error) { that.draw_notebook_list({content: []}, "Server error: " + error.message); } - }); + ); }; /** From 6b85afb077482cd9d1eecaf94dd44472aff7456d Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 11:59:15 -0800 Subject: [PATCH 3/9] Add es6-promise to package_data --- setupbase.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setupbase.py b/setupbase.py index 9b325148b..a604c1626 100644 --- a/setupbase.py +++ b/setupbase.py @@ -150,6 +150,7 @@ def find_package_data(): pjoin(components, "bootstrap", "js", "bootstrap.min.js"), pjoin(components, "bootstrap-tour", "build", "css", "bootstrap-tour.min.css"), pjoin(components, "bootstrap-tour", "build", "js", "bootstrap-tour.min.js"), + pjoin(components, "es6-promise", "*.js"), pjoin(components, "font-awesome", "fonts", "*.*"), pjoin(components, "google-caja", "html-css-sanitizer-minified.js"), pjoin(components, "highlight.js", "build", "highlight.pack.js"), From aecb4bffa5c8ef7dd9c87b3b9ee7ace3c24501fa Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 12:17:49 -0800 Subject: [PATCH 4/9] All aboard the promise train --- IPython/html/static/notebook/js/menubar.js | 11 ++-- IPython/html/static/notebook/js/notebook.js | 60 +++++++++---------- IPython/html/static/services/contents.js | 66 ++++++++------------- IPython/html/static/tree/js/main.js | 11 ++-- IPython/html/static/tree/js/notebooklist.js | 24 ++++---- 5 files changed, 75 insertions(+), 97 deletions(-) diff --git a/IPython/html/static/notebook/js/menubar.js b/IPython/html/static/notebook/js/menubar.js index e187fed1f..c8c42a44c 100644 --- a/IPython/html/static/notebook/js/menubar.js +++ b/IPython/html/static/notebook/js/menubar.js @@ -92,14 +92,13 @@ define([ // Create a new notebook in the same path as the current // notebook's path. var parent = utils.url_path_split(that.notebook.notebook_path)[0]; - that.contents.new_untitled(parent, { - type: "notebook", - success: function (data) { + that.contents.new_untitled(parent, {type: "notebook"}).then( + function (data) { w.location = utils.url_join_encode( that.base_url, 'notebooks', data.path ); - }, - error: function(error) { + }, + function(error) { w.close(); dialog.modal({ title : 'Creating Notebook Failed', @@ -107,7 +106,7 @@ define([ buttons : {'OK' : {'class' : 'btn-primary'}} }); } - }); + ); }); this.element.find('#open_notebook').click(function () { var parent = utils.url_path_split(that.notebook.notebook_path)[0]; diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index f722997cf..09e668bbf 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -1902,12 +1902,12 @@ define([ var start = new Date().getTime(); var that = this; - this.contents.save(this.notebook_path, model, { - success: $.proxy(this.save_notebook_success, this, start), - error: function (error) { + this.contents.save(this.notebook_path, model).then( + $.proxy(this.save_notebook_success, this, start), + function (error) { that.events.trigger('notebook_save_failed.Notebook', error); } - }); + ); }; /** @@ -2025,17 +2025,17 @@ define([ var base_url = this.base_url; var w = window.open(); var parent = utils.url_path_split(this.notebook_path)[0]; - this.contents.copy(this.notebook_path, parent, { - success: function (data) { + this.contents.copy(this.notebook_path, parent).then( + function (data) { w.location = utils.url_join_encode( base_url, 'notebooks', data.path ); }, - error : function(error) { + function(error) { w.close(); console.log(error); - }, - }); + } + ); }; Notebook.prototype.rename = function (new_name) { @@ -2046,15 +2046,15 @@ define([ var that = this; var parent = utils.url_path_split(this.notebook_path)[0]; var new_path = utils.url_path_join(parent, new_name); - this.contents.rename(this.notebook_path, new_path, { - success: function (json) { + this.contents.rename(this.notebook_path, new_path).then( + function (json) { that.notebook_name = json.name; that.notebook_path = json.path; that.session.rename_notebook(json.path); that.events.trigger('notebook_renamed.Notebook', json); }, - error: $.proxy(this.rename_error, this) - }); + $.proxy(this.rename_error, this) + ); }; Notebook.prototype.delete = function () { @@ -2326,12 +2326,12 @@ define([ */ Notebook.prototype.list_checkpoints = function () { var that = this; - this.contents.list_checkpoints(this.notebook_path, { - success: $.proxy(this.list_checkpoints_success, this), - error: function(error) { + this.contents.list_checkpoints(this.notebook_path).then( + $.proxy(this.list_checkpoints_success, this), + function(error) { that.events.trigger('list_checkpoints_failed.Notebook', error); } - }); + ); }; /** @@ -2358,12 +2358,12 @@ define([ */ Notebook.prototype.create_checkpoint = function () { var that = this; - this.contents.create_checkpoint(this.notebook_path, { - success: $.proxy(this.create_checkpoint_success, this), - error: function (error) { + this.contents.create_checkpoint(this.notebook_path).then( + $.proxy(this.create_checkpoint_success, this), + function (error) { that.events.trigger('checkpoint_failed.Notebook', error); } - }); + ); }; /** @@ -2428,13 +2428,12 @@ define([ Notebook.prototype.restore_checkpoint = function (checkpoint) { this.events.trigger('notebook_restoring.Notebook', checkpoint); var that = this; - this.contents.restore_checkpoint(this.notebook_path, - checkpoint, { - success: $.proxy(this.restore_checkpoint_success, this), - error: function (error) { + this.contents.restore_checkpoint(this.notebook_path, checkpoint).then( + $.proxy(this.restore_checkpoint_success, this), + function (error) { that.events.trigger('checkpoint_restore_failed.Notebook', error); } - }); + ); }; /** @@ -2456,13 +2455,12 @@ define([ Notebook.prototype.delete_checkpoint = function (checkpoint) { this.events.trigger('notebook_restoring.Notebook', checkpoint); var that = this; - this.contents.delete_checkpoint(this.notebook_path, - checkpoint, { - success: $.proxy(this.delete_checkpoint_success, this), - error: function (error) { + this.contents.delete_checkpoint(this.notebook_path, checkpoint).then( + $.proxy(this.delete_checkpoint_success, this), + function (error) { that.events.trigger('checkpoint_delete_failed.Notebook', error); } - }); + ); }; /** diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index 2528b7d7b..b90714d87 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -112,33 +112,31 @@ define([ type : "POST", data: data, dataType : "json", - success : options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; - $.ajax(this.api_url(path), settings); + return utils.promising_ajax(this.api_url(path), settings); }; - Contents.prototype.delete = function(path, options) { - var error_callback = options.error || function() {}; + Contents.prototype.delete = function(path) { var settings = { processData : false, type : "DELETE", dataType : "json", - success : options.success || function() {}, - error : function(xhr, status, error) { + }; + var url = this.api_url(path); + return utils.promising_ajax(url, settings).catch( + // Translate certain errors to more specific ones. + function(error) { // TODO: update IPEP27 to specify errors more precisely, so // that error types can be detected here with certainty. - if (xhr.status === 400) { - error_callback(new Contents.DirectoryNotEmptyError()); + if (error.xhr.status === 400) { + return Promise.reject(new Contents.DirectoryNotEmptyError()); } - error_callback(utils.wrap_ajax_error(xhr, status, error)); + return Promise.reject(error); } - }; - var url = this.api_url(path); - $.ajax(url, settings); + ); }; - Contents.prototype.rename = function(path, new_path, options) { + Contents.prototype.rename = function(path, new_path) { var data = {path: new_path}; var settings = { processData : false, @@ -146,28 +144,24 @@ define([ data : JSON.stringify(data), dataType: "json", contentType: 'application/json', - success : options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; var url = this.api_url(path); - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; - Contents.prototype.save = function(path, model, options) { + Contents.prototype.save = function(path, model) { // We do the call with settings so we can set cache to false. var settings = { processData : false, type : "PUT", data : JSON.stringify(model), contentType: 'application/json', - success : options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; var url = this.api_url(path); - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; - Contents.prototype.copy = function(from_file, to_dir, options) { + Contents.prototype.copy = function(from_file, to_dir) { // Copy a file into a given directory via POST // The server will select the name of the copied file var url = this.api_url(to_dir); @@ -177,54 +171,44 @@ define([ type: "POST", data: JSON.stringify({copy_from: from_file}), dataType : "json", - success: options.success || function() {}, - error: this.create_basic_error_handler(options.error) }; - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; /** * Checkpointing Functions */ - Contents.prototype.create_checkpoint = function(path, options) { + Contents.prototype.create_checkpoint = function(path) { var url = this.api_url(path, 'checkpoints'); var settings = { type : "POST", - success: options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; - Contents.prototype.list_checkpoints = function(path, options) { + Contents.prototype.list_checkpoints = function(path) { var url = this.api_url(path, 'checkpoints'); var settings = { type : "GET", - success: options.success, - error : this.create_basic_error_handler(options.error) }; - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; - Contents.prototype.restore_checkpoint = function(path, checkpoint_id, options) { + Contents.prototype.restore_checkpoint = function(path, checkpoint_id) { var url = this.api_url(path, 'checkpoints', checkpoint_id); var settings = { type : "POST", - success: options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; - Contents.prototype.delete_checkpoint = function(path, checkpoint_id, options) { + Contents.prototype.delete_checkpoint = function(path, checkpoint_id) { var url = this.api_url(path, 'checkpoints', checkpoint_id); var settings = { type : "DELETE", - success: options.success || function() {}, - error : this.create_basic_error_handler(options.error) }; - $.ajax(url, settings); + return utils.promising_ajax(url, settings); }; /** diff --git a/IPython/html/static/tree/js/main.js b/IPython/html/static/tree/js/main.js index 53d2ca937..f55c480b5 100644 --- a/IPython/html/static/tree/js/main.js +++ b/IPython/html/static/tree/js/main.js @@ -65,14 +65,13 @@ require([ $('#new_notebook').click(function (e) { var w = window.open(); - contents.new_untitled(common_options.notebook_path, { - type: "notebook", - success: function (data) { + contents.new_untitled(common_options.notebook_path, {type: "notebook"}).then( + function (data) { w.location = utils.url_join_encode( common_options.base_url, 'notebooks', data.path ); - }, - error: function(error) { + }, + function(error) { w.close(); dialog.modal({ title : 'Creating Notebook Failed', @@ -80,7 +79,7 @@ require([ buttons : {'OK' : {'class' : 'btn-primary'}} }); } - }); + ); }); var interval_id=0; diff --git a/IPython/html/static/tree/js/notebooklist.js b/IPython/html/static/tree/js/notebooklist.js index 58ffc4d73..8e727f50e 100644 --- a/IPython/html/static/tree/js/notebooklist.js +++ b/IPython/html/static/tree/js/notebooklist.js @@ -328,11 +328,11 @@ define([ Delete : { class: "btn-danger", click: function() { - notebooklist.contents.delete(path, { - success: function() { + notebooklist.contents.delete(path).then( + function() { notebooklist.notebook_deleted(path); } - }); + ); } }, Cancel : {} @@ -414,13 +414,11 @@ define([ } filedata = item.data('filedata'); - var settings = { - success : function () { - item.removeClass('new-file'); - that.add_link(model, item); - that.add_delete_button(item); - that.session_list.load_sessions(); - }, + var on_success = function () { + item.removeClass('new-file'); + that.add_link(model, item); + that.add_delete_button(item); + that.session_list.load_sessions(); }; var exists = false; @@ -436,8 +434,8 @@ define([ Overwrite : { class: "btn-danger", click: function () { - that.contents.save(path, model, settings); - } + that.contents.save(path, model).then(on_success); + } }, Cancel : { click: function() { item.remove(); } @@ -445,7 +443,7 @@ define([ } }); } else { - that.contents.save(path, model, settings); + that.contents.save(path, model).then(on_success); } return false; From 56320588da41cf768d46da7ec1b4302731bc60ad Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 12:20:53 -0800 Subject: [PATCH 5/9] Return JSON from contents API checkpoint methods --- IPython/html/static/notebook/js/notebook.js | 2 -- IPython/html/static/services/contents.js | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index 09e668bbf..643818d78 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -2341,7 +2341,6 @@ define([ * @param {Object} data JSON representation of a checkpoint */ Notebook.prototype.list_checkpoints_success = function (data) { - data = $.parseJSON(data); this.checkpoints = data; if (data.length) { this.last_checkpoint = data[data.length - 1]; @@ -2373,7 +2372,6 @@ define([ * @param {Object} data JSON representation of a checkpoint */ Notebook.prototype.create_checkpoint_success = function (data) { - data = $.parseJSON(data); this.add_checkpoint(data); this.events.trigger('checkpoint_created.Notebook', data); }; diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index b90714d87..c2ee4ba35 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -183,6 +183,7 @@ define([ var url = this.api_url(path, 'checkpoints'); var settings = { type : "POST", + dataType : "json", }; return utils.promising_ajax(url, settings); }; @@ -191,6 +192,8 @@ define([ var url = this.api_url(path, 'checkpoints'); var settings = { type : "GET", + cache: false, + dataType: "json", }; return utils.promising_ajax(url, settings); }; From 0295f5a4877e19e0f8e1fc6846e043621e14e29a Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 12:30:26 -0800 Subject: [PATCH 6/9] Semicolons --- IPython/html/static/base/js/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IPython/html/static/base/js/utils.js b/IPython/html/static/base/js/utils.js index d782ebff6..23b3dc316 100644 --- a/IPython/html/static/base/js/utils.js +++ b/IPython/html/static/base/js/utils.js @@ -598,11 +598,11 @@ define([ return new Promise(function(resolve, reject) { settings.success = function(data, status, jqXHR) { resolve(data); - } + }; settings.error = function(jqXHR, status, error) { log_ajax_error(jqXHR, status, error); reject(wrap_ajax_error(jqXHR, status, error)); - } + }; $.ajax(url, settings); }); }; From 1e53c1f92445b0e5ddf762ac995936eca7f1b0e3 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 14:25:28 -0800 Subject: [PATCH 7/9] Re-raise errors with throw instead of Promise.reject() --- IPython/html/static/services/contents.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index c2ee4ba35..11c225511 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -129,9 +129,9 @@ define([ // TODO: update IPEP27 to specify errors more precisely, so // that error types can be detected here with certainty. if (error.xhr.status === 400) { - return Promise.reject(new Contents.DirectoryNotEmptyError()); + throw new Contents.DirectoryNotEmptyError(); } - return Promise.reject(error); + throw error; } ); }; From d3699c8e3b00aa45a633eff577020bea887ebb1c Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Thu, 13 Nov 2014 14:44:57 -0800 Subject: [PATCH 8/9] Update JS docstrings in contents API --- IPython/html/static/services/contents.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index 11c225511..760056091 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -73,8 +73,9 @@ define([ * * @method get * @param {String} path - * @param {Function} success - * @param {Function} error + * @param {Object} options + * type : 'notebook', 'file', or 'directory' + * format: 'text' or 'base64'; only relevant for type: 'file' */ Contents.prototype.get = function (path, options) { // We do the call with settings so we can set cache to false. @@ -229,7 +230,6 @@ define([ * last_modified: last modified dat * @method list_notebooks * @param {String} path The path to list notebooks in - * @param {Object} options including success and error callbacks */ Contents.prototype.list_contents = function(path) { return this.get(path, {type: 'directory'}); From 8c962df79c0e0900d58cc588b3055cb7c642d9c2 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Fri, 14 Nov 2014 12:43:44 -0800 Subject: [PATCH 9/9] Load promises polyfill from a script tag So all JS code can assume promises work, without needing to require it. --- IPython/html/static/base/js/utils.js | 1 - IPython/html/templates/page.html | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/IPython/html/static/base/js/utils.js b/IPython/html/static/base/js/utils.js index 23b3dc316..013389cd4 100644 --- a/IPython/html/static/base/js/utils.js +++ b/IPython/html/static/base/js/utils.js @@ -5,7 +5,6 @@ define([ 'base/js/namespace', 'jquery', 'codemirror/lib/codemirror', - 'es6promise', ], function(IPython, $, CodeMirror){ "use strict"; diff --git a/IPython/html/templates/page.html b/IPython/html/templates/page.html index 8c1650bd3..4c314df20 100644 --- a/IPython/html/templates/page.html +++ b/IPython/html/templates/page.html @@ -14,6 +14,7 @@ {% endblock %} +