From 455ae2b8817a91f5bc7e56159a8120e72f31d1cb Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Thu, 2 Oct 2014 00:30:25 +0200 Subject: [PATCH] Fixed promises and its race conditions --- public_html/js/Auth.js | 4 +- public_html/js/BrowsingSettings.js | 4 +- public_html/js/Pager.js | 23 ++++----- public_html/js/PresenterManager.js | 3 ++ public_html/js/Presenters/HomePresenter.js | 2 +- public_html/js/Presenters/LoginPresenter.js | 2 +- public_html/js/Presenters/PagerPresenter.js | 14 ++++- .../js/Presenters/PostListPresenter.js | 7 ++- public_html/js/Presenters/PostPresenter.js | 34 ++++++------- .../js/Presenters/RegistrationPresenter.js | 2 +- .../Presenters/UserAccountRemovalPresenter.js | 2 +- .../UserAccountSettingsPresenter.js | 2 +- .../js/Presenters/UserActivationPresenter.js | 4 +- .../UserBrowsingSettingsPresenter.js | 2 +- .../js/Presenters/UserListPresenter.js | 7 ++- public_html/js/Presenters/UserPresenter.js | 2 +- public_html/js/Promise.js | 51 +++++++++++++++---- public_html/js/Router.js | 7 ++- 18 files changed, 113 insertions(+), 59 deletions(-) diff --git a/public_html/js/Auth.js b/public_html/js/Auth.js index c04ba2d4..1fe4361f 100644 --- a/public_html/js/Auth.js +++ b/public_html/js/Auth.js @@ -86,7 +86,9 @@ App.Auth = function(_, jQuery, util, api, appState, promise) { return promise.make(function(resolve, reject) { jQuery.removeCookie('auth'); appState.set('loginToken', null); - return loginAnonymous().then(resolve).fail(reject); + return promise.wait(loginAnonymous()) + .then(resolve) + .fail(reject); }); } diff --git a/public_html/js/BrowsingSettings.js b/public_html/js/BrowsingSettings.js index 3872c8b8..0e214f53 100644 --- a/public_html/js/BrowsingSettings.js +++ b/public_html/js/BrowsingSettings.js @@ -73,7 +73,9 @@ App.BrowsingSettings = function( return promise.make(function(resolve, reject) { saveToLocalStorage(); if (auth.isLoggedIn()) { - saveToUser(auth.getCurrentUser()).then(resolve).fail(reject); + promise.wait(saveToUser(auth.getCurrentUser())) + .then(resolve) + .fail(reject); } else { resolve(); } diff --git a/public_html/js/Pager.js b/public_html/js/Pager.js index 0a8d0c78..398f8026 100644 --- a/public_html/js/Pager.js +++ b/public_html/js/Pager.js @@ -55,20 +55,19 @@ App.Pager = function( function retrieve() { return promise.make(function(resolve, reject) { - promise.wait( - api.get(url, _.extend({}, searchParams, {page: pageNumber}))) - .then(function(response) { - var pageSize = response.json.pageSize; - var totalRecords = response.json.totalRecords; - totalPages = Math.ceil(totalRecords / pageSize); + promise.wait(api.get(url, _.extend({}, searchParams, {page: pageNumber}))) + .then(function(response) { + var pageSize = response.json.pageSize; + var totalRecords = response.json.totalRecords; + totalPages = Math.ceil(totalRecords / pageSize); - resolve({ - entities: response.json.data, - totalRecords: totalRecords}); + resolve({ + entities: response.json.data, + totalRecords: totalRecords}); - }).fail(function(response) { - reject(response); - }); + }).fail(function(response) { + reject(response); + }); }); } diff --git a/public_html/js/PresenterManager.js b/public_html/js/PresenterManager.js index 9aad5578..5cbb887b 100644 --- a/public_html/js/PresenterManager.js +++ b/public_html/js/PresenterManager.js @@ -34,6 +34,9 @@ App.PresenterManager = function(jQuery, topNavigationPresenter, keyboard) { }, 100); if (lastContentPresenter === null || lastContentPresenter.name !== presenter.name) { + if (lastContentPresenter !== null && lastContentPresenter.deinit) { + lastContentPresenter.deinit(); + } keyboard.reset(); topNavigationPresenter.changeTitle(null); topNavigationPresenter.focus(); diff --git a/public_html/js/Presenters/HomePresenter.js b/public_html/js/Presenters/HomePresenter.js index 81799960..adcec871 100644 --- a/public_html/js/Presenters/HomePresenter.js +++ b/public_html/js/Presenters/HomePresenter.js @@ -21,7 +21,7 @@ App.Presenters.HomePresenter = function( topNavigationPresenter.select('home'); topNavigationPresenter.changeTitle('Home'); - promise.waitAll( + promise.wait( util.promiseTemplate('home'), util.promiseTemplate('post-content'), api.get('/globals'), diff --git a/public_html/js/Presenters/LoginPresenter.js b/public_html/js/Presenters/LoginPresenter.js index 317da96c..c88fa6d3 100644 --- a/public_html/js/Presenters/LoginPresenter.js +++ b/public_html/js/Presenters/LoginPresenter.js @@ -58,7 +58,7 @@ App.Presenters.LoginPresenter = function( return false; } - auth.loginFromCredentials(userNameOrEmail, password, remember) + promise.wait(auth.loginFromCredentials(userNameOrEmail, password, remember)) .then(function(response) { finishLogin(); }).fail(function(response) { diff --git a/public_html/js/Presenters/PagerPresenter.js b/public_html/js/Presenters/PagerPresenter.js index 04aa2187..08ca18e7 100644 --- a/public_html/js/Presenters/PagerPresenter.js +++ b/public_html/js/Presenters/PagerPresenter.js @@ -52,7 +52,7 @@ App.Presenters.PagerPresenter = function( pager.setSearchParams(args.searchParams); pager.setPage(args.page || 1); - retrieve() + promise.wait(retrieve()) .then(loaded) .fail(loaded); @@ -62,6 +62,11 @@ App.Presenters.PagerPresenter = function( } } + + function deinit() { + detachNextPageLoader(); + } + function prevPage() { pager.prevPage(); syncUrl(); @@ -126,7 +131,7 @@ App.Presenters.PagerPresenter = function( showSpinner(); return promise.make(function(resolve, reject) { - pager.retrieve() + promise.wait(pager.retrieve()) .then(function(response) { updateCallback(response, forceClear || !endlessScroll); forceClear = false; @@ -176,6 +181,10 @@ App.Presenters.PagerPresenter = function( }, 100); } + function detachNextPageLoader() { + window.clearInterval(scrollInterval); + } + function showPageList() { $pageList.show(); } @@ -224,6 +233,7 @@ App.Presenters.PagerPresenter = function( return { init: init, reinit: reinit, + deinit: deinit, setPage: setPage, setSearchParams: setSearchParams, }; diff --git a/public_html/js/Presenters/PostListPresenter.js b/public_html/js/Presenters/PostListPresenter.js index 5a11e998..40423206 100644 --- a/public_html/js/Presenters/PostListPresenter.js +++ b/public_html/js/Presenters/PostListPresenter.js @@ -25,7 +25,7 @@ App.Presenters.PostListPresenter = function( topNavigationPresenter.changeTitle('Posts'); searchArgs = util.parseComplexRouteArgs(args.searchArgs); - promise.waitAll( + promise.wait( util.promiseTemplate('post-list'), util.promiseTemplate('post-list-item')) .then(function(listHtml, itemHtml) { @@ -60,6 +60,10 @@ App.Presenters.PostListPresenter = function( order: searchArgs.order}}); } + function deinit() { + pagerPresenter.deinit(); + } + function render() { $el.html(listTemplate()); $searchInput = $el.find('input[name=query]'); @@ -107,6 +111,7 @@ App.Presenters.PostListPresenter = function( return { init: init, reinit: reinit, + deinit: deinit, render: render, }; diff --git a/public_html/js/Presenters/PostPresenter.js b/public_html/js/Presenters/PostPresenter.js index 73679950..44b4eede 100644 --- a/public_html/js/Presenters/PostPresenter.js +++ b/public_html/js/Presenters/PostPresenter.js @@ -49,7 +49,7 @@ App.Presenters.PostPresenter = function( editPrivileges.canChangeThumbnail = auth.hasPrivilege(auth.privileges.changePostThumbnail); editPrivileges.canChangeRelations = auth.hasPrivilege(auth.privileges.changePostRelations); - promise.waitAll( + promise.wait( util.promiseTemplate('post'), util.promiseTemplate('post-edit'), util.promiseTemplate('post-content'), @@ -74,7 +74,7 @@ App.Presenters.PostPresenter = function( function reinit(args, loaded) { postNameOrId = args.postNameOrId; - refreshPost() + promise.wait(refreshPost()) .then(function() { topNavigationPresenter.changeTitle('@' + post.id); render(); @@ -84,7 +84,7 @@ App.Presenters.PostPresenter = function( function refreshPost() { return promise.make(function(resolve, reject) { - promise.waitAll(api.get('/posts/' + postNameOrId)) + promise.wait(api.get('/posts/' + postNameOrId)) .then(function(postResponse) { post = postResponse.json; postScore = postResponse.json.ownScore; @@ -170,7 +170,7 @@ App.Presenters.PostPresenter = function( } function deletePost() { - api.delete('/posts/' + post.id) + promise.wait(api.delete('/posts/' + post.id)) .then(function(response) { router.navigate('#/posts'); }).fail(showGenericError); @@ -185,11 +185,10 @@ App.Presenters.PostPresenter = function( } function featurePost() { - api.post('/posts/' + post.id + '/feature') + promise.wait(api.post('/posts/' + post.id + '/feature')) .then(function(response) { router.navigate('#/home'); - }) - .fail(showGenericError); + }).fail(showGenericError); } function editButtonClicked(e) { @@ -304,19 +303,17 @@ App.Presenters.PostPresenter = function( } function addFavorite() { - api.post('/posts/' + post.id + '/favorites') + promise.wait(api.post('/posts/' + post.id + '/favorites')) .then(function(response) { - refreshPost().then(renderSidebar); - }) - .fail(showGenericError); + promise.wait(refreshPost()).then(renderSidebar); + }).fail(showGenericError); } function deleteFavorite() { - api.delete('/posts/' + post.id + '/favorites') + promise.wait(api.delete('/posts/' + post.id + '/favorites')) .then(function(response) { - refreshPost().then(renderSidebar); - }) - .fail(showGenericError); + promise.wait(refreshPost()).then(renderSidebar); + }).fail(showGenericError); } function scoreUpButtonClicked(e) { @@ -332,11 +329,10 @@ App.Presenters.PostPresenter = function( } function score(scoreValue) { - api.post('/posts/' + post.id + '/score', {score: scoreValue}) + promise.wait(api.post('/posts/' + post.id + '/score', {score: scoreValue})) .then(function() { - refreshPost().then(renderSidebar); - }) - .fail(showGenericError); + promise.wait(refreshPost()).then(renderSidebar); + }).fail(showGenericError); } function showEditError(response) { diff --git a/public_html/js/Presenters/RegistrationPresenter.js b/public_html/js/Presenters/RegistrationPresenter.js index a8faeccd..af69f9a5 100644 --- a/public_html/js/Presenters/RegistrationPresenter.js +++ b/public_html/js/Presenters/RegistrationPresenter.js @@ -47,7 +47,7 @@ App.Presenters.RegistrationPresenter = function( return; } - api.post('/users', formData) + promise.wait(api.post('/users', formData)) .then(function(response) { registrationSuccess(response); }).fail(function(response) { diff --git a/public_html/js/Presenters/UserAccountRemovalPresenter.js b/public_html/js/Presenters/UserAccountRemovalPresenter.js index 7e2156d3..f3296d03 100644 --- a/public_html/js/Presenters/UserAccountRemovalPresenter.js +++ b/public_html/js/Presenters/UserAccountRemovalPresenter.js @@ -54,7 +54,7 @@ App.Presenters.UserAccountRemovalPresenter = function( messagePresenter.showError($messages, 'Must confirm to proceed.'); return; } - api.delete('/users/' + user.name) + promise.wait(api.delete('/users/' + user.name)) .then(function() { auth.logout(); var $messageDiv = messagePresenter.showInfo($messages, 'Account deleted. Back to main page'); diff --git a/public_html/js/Presenters/UserAccountSettingsPresenter.js b/public_html/js/Presenters/UserAccountSettingsPresenter.js index 8baedc22..c7c8ee0d 100644 --- a/public_html/js/Presenters/UserAccountSettingsPresenter.js +++ b/public_html/js/Presenters/UserAccountSettingsPresenter.js @@ -120,7 +120,7 @@ App.Presenters.UserAccountSettingsPresenter = function( delete formData.passwordConfirmation; } - api.put('/users/' + user.name, formData) + promise.wait(api.put('/users/' + user.name, formData)) .then(function(response) { editSuccess(response); }).fail(function(response) { diff --git a/public_html/js/Presenters/UserActivationPresenter.js b/public_html/js/Presenters/UserActivationPresenter.js index 5d4a22fc..30c548c6 100644 --- a/public_html/js/Presenters/UserActivationPresenter.js +++ b/public_html/js/Presenters/UserActivationPresenter.js @@ -70,7 +70,7 @@ App.Presenters.UserActivationPresenter = function( '/password-reset/' + userNameOrEmail : '/activation/' + userNameOrEmail; - api.post(url) + promise.wait(api.post(url)) .then(function(response) { var message = operation === 'passwordReset' ? 'Password reset request sent.' : @@ -92,7 +92,7 @@ App.Presenters.UserActivationPresenter = function( '/finish-password-reset/' + token : '/finish-activation/' + token; - api.post(url) + promise.wait(api.post(url)) .then(function(response) { var message = operation === 'passwordReset' ? 'Your new password is ' + response.json.newPassword + '.' : diff --git a/public_html/js/Presenters/UserBrowsingSettingsPresenter.js b/public_html/js/Presenters/UserBrowsingSettingsPresenter.js index 2e7e40b7..79d48a34 100644 --- a/public_html/js/Presenters/UserBrowsingSettingsPresenter.js +++ b/public_html/js/Presenters/UserBrowsingSettingsPresenter.js @@ -51,7 +51,7 @@ App.Presenters.UserBrowsingSettingsPresenter = function( }, }; - browsingSettings.setSettings(newSettings) + promise.wait(browsingSettings.setSettings(newSettings)) .then(function() { messagePresenter.showInfo($messages, 'Browsing settings updated!'); }); diff --git a/public_html/js/Presenters/UserListPresenter.js b/public_html/js/Presenters/UserListPresenter.js index 9dc89837..a67f7daa 100644 --- a/public_html/js/Presenters/UserListPresenter.js +++ b/public_html/js/Presenters/UserListPresenter.js @@ -18,7 +18,7 @@ App.Presenters.UserListPresenter = function( topNavigationPresenter.select('users'); topNavigationPresenter.changeTitle('Users'); - promise.waitAll( + promise.wait( util.promiseTemplate('user-list'), util.promiseTemplate('user-list-item')) .then(function(listHtml, itemHtml) { @@ -55,6 +55,10 @@ App.Presenters.UserListPresenter = function( order: searchArgs.order}}); } + function deinit() { + pagerPresenter.deinit(); + } + function render() { $el.html(listTemplate()); $el.find('.order a').click(orderLinkClicked); @@ -93,6 +97,7 @@ App.Presenters.UserListPresenter = function( return { init: init, reinit: reinit, + deinit: deinit, render: render, }; diff --git a/public_html/js/Presenters/UserPresenter.js b/public_html/js/Presenters/UserPresenter.js index 7aac2c8e..738bef44 100644 --- a/public_html/js/Presenters/UserPresenter.js +++ b/public_html/js/Presenters/UserPresenter.js @@ -27,7 +27,7 @@ App.Presenters.UserPresenter = function( topNavigationPresenter.select(auth.isLoggedIn(userName) ? 'my-account' : 'users'); topNavigationPresenter.changeTitle(userName); - promise.waitAll( + promise.wait( util.promiseTemplate('user'), api.get('/users/' + userName)) .then(function( diff --git a/public_html/js/Promise.js b/public_html/js/Promise.js index f4d864b2..aef732fc 100644 --- a/public_html/js/Promise.js +++ b/public_html/js/Promise.js @@ -1,28 +1,57 @@ var App = App || {}; -App.Promise = function(jQuery) { +App.Promise = function(_, jQuery) { - function make(callback) - { + var active = []; + + function make(callback) { var deferred = jQuery.Deferred(); - callback(deferred.resolve, deferred.reject); - return deferred.promise(); + var promise = deferred.promise(); + + callback(function() { + deferred.resolve.apply(deferred, arguments); + active = _.without(active, promise); + }, function() { + deferred.reject.apply(deferred, arguments); + active = _.without(active, promise); + }); + + promise.then(function() { + if (!_.contains(active, promise)) { + throw new Error('Broken promise'); + } + }); + + active.push(promise); + return promise; } - function wait(promise) { - return jQuery.when(promise); + function wait() { + var promises = arguments; + var deferred = jQuery.Deferred(); + return jQuery.when.apply(jQuery, promises) + .then(function() { + return deferred.resolve.apply(deferred, arguments); + }).fail(function() { + return deferred.reject.apply(deferred, arguments); + }); } - function waitAll() { - return jQuery.when.apply(jQuery, arguments); + function abortAll() { + active = []; + } + + function getActive() { + return active.length; } return { make: make, wait: wait, - waitAll: waitAll, + getActive: getActive, + abortAll: abortAll, }; }; -App.DI.registerSingleton('promise', ['jQuery'], App.Promise); +App.DI.registerSingleton('promise', ['_', 'jQuery'], App.Promise); diff --git a/public_html/js/Router.js b/public_html/js/Router.js index 1878a966..dd2c6ae7 100644 --- a/public_html/js/Router.js +++ b/public_html/js/Router.js @@ -1,6 +1,6 @@ var App = App || {}; -App.Router = function(pathJs, _, jQuery, util, appState, presenterManager) { +App.Router = function(pathJs, _, jQuery, promise, util, appState, presenterManager) { var root = '#/'; var previousLocation = window.location.href; @@ -65,6 +65,9 @@ App.Router = function(pathJs, _, jQuery, util, appState, presenterManager) { } } } + + //abort every operation that can be executed + promise.abortAll(); previousLocation = window.location.href; var finalParams = _.extend( @@ -87,4 +90,4 @@ App.Router = function(pathJs, _, jQuery, util, appState, presenterManager) { }; -App.DI.registerSingleton('router', ['pathJs', '_', 'jQuery', 'util', 'appState', 'presenterManager'], App.Router); +App.DI.registerSingleton('router', ['pathJs', '_', 'jQuery', 'promise', 'util', 'appState', 'presenterManager'], App.Router);