pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: fix concurrency issue in graph

Browse Source 
Let's assume the following scenario in a graph view
 - toggle a filter on a slow network s.t. it takes a while to load
 - while it's loading, toggle a measure

Before this commit, the view was kind of locked after those two
steps, i.e. toggling measures or changing graph mode did not work
anymore (it could be unlocked by interacting again with the search
view though). The reason is that the new filter triggered a
re-rendering of the withSearch component (the parent of the graph
view), thus calling willUpdateProps on the graph view, which reloads
the data. Toggling a measure cancels the current load operation
(via keepLast), and the promise returned by willUpdateProps is thus
pending forever. From that point, only another re-rendering from the
parent (withSearch) could unlock the situation by cancelling the
previous rendering.

To solve this, we use the new Race primitive. It ensures that the
promise returned by load resolves as soon as the view is actually
reloaded (whether it came from the load itself, of by toggling a
measure).

This commit also refactors the pivot model to use the Race
primitive.

X-original-commit: ceebab7128cb9838024029aefe3910a18cf14e44
Part-of: odoo/odoo#76778
This commit is contained in:
Aaron Bohy
2021-09-17 23:59:34 +00:00
parent cc9c229fda
commit 040c73fe69
3 changed files with 158 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/views/graph/graph_model.js
+13 -12
View File
@@ -1,7 +1,7 @@
/** @odoo-module **/
import { sortBy } from "@web/core/utils/arrays";
import { KeepLast } from "@web/core/utils/concurrency";
import { KeepLast, Race } from "@web/core/utils/concurrency";
import { rankInterval } from "@web/search/utils/dates";
import { getGroupBy } from "@web/search/utils/group_by";
import { GROUPABLE_TYPES } from "@web/search/utils/misc";
@@ -85,7 +85,13 @@ export class GraphModel extends Model {
* @override
*/
setup(params) {
// concurrency management
this.keepLast = new KeepLast();
this.race = new Race();
const _fetchDataPoints = this._fetchDataPoints.bind(this);
this._fetchDataPoints = (...args) => {
return this.race.add(_fetchDataPoints(...args));
};
this.initialGroupBy = null;
@@ -126,9 +132,7 @@ export class GraphModel extends Model {
metaData.additionalMeasures
);
await this._fetchDataPoints(metaData);
this._prepareData();
return this._fetchDataPoints(metaData);
}
/**
@@ -144,17 +148,13 @@ export class GraphModel extends Model {
* @param {Object} params
*/
async updateMetaData(params) {
const metaData = Object.assign({}, this.metaData, params);
if ("measure" in params) {
await this._fetchDataPoints(metaData);
await this._fetchDataPoints(Object.assign({}, this.metaData, params));
} else {
await this.keepLast.add(Promise.resolve());
this.metaData = metaData;
await this.race.getCurrentProm();
this.metaData = Object.assign({}, this.metaData, params);
this._prepareData();
}
this._prepareData();
this.notify();
}
@@ -171,6 +171,7 @@ export class GraphModel extends Model {
async _fetchDataPoints(metaData) {
this.dataPoints = await this.keepLast.add(this._loadDataPoints(metaData));
this.metaData = metaData;
this._prepareData();
}
/**
addons/web/static/src/views/pivot/pivot_model.js
+30 -44
View File
@@ -2,7 +2,7 @@
import { Domain } from "@web/core/domain";
import { cartesian, sections, sortBy, symmetricalDifference } from "@web/core/utils/arrays";
import { KeepLast } from "@web/core/utils/concurrency";
import { KeepLast, Race } from "@web/core/utils/concurrency";
import { computeVariation } from "@web/core/utils/numbers";
import { DEFAULT_INTERVAL } from "@web/search/utils/dates";
import { Model } from "@web/views/helpers/model";
@@ -338,7 +338,13 @@ export class PivotModel extends Model {
* @param {Object} [params.data] previously exported data
*/
setup(params) {
// concurrency management
this.keepLast = new KeepLast();
this.race = new Race();
const _loadData = this._loadData.bind(this);
this._loadData = (...args) => {
return this.race.add(_loadData(...args));
};
let sortedColumn = params.metaData.sortedColumn || null;
if (!sortedColumn && params.metaData.defaultOrder) {
@@ -373,9 +379,6 @@ export class PivotModel extends Model {
this.metaData = this._buildMetaData(metaData);
this.reload = false; // used to discriminate between the first load and subsequent reloads
// loadProm ensures that loadData always returns a promise that resolves when
// the final data is loaded (in case several load requests arrive consecutively)
this.loadProm = null;
this.nextActiveMeasures = null; // allows to toggle several measures consecutively
}
@@ -394,7 +397,7 @@ export class PivotModel extends Model {
* @param {string} [params.interval]
*/
async addGroupBy(params) {
if (this.loadProm) {
if (this.race.getCurrentProm()) {
return; // we are currently reloaded the table
}
@@ -435,7 +438,7 @@ export class PivotModel extends Model {
* @param {'row'|'col'} type
*/
closeGroup(groupId, type) {
if (this.loadProm) {
if (this.race.getCurrentProm()) {
return; // we are currently reloading the table
}
@@ -507,7 +510,7 @@ export class PivotModel extends Model {
* @param {'row'|'col'} type
*/
async expandGroup(groupId, type) {
if (this.loadProm) {
if (this.race.getCurrentProm()) {
return; // we are currently reloaded the table
}
@@ -598,7 +601,7 @@ export class PivotModel extends Model {
* flipping the axes. This method is thus async.
*/
async flip() {
await Promise.resolve(this.loadProm);
await this.race.getCurrentProm();
// swap the data: the main column and the main row
let temp = this.data.rowGroupTree;
@@ -719,7 +722,7 @@ export class PivotModel extends Model {
* @param {number[]} sortedColumn.groupId
*/
sortRows(sortedColumn) {
if (this.loadProm) {
if (this.race.getCurrentProm()) {
return; // we are currently reloaded the table
}
@@ -745,7 +748,7 @@ export class PivotModel extends Model {
// actually reload a lesser amount of information (but still, we need
// to wait in case there is a pending load)
metaData.activeMeasures.splice(index, 1);
await Promise.resolve(this.loadProm);
await Promise.resolve(this.race.getCurrentProm());
this.metaData = metaData;
} else {
metaData.activeMeasures.push(fieldName);
@@ -1347,15 +1350,6 @@ export class PivotModel extends Model {
* @param {Config} config
*/
async _loadData(config, prune = true) {
if (!this.loadProm) {
this.loadProm = new Promise((resolve) => {
this.resolveLoadProm = () => {
this.loadProm = null;
resolve();
};
});
}
config.data = {}; // data will be completely recomputed
const { data, metaData } = config;
data.rowGroupTree = { root: { labels: [], values: [] }, directSubTrees: new Map() };
@@ -1372,33 +1366,25 @@ export class PivotModel extends Model {
const rightDivisors = sections(metaData.fullColGroupBys);
const divisors = cartesian(leftDivisors, rightDivisors);
this._subdivideGroup(group, divisors.slice(0, 1), config)
.then(() => {
return this._subdivideGroup(group, divisors.slice(1), config);
})
.then(async () => {
// keep folded groups folded after the reload if the structure of the table is the same
if (prune && this._hasData(data) && this._hasData(this.data)) {
if (
symmetricalDifference(metaData.rowGroupBys, this.metaData.rowGroupBys)
.length === 0
) {
this._pruneTree(data.rowGroupTree, this.data.rowGroupTree);
}
if (
symmetricalDifference(metaData.colGroupBys, this.metaData.colGroupBys)
.length === 0
) {
this._pruneTree(data.colGroupTree, this.data.colGroupTree);
}
}
await this._subdivideGroup(group, divisors.slice(0, 1), config);
await this._subdivideGroup(group, divisors.slice(1), config);
this.data = config.data;
this.metaData = config.metaData;
this.resolveLoadProm();
});
// keep folded groups folded after the reload if the structure of the table is the same
if (prune && this._hasData(data) && this._hasData(this.data)) {
if (
symmetricalDifference(metaData.rowGroupBys, this.metaData.rowGroupBys).length === 0
) {
this._pruneTree(data.rowGroupTree, this.data.rowGroupTree);
}
if (
symmetricalDifference(metaData.colGroupBys, this.metaData.colGroupBys).length === 0
) {
this._pruneTree(data.colGroupTree, this.data.colGroupTree);
}
}
return this.loadProm;
this.data = config.data;
this.metaData = config.metaData;
}
/**
* Extract the information in the read_group results (groupSubdivisions)
addons/web/static/tests/views/graph_view_tests.js
+115 -1
View File
@@ -1,7 +1,7 @@
/** @odoo-module **/
import { BORDER_WHITE, DEFAULT_BG } from "@web/views/graph/colors";
import { click, triggerEvent } from "@web/../tests/helpers/utils";
import { click, makeDeferred, nextTick, triggerEvent } from "@web/../tests/helpers/utils";
import { createWebClient, doAction } from "@web/../tests/webclient/helpers";
import { dialogService } from "@web/core/dialog/dialog_service";
import { GraphArchParser } from "@web/views/graph/graph_arch_parser";
@@ -3308,4 +3308,118 @@ QUnit.module("Views", (hooks) => {
]);
}
);
QUnit.test(
"concurrent reloads: add a filter, and directly toggle a measure",
async function (assert) {
let def;
const graph = await makeView({
serverData,
type: "graph",
resModel: "foo",
arch: `
<graph type="line">
<field name="product_id"/>
</graph>`,
searchViewArch: `
<search>
<filter name="my_filter" string="My Filter" domain="[('id', '&lt;', 2)]"/>
</search>`,
mockRPC: function (route, args) {
if (args.method === "web_read_group") {
return Promise.resolve(def);
}
},
});
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
// Set a domain (this reload is delayed)
def = makeDeferred();
await toggleFilterMenu(graph);
await toggleMenuItem(graph, "My Filter");
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
// Toggle a measure
await toggleMenu(graph, "Measures");
await toggleMenuItem(graph, "Foo");
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
def.resolve();
await nextTick();
checkDatasets(assert, graph, ["data", "label"], {
data: [82, 157],
label: "Foo",
});
}
);
QUnit.test("change graph mode while loading a filter", async function (assert) {
let def;
const graph = await makeView({
serverData,
type: "graph",
resModel: "foo",
arch: `
<graph type="line">
<field name="product_id"/>
</graph>`,
searchViewArch: `
<search>
<filter name="my_filter" string="My Filter" domain="[('id', '&lt;', 2)]"/>
</search>`,
mockRPC: function (route, args) {
if (args.method === "web_read_group") {
return Promise.resolve(def);
}
},
});
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
checkModeIs(assert, graph, "line");
// Set a domain (this reload is delayed)
def = makeDeferred();
await toggleFilterMenu(graph);
await toggleMenuItem(graph, "My Filter");
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
checkModeIs(assert, graph, "line");
// Change graph mode
await selectMode(graph, "bar");
checkDatasets(assert, graph, ["data", "label"], {
data: [4, 4],
label: "Count",
});
checkModeIs(assert, graph, "line");
def.resolve();
await nextTick();
checkDatasets(assert, graph, ["data", "label"], {
data: [1],
label: "Count",
});
checkModeIs(assert, graph, "bar");
});
});