diff options
| author | Alexandre Kühn <aku@odoo.com> | 2018-07-04 17:17:29 +0200 |
|---|---|---|
| committer | Alexandre Kühn <aku@odoo.com> | 2018-07-05 14:04:33 +0200 |
| commit | 7af09db878997d1e5af09e4a3c314e6df5311e55 (patch) | |
| tree | 278452959dc4d5e26d51c93f705aee13e3a8616c /addons/web/static/src | |
| parent | b27d009ab473a45312a7216feb85a6cc35d33062 (diff) | |
[FIX] web: no concurrent reload on basic model
Before this commit, there was a traceback in the following scenario:
Let's have a kanban view with groupby and a domain.
If we clear the domain then clear the groupby, there could be a crash.
This crash occurs because when we clear the groupby, it performs a `reload`,
and assumes that the datapoint contains grouped data. However, if this
computation takes some time, it is possible to overwrite the datapoint so that
there is no longer grouped data.
This commit fixes the issue by enforcing at most 1 execution of the method
`reload` active at any time. Consequently, successive reloads operations are
now much slower.
Note that it uses the single non-reentrant mutex on the basic_model to enforce
this rule, which means:
- No more concurrent `reload` and `save`.
- If an execution having the mutex in `reload` come accross the same mutex,
it is blocked forever. This is hopefully not the case at the moment,
however a reentrant mutex should be considered in the future.
Diffstat (limited to 'addons/web/static/src')
| -rw-r--r-- | addons/web/static/src/js/views/basic/basic_model.js | 132 |
1 files changed, 73 insertions, 59 deletions
diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index 82bb9e586a8..32fb23d983a 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -814,7 +814,8 @@ var BasicModel = AbstractModel.extend({ return this.mutex.exec(this._applyChange.bind(this, record_id, changes, options)); }, /** - * Reload all data for a given resource + * Reload all data for a given resource. At any time there is at most one + * reload operation active. * * @param {string} id local id for a resource * @param {Object} [options] @@ -823,64 +824,7 @@ var BasicModel = AbstractModel.extend({ * @returns {Deferred<string>} resolves to the id of the resource */ reload: function (id, options) { - options = options || {}; - var element = this.localData[id]; - - if (element.type === 'record') { - if (!options.currentId && (('currentId' in options) || this.isNew(id))) { - var params = { - context: element.context, - fieldsInfo: element.fieldsInfo, - fields: element.fields, - viewType: element.viewType, - }; - return this._makeDefaultRecord(element.model, params); - } - if (!options.keepChanges) { - this.discardChanges(id, {rollback: false}); - } - } else if (element._changes) { - delete element.tempLimitIncrement; - _.each(element._changes, function (change) { - delete change.isNew; - }); - } - - if (options.context !== undefined) { - element.context = options.context; - } - if (options.domain !== undefined) { - element.domain = options.domain; - } - if (options.groupBy !== undefined) { - element.groupedBy = options.groupBy; - } - if (options.limit !== undefined) { - element.limit = options.limit; - } - if (options.offset !== undefined) { - this._setOffset(element.id, options.offset); - } - if (options.loadMoreOffset !== undefined) { - element.loadMoreOffset = options.loadMoreOffset; - } else { - // reset if not specified - element.loadMoreOffset = 0; - } - if (options.currentId !== undefined) { - element.res_id = options.currentId; - } - if (options.ids !== undefined) { - element.res_ids = options.ids; - element.count = element.res_ids.length; - } - if (element.type === 'record') { - element.offset = _.indexOf(element.res_ids, element.res_id); - } - var loadOptions = _.pick(options, 'fieldNames', 'viewType'); - return this._load(element, loadOptions).then(function (result) { - return result.id; - }); + return this.mutex.exec(this._reload.bind(this, id, options)); }, /** * In some case, we may need to remove an element from a list, without going @@ -4009,6 +3953,76 @@ var BasicModel = AbstractModel.extend({ }); }, /** + * Reload all data for a given resource + * + * @private + * @param {string} id local id for a resource + * @param {Object} [options] + * @param {boolean} [options.keepChanges=false] if true, doesn't discard the + * changes on the record before reloading it + * @returns {Deferred<string>} resolves to the id of the resource + */ + _reload: function (id, options) { + options = options || {}; + var element = this.localData[id]; + + if (element.type === 'record') { + if (!options.currentId && (('currentId' in options) || this.isNew(id))) { + var params = { + context: element.context, + fieldsInfo: element.fieldsInfo, + fields: element.fields, + viewType: element.viewType, + }; + return this._makeDefaultRecord(element.model, params); + } + if (!options.keepChanges) { + this.discardChanges(id, {rollback: false}); + } + } else if (element._changes) { + delete element.tempLimitIncrement; + _.each(element._changes, function (change) { + delete change.isNew; + }); + } + + if (options.context !== undefined) { + element.context = options.context; + } + if (options.domain !== undefined) { + element.domain = options.domain; + } + if (options.groupBy !== undefined) { + element.groupedBy = options.groupBy; + } + if (options.limit !== undefined) { + element.limit = options.limit; + } + if (options.offset !== undefined) { + this._setOffset(element.id, options.offset); + } + if (options.loadMoreOffset !== undefined) { + element.loadMoreOffset = options.loadMoreOffset; + } else { + // reset if not specified + element.loadMoreOffset = 0; + } + if (options.currentId !== undefined) { + element.res_id = options.currentId; + } + if (options.ids !== undefined) { + element.res_ids = options.ids; + element.count = element.res_ids.length; + } + if (element.type === 'record') { + element.offset = _.indexOf(element.res_ids, element.res_id); + } + var loadOptions = _.pick(options, 'fieldNames', 'viewType'); + return this._load(element, loadOptions).then(function (result) { + return result.id; + }); + }, + /** * Allows to save a value in the specialData cache associated to a given * record and fieldName. If the value in the cache was already the given * one, nothing is done and the method indicates it by returning false |
