[FIX] web: batch read of m2m in o2m after onchange
Let's assume a one2many field (editable list) in a form view with a many2many (e.g. many2many_tags), and an onchange on the one2many such that, when a row is added to the relation, the server returns update commands for (a subset of) the records being already in the relation. For thoses updated records, the many2many field needs to be read (the onchange only returns the ids in the relation). Before this rev., the many2many field was read independently for each record in the one2many. This could cause a performance issue on large relations. For instance, this was the case on account.invoice records with a lot of lines. Issue 2027356 closes odoo/odoo#34785 Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
This commit is contained in:
@@ -155,6 +155,10 @@ var BasicModel = AbstractModel.extend({
|
||||
// save is performed.
|
||||
this.mutex = new concurrency.Mutex();
|
||||
|
||||
// this array is used to accumulate RPC requests done in the same call
|
||||
// stack, so that they can be batched in the minimum number of RPCs
|
||||
this.batchedRPCsRequests = [];
|
||||
|
||||
this.localData = Object.create(null);
|
||||
this._super.apply(this, arguments);
|
||||
},
|
||||
@@ -3990,6 +3994,86 @@ var BasicModel = AbstractModel.extend({
|
||||
});
|
||||
});
|
||||
},
|
||||
/**
|
||||
* This function accumulates RPC requests done in the same call stack, and
|
||||
* performs them in the next micro task tick so that similar requests can be
|
||||
* batched in a single RPC.
|
||||
*
|
||||
* For now, only 'read' calls are supported.
|
||||
*
|
||||
* @private
|
||||
* @param {Object} params
|
||||
* @returns {Promise}
|
||||
*/
|
||||
_performRPC: function (params) {
|
||||
var self = this;
|
||||
|
||||
// save the RPC request
|
||||
var request = _.extend({}, params);
|
||||
var prom = new Promise(function (resolve, reject) {
|
||||
request.resolve = resolve;
|
||||
request.reject = reject;
|
||||
});
|
||||
this.batchedRPCsRequests.push(request);
|
||||
|
||||
// empty the pool of RPC requests in the next micro tick
|
||||
Promise.resolve().then(function () {
|
||||
if (!self.batchedRPCsRequests.length) {
|
||||
// pool has already been processed
|
||||
return;
|
||||
}
|
||||
|
||||
// reset pool of RPC requests
|
||||
var batchedRPCsRequests = self.batchedRPCsRequests;
|
||||
self.batchedRPCsRequests = [];
|
||||
|
||||
// batch similar requests
|
||||
var batches = {};
|
||||
var key;
|
||||
for (var i = 0; i < batchedRPCsRequests.length; i++) {
|
||||
var request = batchedRPCsRequests[i];
|
||||
key = request.model + ',' + JSON.stringify(request.context);
|
||||
if (!batches[key]) {
|
||||
batches[key] = _.extend({}, request, {requests: [request]});
|
||||
} else {
|
||||
batches[key].ids = _.uniq(batches[key].ids.concat(request.ids));
|
||||
batches[key].fieldNames = _.uniq(batches[key].fieldNames.concat(request.fieldNames));
|
||||
batches[key].requests.push(request);
|
||||
}
|
||||
}
|
||||
|
||||
// perform batched RPCs
|
||||
function onSuccess(batch, results) {
|
||||
for (var i = 0; i < batch.requests.length; i++) {
|
||||
var request = batch.requests[i];
|
||||
var fieldNames = request.fieldNames.concat(['id']);
|
||||
var filteredResults = results.filter(function (record) {
|
||||
return request.ids.indexOf(record.id) >= 0;
|
||||
}).map(function (record) {
|
||||
return _.pick(record, fieldNames);
|
||||
});
|
||||
request.resolve(filteredResults);
|
||||
}
|
||||
}
|
||||
function onFailure(batch, error) {
|
||||
for (var i = 0; i < batch.requests.length; i++) {
|
||||
var request = batch.requests[i];
|
||||
request.reject(error);
|
||||
}
|
||||
}
|
||||
for (key in batches) {
|
||||
var batch = batches[key];
|
||||
self._rpc({
|
||||
model: batch.model,
|
||||
method: 'read',
|
||||
args: [batch.ids, batch.fieldNames],
|
||||
context: batch.context,
|
||||
}).then(onSuccess.bind(null, batch)).guardedCatch(onFailure.bind(null, batch));
|
||||
}
|
||||
});
|
||||
|
||||
return prom;
|
||||
},
|
||||
/**
|
||||
* Once a record is created and some data has been fetched, we need to do
|
||||
* quite a lot of computations to determine what needs to be fetched. This
|
||||
@@ -4197,11 +4281,12 @@ var BasicModel = AbstractModel.extend({
|
||||
|
||||
var def;
|
||||
if (missingIDs.length && fieldNames.length) {
|
||||
def = self._rpc({
|
||||
model: list.model,
|
||||
method: 'read',
|
||||
args: [missingIDs, fieldNames],
|
||||
def = self._performRPC({
|
||||
context: list.getContext(),
|
||||
fieldNames: fieldNames,
|
||||
ids: missingIDs,
|
||||
method: 'read',
|
||||
model: list.model,
|
||||
});
|
||||
} else {
|
||||
def = Promise.resolve(_.map(missingIDs, function (id) {
|
||||
|
||||
@@ -2,7 +2,6 @@ odoo.define('web.field_one_to_many_tests', function (require) {
|
||||
"use strict";
|
||||
|
||||
var AbstractField = require('web.AbstractField');
|
||||
var concurrency = require('web.concurrency');
|
||||
var FormView = require('web.FormView');
|
||||
var KanbanRecord = require('web.KanbanRecord');
|
||||
var ListRenderer = require('web.ListRenderer');
|
||||
@@ -8344,6 +8343,57 @@ QUnit.module('fields', {}, function () {
|
||||
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('many2manys inside a one2many are fetched in batch after onchange', async function (assert) {
|
||||
assert.expect(7);
|
||||
|
||||
this.data.partner.onchanges = {
|
||||
turtles: function (obj) {
|
||||
obj.turtles = [
|
||||
[5],
|
||||
[1, 1, {
|
||||
turtle_foo: "leonardo",
|
||||
partner_ids: [[4, 2]],
|
||||
}],
|
||||
[1, 2, {
|
||||
turtle_foo: "donatello",
|
||||
partner_ids: [[4, 2], [4, 4]],
|
||||
}],
|
||||
];
|
||||
},
|
||||
};
|
||||
|
||||
var form = await createView({
|
||||
View: FormView,
|
||||
model: 'partner',
|
||||
data: this.data,
|
||||
arch: '<form>' +
|
||||
'<field name="turtles">' +
|
||||
'<tree editable="bottom">' +
|
||||
'<field name="turtle_foo"/>' +
|
||||
'<field name="partner_ids" widget="many2many_tags"/>' +
|
||||
'</tree>' +
|
||||
'</field>' +
|
||||
'</form>',
|
||||
enableBasicModelBachedRPCs: true,
|
||||
mockRPC: function (route, args) {
|
||||
assert.step(args.method || route);
|
||||
if (args.method === 'read') {
|
||||
assert.deepEqual(args.args[0], [2, 4],
|
||||
'should read the partner_ids once, batched');
|
||||
}
|
||||
return this._super.apply(this, arguments);
|
||||
},
|
||||
});
|
||||
|
||||
assert.containsN(form, '.o_data_row', 2);
|
||||
assert.strictEqual(form.$('.o_field_widget[name="partner_ids"]').text().replace(/\s/g, ''),
|
||||
"secondrecordsecondrecordaaa");
|
||||
|
||||
assert.verifySteps(['default_get', 'onchange', 'read']);
|
||||
|
||||
form.destroy();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user