From 2679fdd9b9740db27df87bd38817ad839f4778a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 25 Apr 2017 09:32:27 +0200
Subject: [FIX] web: add mutex in basic controller

Before this commit, concurrency issues were mostly handled by the mutex
in the basic model.  That mutex makes sure that save and change
operations are properly serialized.  However, this is not enough. Here
is an example:

1. we have a required field with an onchange and a current value
2. the user changes the value to an empty value. This triggers an
   onchange
3. the user clicks on save, before the return of the onchange
4. the controller checks with the renderer if it can be saved.  At that
   point, the onchange is not yet complete, so the values stored in the
   fields are still the initial ones, which are valid (non empty value
   for required fields)
5. so, the controller request a save operation
6. the model add the save operation in the mutex, wait for the onchange
   to complete, then apply the save operation
7. we end up with a required field with an empty value

The issue here is that the model properly saves and apply onchanges, but
the responsability to check if a record is valid (which depends on the
modifiers AND the field widgets) is the renderer's responsability (it
has to ask the question to all its sub field widgets).

So, we need to serialize 3 operations: applying the onchange, checking
if the save is valid, and saving the record.  This cannot be only in the
model, so we need to add a concurrency control mechanism somewhere else.
What we do in this commit is to add a mutex on the basic controller (it
will most likely be used by the list controller as well).  In this
mutex, we currently serialize the field change operation and the save
operation, but I assume some other operations will need to be handled by
the mutex.

In addition to this, we also change some asynchronous methods to return
a deferred, so we are ready for other kind of asynchronous behaviour in
the future.
---
 .../web/static/src/js/views/basic/basic_controller.js | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

(limited to 'addons/web/static/src/js/views/basic/basic_controller.js')

diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js
index 7beb15a9d70..97e0ca0f4be 100644
--- a/addons/web/static/src/js/views/basic/basic_controller.js
+++ b/addons/web/static/src/js/views/basic/basic_controller.js
@@ -8,9 +8,10 @@ odoo.define('web.BasicController', function (require) {
  */
 
 var AbstractController = require('web.AbstractController');
-var FieldManagerMixin = require('web.FieldManagerMixin');
+var concurrency = require('web.concurrency');
 var core = require('web.core');
 var Dialog = require('web.Dialog');
+var FieldManagerMixin = require('web.FieldManagerMixin');
 var Pager = require('web.Pager');
 
 var _t = core._t;
@@ -34,6 +35,7 @@ return AbstractController.extend(FieldManagerMixin, {
         FieldManagerMixin.init.call(this, this.model);
         this.handle = params.initialState.id;
         this.isDirty = false;
+        this.mutex = new concurrency.Mutex();
     },
     /**
      * @override
@@ -87,6 +89,18 @@ return AbstractController.extend(FieldManagerMixin, {
     // Private
     //--------------------------------------------------------------------------
 
+    /**
+     * We override applyChanges (from the field manager mixin) to protect it
+     * with a mutex.
+     *
+     * @override
+     */
+    _applyChanges: function (dataPointID, changes, event) {
+        var _super = FieldManagerMixin._applyChanges.bind(this);
+        return this.mutex.exec(function () {
+            return _super(dataPointID, changes, event);
+        });
+    },
     /**
      * When the user clicks on a 'action button', this function determines what
      * should happen.
@@ -132,10 +146,11 @@ return AbstractController.extend(FieldManagerMixin, {
      * @param {string} id - the id of one of the view's records
      * @param {string[]} fields - the changed fields
      * @param {OdooEvent} e - the event that triggered the change
+     * @returns {Deferred}
      */
     _confirmChange: function (id, fields, e) {
         var state = this.model.get(this.handle);
-        this.renderer.confirmChange(state, id, fields, e);
+        return this.renderer.confirmChange(state, id, fields, e);
     },
     /**
      * Delete records (and ask for confirmation if necessary)
-- 
cgit v1.2.3