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

[FIX] mail: remove reactive from Model/store internals

Browse Source 
Internal code doesn't need to use reactive when reading store or Model,
especially not in `insert` that is not meant to access anything observed
except when actually writing records (in which case `_proxy` is used).

The `get` part of `preinsert` (and its full chain) must be kept with the
full proxy however, to ensure proper trigger of reactive callback when
returned record changes.

Part-of: odoo/odoo#147814
This commit is contained in:
Sébastien Theys
2024-01-03 16:59:13 +00:00
parent de5c0b19ab
commit 403a7c2eb7
2 changed files with 72 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/src/core/common/record.js
+55 -50
View File
@@ -38,7 +38,7 @@ function updateFields(record, vals) {
// "store[Model] =" is considered a Model.insert()
record[fieldName].insert(value);
} else {
const fieldDefinition = record._RawModel._fields.get(fieldName);
const fieldDefinition = record.Model._fields.get(fieldName);
if (!fieldDefinition || Record.isAttr(fieldDefinition)) {
updateAttr(record, fieldName, value);
} else {
@@ -54,7 +54,7 @@ function updateFields(record, vals) {
* @param {any} value
*/
function updateAttr(record, fieldName, value) {
const fieldDefinition = record._RawModel._fields.get(fieldName);
const fieldDefinition = record.Model._fields.get(fieldName);
// ensure each field write goes through the proxy exactly once to trigger reactives
const targetRecord = record._proxyUsed.has(fieldName) ? record : record._proxy;
if (
@@ -172,7 +172,7 @@ function sortRecordList(recordListFullProxy, func) {
export function makeStore(env) {
Record.UPDATE = 0;
const recordByLocalId = new Map();
const recordByLocalId = reactive(new Map());
const res = {
// fake store for now, until it becomes a model
/** @type {import("models").Store} */
@@ -193,7 +193,7 @@ export function makeStore(env) {
// work-around: make an object whose prototype is the class, so that static props become
// instance props.
/** @type {typeof Record} */
const Model = Object.assign(Object.create(OgClass), { env, store: res.store });
const Model = Object.create(OgClass);
// Produce another class with changed prototype, so that there are automatic get/set on relational fields
const Class = {
[OgClass.name]: class extends OgClass {
@@ -204,7 +204,7 @@ export function makeStore(env) {
record._proxyUsed = new Set();
record._updateFields = new Set();
record._raw = record;
record._RawModel = Model;
record.Model = Model;
const recordProxyInternal = new Proxy(record, {
get(record, name, recordFullProxy) {
recordFullProxy = record._downgradeProxy(recordFullProxy);
@@ -267,7 +267,7 @@ export function makeStore(env) {
const recordProxy = reactive(recordProxyInternal);
record._proxy = recordProxy;
if (record instanceof BaseStore) {
res.store = recordProxy;
res.store = record;
}
for (const [name, fieldDefinition] of Model._fields) {
const SYM = record[name]?.[0];
@@ -287,11 +287,7 @@ export function makeStore(env) {
owner: record,
_raw: recordList,
});
if (record instanceof BaseStore) {
recordList.store = recordProxy;
} else {
recordList.store = res.store;
}
recordList.store = res.store;
field.value = recordList;
} else {
record[name] = fieldDefinition.default;
@@ -410,7 +406,8 @@ export function makeStore(env) {
}[OgClass.name];
Object.assign(Model, {
Class,
records: JSON.parse(JSON.stringify(OgClass.records)),
env,
records: reactive({}),
_fields: new Map(),
});
Models[name] = Model;
@@ -454,15 +451,20 @@ export function makeStore(env) {
}
}
}
/**
* store/_rawStore are assigned on models at next step, but they are
* required on Store model to make the initial store insert.
*/
Object.assign(res.store.Store, { store: res.store, _rawStore: res.store });
// Make true store (as a model)
res.store = res.store.Store.insert();
const store = toRaw(res.store)._raw;
res.store = toRaw(res.store.Store.insert())._raw;
for (const Model of Object.values(Models)) {
Model.store = res.store;
res.store[Model.name] = Model;
Model._rawStore = res.store;
Model.store = res.store._proxy;
res.store._proxy[Model.name] = Model;
}
Object.assign(store, { Models, storeReady: true });
return res.store;
Object.assign(res.store, { Models, storeReady: true });
return res.store._proxy;
}
class RecordUses {
@@ -522,7 +524,7 @@ class RecordList extends Array {
data = [];
get fieldDefinition() {
return this.owner._RawModel._fields.get(this.name);
return this.owner.Model._fields.get(this.name);
}
constructor() {
@@ -572,7 +574,7 @@ class RecordList extends Array {
// support for "array[index] = r3" syntax
const index = parseInt(name);
recordList._insert(val, function recordListSet_Insert(newRecord) {
const oldRecord = toRaw(recordList.store)._raw.recordByLocalId.get(
const oldRecord = toRaw(recordList.store.recordByLocalId).get(
recordList.data[index]
);
if (oldRecord && oldRecord.notEq(newRecord)) {
@@ -1021,7 +1023,7 @@ export class Record {
static trusted = false;
static id;
/** @type {Object<string, Record>} */
static records = {};
static records;
/** @type {import("models").Store} */
static store;
/** @type {RecordField[]} */
@@ -1108,8 +1110,9 @@ export class Record {
}
for (const [localId, names] of record.__uses__.data.entries()) {
for (const [name2, count] of names.entries()) {
const usingRecordProxy =
record._RawModel.store.recordByLocalId.get(localId);
const usingRecordProxy = toRaw(
record.Model._rawStore.recordByLocalId
).get(localId);
if (!usingRecordProxy) {
// record already deleted, clean inverses
record.__uses__.data.delete(localId);
@@ -1127,7 +1130,7 @@ export class Record {
}
}
delete record.Model.records[record.localId];
record._RawModel.store.recordByLocalId.delete(record.localId);
record.Model._rawStore.recordByLocalId.delete(record.localId);
}
}
Record.UPDATE--;
@@ -1263,7 +1266,7 @@ export class Record {
*
* @type {Map<string, FieldDefinition>}
*/
static _fields = new Map();
static _fields;
static isRecord(record) {
return Boolean(record?.[IS_RECORD_SYM]);
}
@@ -1331,6 +1334,7 @@ export class Record {
return res;
}
static _retrieveIdFromData(data) {
const Model = toRaw(this);
const res = {};
function _deepRetrieve(expr2) {
if (typeof expr2 === "string") {
@@ -1359,18 +1363,18 @@ export class Record {
}
}
}
if (this.id === undefined) {
if (Model.id === undefined) {
return res;
}
if (typeof this.id === "string") {
if (typeof Model.id === "string") {
if (typeof data !== "object" || data === null) {
return { [this.id]: data }; // non-object data => single id
return { [Model.id]: data }; // non-object data => single id
}
if (Record.isCommand(data[this.id])) {
if (Record.isCommand(data[Model.id])) {
// Note: only Record.one() is supported
const [cmd, data2] = data[this.id].at(-1);
const [cmd, data2] = data[Model.id].at(-1);
return Object.assign(res, {
[this.id]:
[Model.id]:
cmd === "DELETE"
? undefined
: cmd === "DELETE.noinv"
@@ -1380,9 +1384,9 @@ export class Record {
: data2,
});
}
return { [this.id]: data[this.id] };
return { [Model.id]: data[Model.id] };
}
for (const expr of this.id) {
for (const expr of Model.id) {
if (typeof expr === "symbol") {
continue;
}
@@ -1407,12 +1411,10 @@ export class Record {
* @returns {Record}
*/
static new(data) {
const ModelProxy = this;
const Model = toRaw(ModelProxy);
const Model = toRaw(this);
return Record.MAKE_UPDATE(function RecordNew() {
const recordProxy = new Model.Class();
const record = toRaw(recordProxy)._raw;
record.Model = ModelProxy;
const ids = Model._retrieveIdFromData(data);
for (const name in ids) {
if (
@@ -1423,21 +1425,21 @@ export class Record {
) {
// preinsert that record in relational field,
// as it is required to make current local id
ids[name] = Model.store[Model._fields.get(name).targetModel].preinsert(
ids[name] = Model._rawStore[Model._fields.get(name).targetModel].preinsert(
ids[name]
);
}
}
Object.assign(record, { localId: Model.localId(ids) });
Object.assign(recordProxy, { ...ids });
ModelProxy.records[record.localId] = recordProxy;
if (record._RawModel.name === "Store") {
Model.records[record.localId] = recordProxy;
if (record.Model.name === "Store") {
Object.assign(record, {
env: Model.store.env,
recordByLocalId: Model.store.recordByLocalId,
env: Model._rawStore.env,
recordByLocalId: Model._rawStore.recordByLocalId,
});
}
Model.store.recordByLocalId.set(record.localId, recordProxy);
Model._rawStore.recordByLocalId.set(record.localId, recordProxy);
for (const field of record._fields.values()) {
field.requestCompute?.();
field.requestSort?.();
@@ -1524,7 +1526,7 @@ export class Record {
}
/** @returns {Record|Record[]} */
static insert(data, options = {}) {
const SubClass = this;
const ModelFullProxy = this;
return Record.MAKE_UPDATE(function RecordInsert() {
const isMulti = Array.isArray(data);
if (!isMulti) {
@@ -1533,7 +1535,7 @@ export class Record {
const oldTrusted = Record.trusted;
Record.trusted = options.html ?? Record.trusted;
const res = data.map(function RecordInsertMap(d) {
return SubClass._insert(d, options);
return ModelFullProxy._insert(d, options);
});
Record.trusted = oldTrusted;
if (!isMulti) {
@@ -1544,7 +1546,8 @@ export class Record {
}
/** @returns {Record} */
static _insert(data) {
const res = this.preinsert(data);
const ModelFullProxy = this;
const res = ModelFullProxy.preinsert(data);
res.update(data);
return res;
}
@@ -1553,7 +1556,9 @@ export class Record {
* @returns {Record}
*/
static preinsert(data) {
return this.get(data) ?? this.new(data);
const ModelFullProxy = this;
const Model = toRaw(ModelFullProxy);
return ModelFullProxy.get(data) ?? Model.new(data);
}
static isCommand(data) {
return ["ADD", "DELETE", "ADD.noinv", "DELETE.noinv"].includes(data?.[0]?.[0]);
@@ -1569,7 +1574,7 @@ export class Record {
_fields = new Map();
__uses__ = markRaw(new RecordUses());
get _store() {
return toRaw(this)._raw._RawModel.store;
return toRaw(this)._raw.Model._rawStore._proxy;
}
/**
* Technical attribute, contains the Model entry in the store.
@@ -1602,7 +1607,7 @@ export class Record {
updateFields(record, data);
} else {
// update on single-id data
updateFields(record, { [record._RawModel.id]: data });
updateFields(record, { [record.Model.id]: data });
}
});
}
@@ -1659,14 +1664,14 @@ export class Record {
delete data._proxyInternal;
delete data._proxyUsed;
delete data._raw;
delete data._RawModel;
delete data.Model;
delete data._updateFields;
delete data.__uses__;
delete data.Model;
return data;
}
toIdData() {
const data = this._RawModel._retrieveIdFromData(this);
const data = this.Model._retrieveIdFromData(this);
for (const [name, val] of Object.entries(data)) {
if (Record.isRecord(val)) {
data[name] = val.toIdData();
addons/mail/static/tests/core/record_tests.js
+17
View File
@@ -582,3 +582,20 @@ QUnit.test("lazy sort should re-sort while they are observed", async (assert) =>
message.sequence = 10;
assert.verifySteps(["render 2,1"]);
});
QUnit.test("store updates can be observed", async (assert) => {
const store = await start();
function onUpdate() {
assert.step(`abc:${reactiveStore.abc}`);
}
const rawStore = toRaw(store)._raw;
const reactiveStore = reactive(store, onUpdate);
onUpdate();
assert.verifySteps(["abc:undefined"]);
store.abc = 1;
assert.verifySteps(["abc:1"], "observable from makeStore");
rawStore._store.abc = 2;
assert.verifySteps(["abc:2"], "observable from record._store");
rawStore.Model.store.abc = 3;
assert.verifySteps(["abc:3"], "observable from Model.store");
});