From f880ecb5d98f31d3ad3051df96f55b76bc0820a2 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 1 Apr 2011 13:50:05 +0200 Subject: [PATCH] [FIX] race condition on auto_search due to desynchronized loading of search view and {any default main view} If loading of search view completes before loading of content view, do_search is immediately called on search view and forwarded to content view which is *not* ready yet and therefore blows up. Solved by having start() methods return a promise object (or a non-promise object taken as a fulfilled promise) which the viewmanager can use (from both search view and initial content view) to synchronize when both are ready and it's possible to start a search bzr revid: xmo@openerp.com-20110401115005-inceqsj7qn4k3b8e --- addons/base/static/src/js/chrome.js | 44 +++++++++++++++++++++++++++-- addons/base/static/src/js/form.js | 2 +- addons/base/static/src/js/list.js | 2 +- addons/base/static/src/js/search.js | 2 +- addons/base/static/src/js/views.js | 25 +++++++++++----- 5 files changed, 62 insertions(+), 13 deletions(-) diff --git a/addons/base/static/src/js/chrome.js b/addons/base/static/src/js/chrome.js index 14577c9d0ab..523f2c8a2b7 100644 --- a/addons/base/static/src/js/chrome.js +++ b/addons/base/static/src/js/chrome.js @@ -184,8 +184,19 @@ openerp.base.BasicController = Class.extend( * Controller start * event binding, rpc and callback calling required to initialize the * object can happen here + * + * Returns a promise object letting callers (subclasses and direct callers) + * know when this component is done starting + * + * @returns {jQuery.Deferred} */ start: function() { + // returns an already fulfilled promise. Maybe we could return nothing? + // $.when can take non-deferred and in that case it simply considers + // them all as fulfilled promises. + // But in thise case we *have* to ensure callers use $.when and don't + // try to call deferred methods on this return value. + return $.Deferred().done().promise(); }, stop: function() { }, @@ -250,6 +261,19 @@ openerp.base.Session = openerp.base.BasicController.extend( } }); }, + /** + * Executes an RPC call, registering the provided callbacks. + * + * Registers a default error callback if none is provided, and handles + * setting the correct session id and session context in the parameter + * objects + * + * @param {String} url RPC endpoint + * @param {Object} params call parameters + * @param {Function} success_callback function to execute on RPC call success + * @param {Function} error_callback function to execute on RPC call failure + * @returns {jQuery.Deferred} jquery-provided ajax deferred + */ rpc: function(url, params, success_callback, error_callback) { // Construct a JSON-RPC2 request, method is currently unused params.session_id = this.session_id; @@ -259,17 +283,22 @@ openerp.base.Session = openerp.base.BasicController.extend( error_callback = typeof(error_callback) != "undefined" ? error_callback : this.on_rpc_error; // Call using the rpc_mode - this.rpc_ajax(url, { + return this.rpc_ajax(url, { jsonrpc: "2.0", method: "call", params: params, id:null }, success_callback, error_callback); }, + /** + * Raw JSON-RPC call + * + * @returns {jQuery.Deferred} ajax-based deferred object + */ rpc_ajax: function(url, payload, success_callback, error_callback) { var self = this; this.on_rpc_request(); - $.ajax({ + return $.ajax({ type: "POST", url: url, dataType: 'json', @@ -446,9 +475,18 @@ openerp.base.Controller = openerp.base.BasicController.extend( if(this.session) this.session.log.apply(this.session,arguments); }, + /** + * Performs a JSON-RPC call + * + * @param {String} url endpoint url + * @param {Object} data RPC parameters + * @param {Function} success RPC call success callback + * @param {Function} error RPC call error callback + * @returns {jQuery.Deferred} deferred object for the RPC call + */ rpc: function(url, data, success, error) { // TODO: support additional arguments ? - this.session.rpc(url, data, success, error); + return this.session.rpc(url, data, success, error); } }); diff --git a/addons/base/static/src/js/form.js b/addons/base/static/src/js/form.js index 707a5149542..65ebd80d751 100644 --- a/addons/base/static/src/js/form.js +++ b/addons/base/static/src/js/form.js @@ -30,7 +30,7 @@ openerp.base.FormView = openerp.base.Controller.extend( }, start: function() { //this.log('Starting FormView '+this.model+this.view_id) - this.rpc("/base/formview/load", {"model": this.model, "view_id": this.view_id}, this.on_loaded); + return this.rpc("/base/formview/load", {"model": this.model, "view_id": this.view_id}, this.on_loaded); }, on_loaded: function(data) { var self = this; diff --git a/addons/base/static/src/js/list.js b/addons/base/static/src/js/list.js index 5c8599176a1..c64f08a79cf 100644 --- a/addons/base/static/src/js/list.js +++ b/addons/base/static/src/js/list.js @@ -23,7 +23,7 @@ openerp.base.ListView = openerp.base.Controller.extend({ }, start: function() { //this.log('Starting ListView '+this.model+this.view_id) - this.rpc("/base/listview/load", {"model": this.model, "view_id":this.view_id}, this.on_loaded); + return this.rpc("/base/listview/load", {"model": this.model, "view_id":this.view_id}, this.on_loaded); }, on_loaded: function(data) { this.fields_view = data.fields_view; diff --git a/addons/base/static/src/js/search.js b/addons/base/static/src/js/search.js index ef00a051626..b66d1d759aa 100644 --- a/addons/base/static/src/js/search.js +++ b/addons/base/static/src/js/search.js @@ -14,7 +14,7 @@ openerp.base.SearchView = openerp.base.Controller.extend({ }, start: function() { //this.log('Starting SearchView '+this.model+this.view_id) - this.rpc("/base/searchview/load", {"model": this.model, "view_id":this.view_id}, this.on_loaded); + return this.rpc("/base/searchview/load", {"model": this.model, "view_id":this.view_id}, this.on_loaded); }, show: function () { this.$element.show(); diff --git a/addons/base/static/src/js/views.js b/addons/base/static/src/js/views.js index 4e26766bafc..c914edfea85 100644 --- a/addons/base/static/src/js/views.js +++ b/addons/base/static/src/js/views.js @@ -44,7 +44,14 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ }, start: function() { }, + /** + * Asks the view manager to switch visualization mode. + * + * @param {String} view_type type of view to display + * @returns {jQuery.Deferred} new view loading promise + */ on_mode_switch: function(view_type) { + var view_promise; this.active_view = view_type; var view = this.views[view_type]; if (!view.controller) { @@ -52,7 +59,7 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ var controller = new (openerp.base.views.get_object(view_type))( this.session, this.element_id + "_view_" + view_type, this.dataset, view.view_id); - controller.start(); + view_promise = controller.start(); this.views[view_type].controller = controller; } @@ -72,6 +79,7 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ this.views[i].controller.$element.toggle(i === view_type); } } + return view_promise; }, /** * Extract search view defaults from the current action's context. @@ -94,6 +102,7 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ * Sets up the current viewmanager's search view. * * @param action the action being executed + * @returns {jQuery.Deferred} search view startup deferred */ setup_search_view:function (action) { var self = this; @@ -108,7 +117,7 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ searchview.on_search.add(function() { self.views[self.active_view].controller.do_search.apply(self, arguments); }); - searchview.start(); + return searchview.start(); }, do_action_window: function(action) { var self = this; @@ -118,7 +127,7 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ this.$element.html(QWeb.render("ViewManager", {"prefix": this.element_id, views: action.views})); - this.setup_search_view(action); + var searchview_loaded = this.setup_search_view(action); this.$element.find('.views-switchers button').click(function() { self.on_mode_switch($(this).data('view-type')); @@ -127,11 +136,13 @@ openerp.base.ViewManager = openerp.base.Controller.extend({ self.views[view[1]] = { view_id: view[0], controller: null }; }); - if (action['auto_search']) { - this.searchview.on_loaded.add_last(this.searchview.do_search); - } // switch to the first one in sequence - this.on_mode_switch(action.views[0][1]); + var inital_view_loaded = this.on_mode_switch(action.views[0][1]); + + if (action['auto_search']) { + $.when(searchview_loaded, inital_view_loaded) + .then(this.searchview.do_search); + } }, // create when root, also add to parent when o2m on_create: function() {