From 9850b8e4dcc67598d91a34ee8ee5777dcdbbacbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 26 May 2021 11:22:41 +0000 Subject: [PATCH] [REF] web: allow services to specify async methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit improves the service API to allow the service creator to mark some methods of the service as 'async'. This is a very important feature when we consider the fact that a component calling such a method may be destroyed before the asynchronous work is complete. In that case, we do not want any code from the component to be executed. Such async method, when called in a component (i.e. when the service was obtained with the useService hook), will be wrapped in such a way that the returned promises will not be resolved if the component is destroyed. Note that it only impact method calls for services obtained from the useService hook, not calls for the actual service obtained from the env. closes odoo-dev/odoo#886 Signed-off-by: Géry Debongnie (ged) --- .../static/src/core/network/rpc_service.js | 10 +--- addons/web/static/src/core/orm_service.js | 49 ++++++++++--------- .../static/src/core/popover/popover_hook.js | 39 +++++++++++++++ .../src/core/popover/popover_service.js | 31 ------------ addons/web/static/src/core/service_hook.js | 25 ++++++++-- addons/web/static/src/env.js | 6 +-- .../tests/core/network/rpc_service_tests.js | 2 +- .../static/tests/core/orm_service_tests.js | 1 - .../core/popover/popover_service_tests.js | 8 +-- 9 files changed, 97 insertions(+), 74 deletions(-) create mode 100644 addons/web/static/src/core/popover/popover_hook.js diff --git a/addons/web/static/src/core/network/rpc_service.js b/addons/web/static/src/core/network/rpc_service.js index 38d9c25e60b..0bf6056a51c 100644 --- a/addons/web/static/src/core/network/rpc_service.js +++ b/addons/web/static/src/core/network/rpc_service.js @@ -96,21 +96,13 @@ function jsonrpc(env, rpcId, url, params, settings = {}) { // RPC service // ----------------------------------------------------------------------------- export const rpcService = { + async: true, start(env) { let rpcId = 0; return function rpc(route, params = {}, settings) { return jsonrpc(env, rpcId++, route, params, settings); }; }, - specializeForComponent(component, rpc) { - return async (...args) => { - if (component.__owl__.status === 5 /* DESTROYED */) { - throw new Error("A destroyed component should never initiate a RPC"); - } - const result = await rpc(...args); - return component.__owl__.status === 5 ? new Promise(() => {}) : result; - }; - }, }; registry.category("services").add("rpc", rpcService); diff --git a/addons/web/static/src/core/orm_service.js b/addons/web/static/src/core/orm_service.js index a5a4a77d383..dccf57efefa 100644 --- a/addons/web/static/src/core/orm_service.js +++ b/addons/web/static/src/core/orm_service.js @@ -1,6 +1,5 @@ /** @odoo-module **/ -import { useService } from "./service_hook"; import { registry } from "./registry"; /** @@ -55,22 +54,6 @@ class ORM { return this.call(model, "read", [ids, fields], { context: ctx }); } - unlink(model, ids, ctx) { - return this.call(model, "unlink", [ids], { context: ctx }); - } - - write(model, ids, data, ctx) { - return this.call(model, "write", [ids, data], { context: ctx }); - } - - search(model, domain, options = {}, ctx = {}) { - const kwargs = { - context: ctx, - }; - assignOptions(kwargs, options, ["offset", "limit", "order"]); - return this.call(model, "search", [domain], kwargs); - } - readGroup(model, domain, fields, groupby, options = {}, ctx = {}) { const kwargs = { domain, @@ -82,17 +65,33 @@ class ORM { return this.call(model, "web_read_group", [], kwargs); } + search(model, domain, options = {}, ctx = {}) { + const kwargs = { + context: ctx, + }; + assignOptions(kwargs, options, ["offset", "limit", "order"]); + return this.call(model, "search", [domain], kwargs); + } + searchRead(model, domain, fields, options = {}, ctx = {}) { const kwargs = { context: ctx, domain, fields }; assignOptions(kwargs, options, ["offset", "limit", "order"]); return this.call(model, "search_read", [], kwargs); } + unlink(model, ids, ctx) { + return this.call(model, "unlink", [ids], { context: ctx }); + } + webSearchRead(model, domain, fields, options = {}, ctx = {}) { const kwargs = { context: ctx, domain, fields }; assignOptions(kwargs, options, ["offset", "limit", "order"]); return this.call(model, "web_search_read", [], kwargs); } + + write(model, ids, data, ctx) { + return this.call(model, "write", [ids, data], { context: ctx }); + } } /** @@ -109,14 +108,20 @@ class ORM { */ export const ormService = { dependencies: ["rpc", "user"], + async: [ + "call", + "create", + "read", + "readGroup", + "search", + "searchRead", + "unlink", + "web_search_read", + "write", + ], start(env, { rpc, user }) { return new ORM(rpc, user); }, - specializeForComponent() { - const rpc = useService("rpc"); - const user = useService("user"); - return new ORM(rpc, user); - }, }; registry.category("services").add("orm", ormService); diff --git a/addons/web/static/src/core/popover/popover_hook.js b/addons/web/static/src/core/popover/popover_hook.js new file mode 100644 index 00000000000..e7820f00ae3 --- /dev/null +++ b/addons/web/static/src/core/popover/popover_hook.js @@ -0,0 +1,39 @@ +/** @odoo-module **/ + +import { useService } from "../service_hook"; + +const { onWillUnmount, useComponent } = owl.hooks; + +export function usePopover() { + const keys = new Set(); + const service = useService("popover"); + const component = useComponent(); + + onWillUnmount(function () { + for (const key of keys) { + service.remove(key); + } + keys.clear(); + }); + return Object.assign(Object.create(service), { + add(params) { + const newParams = Object.create(params); + newParams.onClose = function (key) { + if (!params.keepOnClose) { + // manager will delete the popover if keepOnClose is falsy + keys.delete(key); + } + if (params.onClose && component.__owl__.status !== 5 /* DESTROYED */) { + params.onClose(key); + } + }; + const key = service.add(newParams); + keys.add(key); + return key; + }, + remove(key) { + keys.delete(key); + service.remove(key); + }, + }); +} diff --git a/addons/web/static/src/core/popover/popover_service.js b/addons/web/static/src/core/popover/popover_service.js index f033e1b3ba9..b6b017c5b0a 100644 --- a/addons/web/static/src/core/popover/popover_service.js +++ b/addons/web/static/src/core/popover/popover_service.js @@ -7,7 +7,6 @@ import { Popover } from "./popover"; const { Component } = owl; const { EventBus } = owl.core; -const { onWillUnmount } = owl.hooks; const { xml } = owl.tags; export class KeyAlreadyExistsError extends Error {} @@ -131,36 +130,6 @@ export const popoverService = { }, }; }, - specializeForComponent(component, service) { - const keys = new Set(); - onWillUnmount(function () { - for (const key of keys) { - service.remove(key); - } - keys.clear(); - }); - return Object.assign(Object.create(service), { - add(params) { - const newParams = Object.create(params); - newParams.onClose = function (key) { - if (!params.keepOnClose) { - // manager will delete the popover if keepOnClose is falsy - keys.delete(key); - } - if (params.onClose && component.__owl__.status !== 5 /* DESTROYED */) { - params.onClose(key); - } - }; - const key = service.add(newParams); - keys.add(key); - return key; - }, - remove(key) { - keys.delete(key); - service.remove(key); - }, - }); - }, }; registry.category("services").add("popover", popoverService); diff --git a/addons/web/static/src/core/service_hook.js b/addons/web/static/src/core/service_hook.js index dc80403c465..5fd2f4ef321 100644 --- a/addons/web/static/src/core/service_hook.js +++ b/addons/web/static/src/core/service_hook.js @@ -1,6 +1,6 @@ /** @odoo-module **/ -import { SPECIAL_METHOD } from "../env"; +import { SERVICES_METADATA } from "../env"; const { useComponent } = owl.hooks; @@ -17,8 +17,27 @@ export function useService(serviceName) { throw new Error(`Service ${serviceName} is not available`); } const service = services[serviceName]; - if (service && SPECIAL_METHOD in service) { - return service[SPECIAL_METHOD](component, service); + if (serviceName in SERVICES_METADATA) { + if (service instanceof Function) { + return protectMethod(component, null, service); + } else { + const methods = SERVICES_METADATA[serviceName]; + const result = Object.create(service); + for (let method of methods) { + result[method] = protectMethod(component, service, service[method]); + } + return result; + } } return service; } + +function protectMethod(component, caller, fn) { + return async (...args) => { + if (component.__owl__.status === 5 /* DESTROYED */) { + throw new Error("Component is destroyed"); + } + const result = await fn.call(caller, ...args); + return component.__owl__.status === 5 ? new Promise(() => {}) : result; + }; +} diff --git a/addons/web/static/src/env.js b/addons/web/static/src/env.js index 0b900fcc609..58d3625fad0 100644 --- a/addons/web/static/src/env.js +++ b/addons/web/static/src/env.js @@ -47,7 +47,7 @@ export function makeEnv(debug) { const serviceRegistry = registry.category("services"); -export const SPECIAL_METHOD = Symbol("special_method"); +export const SERVICES_METADATA = {}; /** * Start all services registered in the service registry, while making sure @@ -96,8 +96,8 @@ async function _startServices(env, toStart, timeoutId) { const entries = (service.dependencies || []).map((dep) => [dep, services[dep]]); const dependencies = Object.fromEntries(entries); const value = service.start(env, dependencies); - if (value && "specializeForComponent" in service) { - value[SPECIAL_METHOD] = service.specializeForComponent; + if ("async" in service) { + SERVICES_METADATA[name] = service.async; } if (value instanceof Promise) { proms.push( diff --git a/addons/web/static/tests/core/network/rpc_service_tests.js b/addons/web/static/tests/core/network/rpc_service_tests.js index 22bde300301..22c2df5f608 100644 --- a/addons/web/static/tests/core/network/rpc_service_tests.js +++ b/addons/web/static/tests/core/network/rpc_service_tests.js @@ -163,7 +163,7 @@ QUnit.test("rpc initiated from destroyed components throw exception", async (ass try { await component.rpc("/my/route"); } catch (e) { - assert.strictEqual(e.message, "A destroyed component should never initiate a RPC"); + assert.strictEqual(e.message, "Component is destroyed"); } }); diff --git a/addons/web/static/tests/core/orm_service_tests.js b/addons/web/static/tests/core/orm_service_tests.js index 7c9ad232532..ff90b25b752 100644 --- a/addons/web/static/tests/core/orm_service_tests.js +++ b/addons/web/static/tests/core/orm_service_tests.js @@ -255,7 +255,6 @@ QUnit.test("useModel is specialized for component", async (assert) => { const target = getFixture(); const component = await mount(MyComponent, { env, target }); - assert.strictEqual(component.rpc, component.orm.rpc); assert.notStrictEqual(component.orm, env.services.orm); }); diff --git a/addons/web/static/tests/core/popover/popover_service_tests.js b/addons/web/static/tests/core/popover/popover_service_tests.js index 65854e817dd..e3256f5e63a 100644 --- a/addons/web/static/tests/core/popover/popover_service_tests.js +++ b/addons/web/static/tests/core/popover/popover_service_tests.js @@ -8,7 +8,7 @@ import { popoverService, } from "@web/core/popover/popover_service"; import { registry } from "@web/core/registry"; -import { useService } from "@web/core/service_hook"; +import { usePopover } from "@web/core/popover/popover_hook"; import { registerCleanup } from "../../helpers/cleanup"; import { clearRegistryWithCleanup, makeTestEnv } from "../../helpers/mock_env"; import { click, getFixture, nextTick } from "../../helpers/utils"; @@ -246,7 +246,7 @@ QUnit.test("remove popover when component is unmount", async function (assert) { class MyComp extends Component { setup() { - this.popoverService = useService("popover"); + this.popoverService = usePopover(); } showPopover(key) { const params = { @@ -304,7 +304,7 @@ QUnit.test("service does not try to remove destroyed popover", async function (a class MyComp extends Component { setup() { - this.popoverService = useService("popover"); + this.popoverService = usePopover(); } showPopover() { const params = { @@ -345,7 +345,7 @@ QUnit.test("popover kept on close is removed when component unmount", async func class MyComp extends Component { setup() { - this.popoverService = useService("popover"); + this.popoverService = usePopover(); } showPopover() { const params = {