From 0390d765069ce9048ec5352eaa889b424510e618 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= <lul@odoo.com>
Date: Wed, 4 Jan 2023 15:35:59 +0000
Subject: [PATCH] [FIX] spreadsheet: fix concurrency issues in data sources
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. Insert a pivot in a spreadsheet
2. Create a global filter on a field of the pivot
3. With a slow network, change the value of the global filter quickly
   twice.

Two concurrent requests are sent to the server.
When the first one resolves, it marks the data source as loaded.
However, at this point, the model (`this._model`) is no longer the one
created by the first request. It's the one created by the second request
which is still loading and has no data.

We end up with a data source which is marked as loaded but has no data.

Task 3120203

closes odoo/odoo#109118

X-original-commit: 7cf02cedac189729af732faae02ec6195af26e5c
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
---
 .../static/src/data_sources/data_source.js    | 15 ++++--
 .../data_sources/odoo_views_data_source.js    |  7 ---
 .../tests/data_fetching/data_source_test.js   | 46 +++++++++++++++++++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 addons/spreadsheet/static/tests/data_fetching/data_source_test.js

diff --git a/addons/spreadsheet/static/src/data_sources/data_source.js b/addons/spreadsheet/static/src/data_sources/data_source.js
index fb776d75044..f6142e96599 100644
--- a/addons/spreadsheet/static/src/data_sources/data_source.js
+++ b/addons/spreadsheet/static/src/data_sources/data_source.js
@@ -1,6 +1,7 @@
 /** @odoo-module */
 
 import { LoadingDataError } from "@spreadsheet/o_spreadsheet/errors";
+import { KeepLast } from "@web/core/utils/concurrency";
 
 /**
  * DataSource is an abstract class that contains the logic of fetching and
@@ -23,6 +24,7 @@ export class LoadableDataSource {
          */
         this._lastUpdate = undefined;
 
+        this._concurrency = new KeepLast();
         /**
          * Promise to control the loading of data
          */
@@ -32,8 +34,8 @@ export class LoadableDataSource {
 
     /**
      * Load data in the model
-     * @param {Object} params Params for fetching data
-     * @param {boolean=false} params.reload Force the reload of the data
+     * @param {object} [params] Params for fetching data
+     * @param {boolean} [params.reload=false] Force the reload of the data
      *
      * @returns {Promise} Resolved when data are fetched.
      */
@@ -43,7 +45,7 @@ export class LoadableDataSource {
         }
         if (!this._loadPromise) {
             this._isFullyLoaded = false;
-            this._loadPromise = this._load().then(() => {
+            this._loadPromise = this._concurrency.add(this._load()).then(() => {
                 this._lastUpdate = Date.now();
                 this._isFullyLoaded = true;
                 this._notify();
@@ -56,6 +58,13 @@ export class LoadableDataSource {
         return this._lastUpdate;
     }
 
+    /**
+     * @returns {boolean}
+     */
+    isReady() {
+        return this._isFullyLoaded;
+    }
+
     /**
      * @protected
      */
diff --git a/addons/spreadsheet/static/src/data_sources/odoo_views_data_source.js b/addons/spreadsheet/static/src/data_sources/odoo_views_data_source.js
index 439bad81468..454ce332950 100644
--- a/addons/spreadsheet/static/src/data_sources/odoo_views_data_source.js
+++ b/addons/spreadsheet/static/src/data_sources/odoo_views_data_source.js
@@ -80,13 +80,6 @@ export class OdooViewsDataSource extends LoadableDataSource {
         await this.loadMetadata();
     }
 
-    /**
-     * @returns {boolean}
-     */
-    isReady() {
-        return this._isFullyLoaded;
-    }
-
     isMetaDataLoaded() {
         return this._metaData.fields !== undefined;
     }
diff --git a/addons/spreadsheet/static/tests/data_fetching/data_source_test.js b/addons/spreadsheet/static/tests/data_fetching/data_source_test.js
new file mode 100644
index 00000000000..6c5cfbd4b6e
--- /dev/null
+++ b/addons/spreadsheet/static/tests/data_fetching/data_source_test.js
@@ -0,0 +1,46 @@
+/** @odoo-module */
+
+import { nextTick } from "@web/../tests/helpers/utils";
+import { LoadableDataSource } from "@spreadsheet/data_sources/data_source";
+import { Deferred } from "@web/core/utils/concurrency";
+
+QUnit.module("spreadsheet data source", {}, () => {
+    QUnit.test(
+        "data source is ready after all concurrent requests are resolved",
+        async (assert) => {
+            const def1 = new Deferred();
+            const def2 = new Deferred();
+            let req = 0;
+            class TestDataSource extends LoadableDataSource {
+                constructor() {
+                    super(...arguments);
+                    this.data = null;
+                }
+                async _load() {
+                    this.data = null;
+                    switch (++req) {
+                        case 1:
+                            await def1;
+                            break;
+                        case 2:
+                            await def2;
+                            break;
+                    }
+                    this.data = "something";
+                }
+            }
+            const dataSource = new TestDataSource({
+                notify: () => {},
+            });
+            dataSource.load();
+            dataSource.load({ reload: true });
+            assert.strictEqual(dataSource.isReady(), false);
+            def1.resolve();
+            await nextTick();
+            assert.strictEqual(dataSource.isReady(), false);
+            def2.resolve();
+            await nextTick();
+            assert.strictEqual(dataSource.isReady(), true);
+        }
+    );
+});