From bccfb6e5d09f7792a36cb40ab49a3e76f4df4467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Wed, 18 Aug 2021 19:39:47 +0000 Subject: [PATCH] [IMP] mail: remove modelManager and messagingCreatedPromise from env Part of task-2582313 --- .../component_hooks/use_models/use_models.js | 19 +++-- .../component_hooks/use_update/use_update.js | 13 +-- .../chatter_container/chatter_container.js | 5 +- addons/mail/static/src/env/test_env.js | 10 +-- addons/mail/static/src/main.js | 22 ++--- addons/mail/static/src/model/model_manager.js | 82 +++++++++++++++---- addons/mail/static/src/models/model/model.js | 72 ++++++++++++++-- .../services/dialog_service/dialog_service.js | 2 - .../src/services/messaging/messaging.js | 50 ++--------- .../new_message_service.js | 12 +-- addons/mail/static/src/utils/test_utils.js | 59 +++++++------ .../src/webclient/commands/mail_providers.js | 12 +-- .../static/src/widgets/discuss/discuss.js | 2 +- 13 files changed, 202 insertions(+), 158 deletions(-) diff --git a/addons/mail/static/src/component_hooks/use_models/use_models.js b/addons/mail/static/src/component_hooks/use_models/use_models.js index 61db803e5b4..ceaa809b23d 100644 --- a/addons/mail/static/src/component_hooks/use_models/use_models.js +++ b/addons/mail/static/src/component_hooks/use_models/use_models.js @@ -10,27 +10,28 @@ const { useComponent } = owl.hooks; */ export function useModels() { const component = useComponent(); + const { modelManager } = component.env.services.messaging; const listener = new Listener({ onChange: () => component.render(), }); const __render = component.__render; component.__render = fiber => { - if (component.env.modelManager) { - component.env.modelManager.startListening(listener); + if (modelManager) { + modelManager.startListening(listener); } __render.call(component, fiber); - if (component.env.modelManager) { - component.env.modelManager.stopListening(listener); + if (modelManager) { + modelManager.stopListening(listener); } }; const __destroy = component.__destroy; component.__destroy = parent => { - if (component.env.modelManager) { - component.env.modelManager.removeListener(listener); + if (modelManager) { + modelManager.removeListener(listener); } __destroy.call(component, parent); }; - if (!component.env.messaging) { - component.env.messagingCreatedPromise.then(() => component.render()); - } + modelManager.messagingCreatedPromise.then(() => { + component.render(); + }); } diff --git a/addons/mail/static/src/component_hooks/use_update/use_update.js b/addons/mail/static/src/component_hooks/use_update/use_update.js index 61cf52eb375..caf4c85f8b2 100644 --- a/addons/mail/static/src/component_hooks/use_update/use_update.js +++ b/addons/mail/static/src/component_hooks/use_update/use_update.js @@ -12,24 +12,25 @@ const { onMounted, onPatched, useComponent } = owl.hooks; */ export function useUpdate({ func }) { const component = useComponent(); + const { modelManager } = component.env.services.messaging; const listener = new Listener({ onChange: () => component.render(), }); function onUpdate() { - if (component.env.modelManager) { - component.env.modelManager.startListening(listener); + if (modelManager) { + modelManager.startListening(listener); } func(); - if (component.env.modelManager) { - component.env.modelManager.stopListening(listener); + if (modelManager) { + modelManager.stopListening(listener); } } onMounted(onUpdate); onPatched(onUpdate); const __destroy = component.__destroy; component.__destroy = parent => { - if (component.env.modelManager) { - component.env.modelManager.removeListener(listener); + if (modelManager) { + modelManager.removeListener(listener); } __destroy.call(component, parent); }; diff --git a/addons/mail/static/src/components/chatter_container/chatter_container.js b/addons/mail/static/src/components/chatter_container/chatter_container.js index 07850f4157a..baa2107a243 100644 --- a/addons/mail/static/src/components/chatter_container/chatter_container.js +++ b/addons/mail/static/src/components/chatter_container/chatter_container.js @@ -50,8 +50,7 @@ export class ChatterContainer extends Component { * @private */ async _insertFromProps(props) { - await this.env.messagingCreatedPromise; - await this.env.messaging.initializedPromise; + const messaging = await this.env.services.messaging.modelManager.getMessaging(); if (this.__owl__.status === 5 /* destroyed */) { return; } @@ -60,7 +59,7 @@ export class ChatterContainer extends Component { values.threadId = clear(); } if (!this.chatter) { - this.chatter = this.env.models['mail.chatter'].create(values); + this.chatter = messaging.models['mail.chatter'].create(values); } else { this.chatter.update(values); } diff --git a/addons/mail/static/src/env/test_env.js b/addons/mail/static/src/env/test_env.js index 8b565c05a75..b095fc8b3b8 100644 --- a/addons/mail/static/src/env/test_env.js +++ b/addons/mail/static/src/env/test_env.js @@ -1,6 +1,5 @@ /** @odoo-module **/ -import { makeDeferred } from '@mail/utils/deferred/deferred'; import { nextTick } from '@mail/utils/utils'; /** @@ -24,12 +23,11 @@ export function addMessagingToEnv(providedEnv = {}) { }, (env.browser && env.browser.Notification) || {}), }, env.browser), isMessagingInitialized() { - if (!this.messaging) { - return false; - } - return this.messaging.isInitialized; + return ( + this.services.messaging.modelManager.messaging && + this.services.messaging.modelManager.messaging.isInitialized + ); }, - messagingCreatedPromise: makeDeferred(), }); return env; } diff --git a/addons/mail/static/src/main.js b/addons/mail/static/src/main.js index e94277fb1df..7d4a8761c8d 100644 --- a/addons/mail/static/src/main.js +++ b/addons/mail/static/src/main.js @@ -1,7 +1,6 @@ /** @odoo-module **/ -import MessagingService from '@mail/services/messaging/messaging'; -import { makeDeferred } from '@mail/utils/deferred/deferred'; +import { MessagingService } from '@mail/services/messaging/messaging'; import env from 'web.commonEnv'; import { serviceRegistry } from 'web.core'; @@ -11,27 +10,20 @@ import { serviceRegistry } from 'web.core'; */ Object.assign(env, { isMessagingInitialized() { - if (!this.modelManager || !this.modelManager.messaging) { - return false; - } - return this.modelManager.messaging.isInitialized; + return ( + this.services.messaging.modelManager.messaging && + this.services.messaging.modelManager.messaging.isInitialized + ); }, - /** - * Promise which becomes resolved when messaging is created. - * - * Useful for discuss widget to know when messaging is created, because this - * is an essential condition to make it work. - */ - messagingCreatedPromise: makeDeferred(), }); Object.defineProperty(env, 'messaging', { get() { - return this.modelManager && this.modelManager.messaging; + return this.services.messaging.modelManager.messaging; }, }); Object.defineProperty(env, 'models', { get() { - return this.modelManager && this.modelManager.models; + return this.services.messaging.modelManager.models; }, }); diff --git a/addons/mail/static/src/model/model_manager.js b/addons/mail/static/src/model/model_manager.js index ff3cb7e4ba9..4cdf4b3087f 100644 --- a/addons/mail/static/src/model/model_manager.js +++ b/addons/mail/static/src/model/model_manager.js @@ -5,6 +5,7 @@ import { ModelField } from '@mail/model/model_field'; import { Listener } from '@mail/model/model_listener'; import { patchClassMethods, patchInstanceMethods } from '@mail/utils/utils'; import { link, unlinkAll } from '@mail/model/model_field_command'; +import { makeDeferred } from '@mail/utils/deferred/deferred'; /** * Object that manage models and records, notably their update cycle: whenever @@ -23,6 +24,16 @@ export class ModelManager { * The messaging env. */ this.env = env; + /** + * Promise which becomes resolved when messaging is created. Useful for + * waiting before accessing `this.messaging`. + */ + this.messagingCreatedPromise = makeDeferred(); + /** + * Promise which becomes resolved when messaging is initialized. Useful + * for waiting before accessing `this.messaging`. + */ + this.messagingInitializedPromise = makeDeferred(); //---------------------------------------------------------------------- // Various variables that are necessary to handle an update cycle. The @@ -99,12 +110,36 @@ export class ModelManager { } /** - * Called when all JS modules that register or patch models have been - * done. This launches generation of models. + * Starts the generation of models as soon as all files have been loaded and + * starts messaging itself afterwards. * * @param {Object} values field name/value pairs to give at messaging create */ - start(values) { + async start(values) { + if (document.readyState === 'loading') { + await new Promise(resolve => { + /** + * Called when all JS resources are loaded. This is useful in order + * to do some processing after other JS files have been parsed, for + * example new models or patched models that are coming from + * other modules, because some of those patches might need to be + * applied before messaging initialization. + */ + window.addEventListener('load', resolve); + }); + } + /** + * All JS resources are loaded, but not necessarily processed. + * We assume no messaging-related modules return any Promise, + * therefore they should be processed *at most* asynchronously at + * "Promise time". + */ + await new Promise(resolve => setTimeout(resolve)); + /** + * Some models require session data, like locale text direction (depends on + * fully loaded translation). + */ + await this.env.session.is_bound; /** * Generate the models. */ @@ -113,6 +148,9 @@ export class ModelManager { * Create the messaging singleton record. */ this.models['mail.messaging'].create(values); + this.messagingCreatedPromise.resolve(); + await this.messaging.start(); + this.messagingInitializedPromise.resolve(); } //-------------------------------------------------------------------------- @@ -246,6 +284,19 @@ export class ModelManager { return; } + /** + * Returns the messaging record once it is initialized. This method should + * be considered the main entry point to the messaging system for outside + * code. + * + * @returns {mail.messaging} + **/ + async getMessaging() { + await this.messagingCreatedPromise; + await this.messagingInitializedPromise; + return this.messaging; + } + /** * This method creates a record or updates one of provided Model, based on * provided data. This method assumes that records are uniquely identifiable @@ -362,6 +413,10 @@ export class ModelManager { for (const Model of Object.values(Models)) { for (const fieldName in Model.fields) { const field = Model.fields[fieldName]; + // 0. Forbidden name. + if (fieldName in Model.prototype) { + throw new Error(`Field "${Model.modelName}/${fieldName}" has a forbidden name.`); + } // 1. Field type is required. if (!(['attribute', 'relation'].includes(field.fieldType))) { throw new Error(`Field "${Model.modelName}/${fieldName}" has unsupported type ${field.fieldType}.`); @@ -628,8 +683,6 @@ export class ModelManager { this._messaging = record; } Object.assign(record, { - // The messaging env. - env: this.env, // The unique record identifier. localId, // Listeners that are bound to this record, to be notified of @@ -835,15 +888,9 @@ export class ModelManager { if (!generatable) { throw new Error(`Cannot generate following Model: ${toGenerateNames.join(', ')}`); } - // Make environment accessible from Model. + // Make model manager accessible from Model. const Model = generatable.factory(Models); - Model.env = this.env; - const modelManager = this; - Object.defineProperty(Model, 'messaging', { - get() { - return modelManager.messaging; - }, - }); + Model.modelManager = this; /** * Contains all records. key is local id, while value is the record. */ @@ -1061,7 +1108,6 @@ export class ModelManager { * access to field getter and to prevent field from being written * without calling update (which is necessary to process update cycle). */ - const modelManager = this; for (const Model of Object.values(Models)) { // Object with fieldName/field as key/value pair, for quick access. Model.__fieldMap = Model.__combinedFields; @@ -1074,10 +1120,10 @@ export class ModelManager { for (const field of Model.__fieldList) { Object.defineProperty(Model.prototype, field.fieldName, { get() { // this is bound to record - for (const listener of modelManager._listeners) { - modelManager._localIdsObservedByListener.get(listener).add(this.localId); - modelManager._listenersObservingLocalId.get(this.localId).add(listener); - modelManager._listenersObservingFieldOfLocalId.get(this.localId).get(field).add(listener); + for (const listener of this.modelManager._listeners) { + this.modelManager._localIdsObservedByListener.get(listener).add(this.localId); + this.modelManager._listenersObservingLocalId.get(this.localId).add(listener); + this.modelManager._listenersObservingFieldOfLocalId.get(this.localId).get(field).add(listener); } return field.get(this); }, diff --git a/addons/mail/static/src/models/model/model.js b/addons/mail/static/src/models/model/model.js index 1b91b9aedcf..cecce780472 100644 --- a/addons/mail/static/src/models/model/model.js +++ b/addons/mail/static/src/models/model/model.js @@ -81,7 +81,7 @@ function factory() { * @returns {mail.model[]} */ static all(filterFunc) { - return this.env.modelManager.all(this, filterFunc); + return this.modelManager.all(this, filterFunc); } /** @@ -96,7 +96,16 @@ function factory() { * @returns {mail.model|mail.model[]} newly created record(s) */ static create(data) { - return this.env.modelManager.create(this, data); + return this.modelManager.create(this, data); + } + + /** + * Returns the current env. + * + * @returns {Object} + */ + static get env() { + return this.modelManager.env; } /** @@ -107,7 +116,7 @@ function factory() { * @returns {mail.model|undefined} */ static find(findFunc) { - return this.env.modelManager.find(this, findFunc); + return this.modelManager.find(this, findFunc); } /** @@ -120,7 +129,7 @@ function factory() { * @returns {mail.model|undefined} */ static findFromIdentifyingData(data) { - return this.env.modelManager.findFromIdentifyingData(this, data); + return this.modelManager.findFromIdentifyingData(this, data); } /** @@ -137,7 +146,7 @@ function factory() { * @returns {mail.model|undefined} */ static get(localId, { isCheckingInheritance } = {}) { - return this.env.modelManager.get(this, localId, { isCheckingInheritance }); + return this.modelManager.get(this, localId, { isCheckingInheritance }); } /** @@ -150,7 +159,25 @@ function factory() { * @returns {mail.model|mail.model[]} created or updated record(s). */ static insert(data) { - return this.env.modelManager.insert(this, data); + return this.modelManager.insert(this, data); + } + + /** + * Returns the messaging singleton. + * + * @returns {mail.messaging} + */ + static get messaging() { + return this.modelManager.messaging; + } + + /** + * Returns all existing models. + * + * @returns {Object} keys are model name, values are model class. + */ + static get models() { + return this.modelManager.models; } /** @@ -187,7 +214,16 @@ function factory() { * This method deletes this record. */ delete() { - this.env.modelManager.delete(this); + this.modelManager.delete(this); + } + + /** + * Returns the current env. + * + * @returns {Object} + */ + get env() { + return this.modelManager.env; } /** @@ -196,7 +232,25 @@ function factory() { * @returns {boolean} */ exists() { - return this.env.modelManager.exists(this.constructor, this); + return this.modelManager.exists(this.constructor, this); + } + + /** + * Returns the model manager. + * + * @returns {ModelManager} + */ + get modelManager() { + return this.constructor.modelManager; + } + + /** + * Returns all existing models. + * + * @returns {Object} keys are model name, values are model class. + */ + get models() { + return this.modelManager.models; } /** @@ -205,7 +259,7 @@ function factory() { * @param {Object} [data={}] */ update(data = {}) { - this.env.modelManager.update(this, data); + this.modelManager.update(this, data); } //---------------------------------------------------------------------- diff --git a/addons/mail/static/src/services/dialog_service/dialog_service.js b/addons/mail/static/src/services/dialog_service/dialog_service.js index 488b85f21cf..d6372d81217 100644 --- a/addons/mail/static/src/services/dialog_service/dialog_service.js +++ b/addons/mail/static/src/services/dialog_service/dialog_service.js @@ -12,7 +12,6 @@ const DialogService = AbstractService.extend({ */ start() { this._super(...arguments); - this._webClientReady = false; this._listenHomeMenu(); }, /** @@ -65,7 +64,6 @@ const DialogService = AbstractService.extend({ */ async _onWebClientReady() { await this._mount(); - this._webClientReady = true; } }); diff --git a/addons/mail/static/src/services/messaging/messaging.js b/addons/mail/static/src/services/messaging/messaging.js index 5fc156477af..4d916ff9503 100644 --- a/addons/mail/static/src/services/messaging/messaging.js +++ b/addons/mail/static/src/services/messaging/messaging.js @@ -4,55 +4,21 @@ import { ModelManager } from '@mail/model/model_manager'; import AbstractService from 'web.AbstractService'; -export default AbstractService.extend({ +export const MessagingService = AbstractService.extend({ dependencies: ['bus_service'], + messagingValues: {}, /** * @override */ - init() { - this._super(...arguments); - this.messaging = undefined; - this._modelManager = new ModelManager(this.env); - this.env.modelManager = this._modelManager; + init(env) { + this._super(env); + this.modelManager = new ModelManager(env); }, /** * @override */ - async start() { - this._super(...arguments); - if (document.readyState === 'loading') { - await new Promise(resolve => { - /** - * Called when all JS resources are loaded. This is useful in order - * to do some processing after other JS files have been parsed, for - * example new models or patched models that are coming from - * other modules, because some of those patches might need to be - * applied before messaging initialization. - */ - window.addEventListener('load', resolve); - }); - } - /** - * All JS resources are loaded, but not necessarily processed. - * We assume no messaging-related modules return any Promise, - * therefore they should be processed *at most* asynchronously at - * "Promise time". - */ - await new Promise(resolve => setTimeout(resolve)); - /** - * Some models require session data, like locale text direction (depends on - * fully loaded translation). - */ - await this.env.session.is_bound; - await this.startModelManager(); - this.messaging = this._modelManager.messaging; - this.env.messagingCreatedPromise.resolve(); - await this.messaging.start(); - }, - /** - * Starts the model manager. - */ - async startModelManager() { - await this._modelManager.start(); + start() { + this._super(); + this.modelManager.start(this.messagingValues); }, }); diff --git a/addons/mail/static/src/services/new_message_service/new_message_service.js b/addons/mail/static/src/services/new_message_service/new_message_service.js index d30c95b2316..0352f44aa16 100644 --- a/addons/mail/static/src/services/new_message_service/new_message_service.js +++ b/addons/mail/static/src/services/new_message_service/new_message_service.js @@ -2,22 +2,18 @@ import { registry } from "@web/core/registry"; -const { Component } = owl; - export const newMessageService = { dependencies: ["command"], start(newEnv, { command }) { command.add("Message a user", async () => { - const env = Component.env; - await env.messagingCreatedPromise; - await env.messaging.initializedPromise; - if (env.messaging.discuss.isOpen) { - env.messaging.discuss.update({ + const messaging = await owl.Component.env.services.messaging.modelManager.getMessaging(); + if (messaging.discuss.isOpen) { + messaging.discuss.update({ isAddingChannel: false, isAddingChat: true, }); } else { - env.messaging.chatWindowManager.openNewMessage(); + messaging.chatWindowManager.openNewMessage(); } }, { category: "mail", diff --git a/addons/mail/static/src/utils/test_utils.js b/addons/mail/static/src/utils/test_utils.js index 5942a3b74c9..6231ca33751 100644 --- a/addons/mail/static/src/utils/test_utils.js +++ b/addons/mail/static/src/utils/test_utils.js @@ -7,7 +7,7 @@ import { addTimeControlToEnv, } from '@mail/env/test_env'; import ChatWindowService from '@mail/services/chat_window_service/chat_window_service'; -import MessagingService from '@mail/services/messaging/messaging'; +import { MessagingService } from '@mail/services/messaging/messaging'; import { makeDeferred } from '@mail/utils/deferred/deferred'; import DialogService from '@mail/services/dialog_service/dialog_service'; import { getMessagingComponent } from '@mail/utils/messaging_component'; @@ -328,7 +328,7 @@ function beforeEach(self) { if (!this.env || !this.env.services.messaging) { return undefined; } - return this.env.services.messaging.messaging; + return this.env.services.messaging.modelManager.messaging; }, }); } @@ -534,6 +534,14 @@ async function start(param0 = {}) { }), local_storage: AbstractStorageService.extend({ storage: new RamStorage() }), messaging: MessagingService.extend({ + // test specific values + messagingValues: { + autofetchPartnerImStatus: false, + disableAnimation: true, + isQUnitTest: true, + loadingBaseDelayDuration, + messagingBus, + }, /** * Override to bind the getters on the actual env that is given, * because the env in the scope of this test might only serve as @@ -541,46 +549,34 @@ async function start(param0 = {}) { * * @override */ - init() { - this._super(...arguments); - Object.defineProperty(this.env, 'messaging', { + init(env) { + Object.defineProperty(env, 'messaging', { get() { - return this.modelManager.messaging; + return this.services.messaging.modelManager.messaging; }, }); - Object.defineProperty(this.env, 'models', { + Object.defineProperty(env, 'models', { get() { - return this.modelManager.models; + return this.services.messaging.modelManager.models; }, }); + this._super(env); }, /** - * Override to control when messaging is created, useful to test - * spinners and race conditions. + * Override: + * - to ensure the test setup is complete before starting otherwise + * for example the mock server might not be ready yet at init + * messaging, + * - to add control on when messaging is created, useful to test + * spinners and race conditions. * * @override */ async start() { const _super = this._super.bind(this); - await messagingBeforeCreationDeferred; - await _super(); - }, - /** - * Override to ensure the test setup is complete before starting - * otherwise for example the mock server might not be ready yet. - * And also override to give test values to messaging. - * - * @override - */ - async startModelManager() { await testSetupDoneDeferred; - await this._modelManager.start({ - autofetchPartnerImStatus: false, - disableAnimation: true, - isQUnitTest: true, - loadingBaseDelayDuration, - messagingBus, - }); + await messagingBeforeCreationDeferred; + _super(); }, }), }, param0.services); @@ -727,12 +723,13 @@ async function start(param0 = {}) { mockServer, widget, }; + const { modelManager } = testEnv.services.messaging; if (waitUntilMessagingCondition === 'created') { - await testEnv.messagingCreatedPromise; + await modelManager.messagingCreatedPromise; } if (waitUntilMessagingCondition === 'initialized') { - await testEnv.messagingCreatedPromise; - await testEnv.modelManager.messaging.initializedPromise; + await modelManager.messagingCreatedPromise; + await modelManager.messagingInitializedPromise; } if (mountCallbacks.length > 0) { await afterNextRender(async () => { diff --git a/addons/mail/static/src/webclient/commands/mail_providers.js b/addons/mail/static/src/webclient/commands/mail_providers.js index 7bc33f4504f..bde9023d78c 100644 --- a/addons/mail/static/src/webclient/commands/mail_providers.js +++ b/addons/mail/static/src/webclient/commands/mail_providers.js @@ -18,11 +18,9 @@ const commandProviderRegistry = registry.category("command_provider"); commandProviderRegistry.add("partner", { nameSpace: "@", async provide(newEnv, options) { - const env = Component.env; - await env.messagingCreatedPromise; - await env.messaging.initializedPromise; + const messaging = await Component.env.services.messaging.modelManager.getMessaging(); const suggestions = []; - await env.models['mail.partner'].imSearch({ + await messaging.models['mail.partner'].imSearch({ callback(partners) { partners.forEach((partner) => { suggestions.push({ @@ -47,10 +45,8 @@ commandProviderRegistry.add("partner", { commandProviderRegistry.add("channel", { nameSpace: "#", async provide(newEnv, options) { - const env = Component.env; - await env.messagingCreatedPromise; - await env.messaging.initializedPromise; - const channels = await env.models['mail.thread'].searchChannelsToOpen({ + const messaging = await Component.env.services.messaging.modelManager.getMessaging(); + const channels = await messaging.models['mail.thread'].searchChannelsToOpen({ limit: 10, searchTerm: options.searchValue, }); diff --git a/addons/mail/static/src/widgets/discuss/discuss.js b/addons/mail/static/src/widgets/discuss/discuss.js index ff87bcff37d..c78b6960447 100644 --- a/addons/mail/static/src/widgets/discuss/discuss.js +++ b/addons/mail/static/src/widgets/discuss/discuss.js @@ -32,7 +32,7 @@ const DiscussWidget = AbstractAction.extend({ this._lastPushStateActiveThread = null; this.env = Component.env; - this.env.messagingCreatedPromise.then(() => { + Component.env.services.messaging.modelManager.messagingCreatedPromise.then(() => { const initActiveId = this.options.active_id || (this.action.context && this.action.context.active_id) || (this.action.params && this.action.params.default_active_id) ||