From 517b79a5de1f4fc6e67626affbc5c5ed9a5cbc3a Mon Sep 17 00:00:00 2001 From: "Joseph (jcb)" Date: Sat, 6 Jan 2024 12:22:48 +0100 Subject: [PATCH] [IMP] point_of_sale: lazy reactive getters This commit introduces an implementation of lazy reactive computed value that is pull-based -- it only recomputes when it's needed. Check the following PR in odoo/owl for its origin: https://github.com/odoo/owl/pull/1499 It's basically an automatic caching mechanism that taps to the reactivity primitive of odoo/owl. Because it depends on odoo/owl's reactivity, the logic inside a getter is only recomputed when its dependencies changed. Example: Given the following base values `A`, `B`, `C` and `D`, and dependent values `AB`, `BC` and `X`. ```mermaid graph TD A --> AB B --> AB B --> BC C --> BC AB --> X D --> X ``` - Changing `A` should recompute `AB` and `X`. - Changing `B` should recompute `AB`, `BC` and `X`. - Changing `C` should recompute `BC`. - Changing `D` should recompute `X`. It's also pull-based (thus called _lazy_) which means that recomputations are only called when necessary. Imagine a view (component) is only displaying `AB` and `BC` (check above diagram). Changing `D` should invalidate value of `X`, but since the view isn't dependent on `X`, there won't be a rerendering and the logic of `X` won't be called. An obvious benefit of this change is performance but we can consider it as minor. A case where performance change is noticeable is when pos loaded 20k products. In that situation, before this commit, there is a noticeable delay when clicking a product. With this commit, the app feels as if only few products are loaded. Aside from performance, developers also benefits on debugging getters because the logic is only called once per rendering. DX will also be good because we just write basic getters when we want them to be lazy and reactive. Should we want a getter to be called everytime, we define a normal method. IMPORTANT NOTE: Mutations inside a getter isn't allowed. It might result to infinite loop. For initial utilization of this feature, we converted few methods into lazy getters. 1. `get productsToDisplay` - This was originally in the `ProductScreen` component. We moved it to `PosStore`. As a result, `productsToDisplay` is now only recomputed when necessary (such as changing the category or changing the search word). Logic inside the getter is not called excessively when `ProductScreen` rerenders, e.g. when adding a new orderline. 2. `getProductsByCategory(category)` is now `get associatedProducts` in the `PosCategory` model. 3. `getUnitDisplayPrice` is now `get unitDisplayPrice` in `PosOrderLine`. 4. `getAllPrices()` and `getAllPrices(1)` calls are now hidden behind the `get allPrices` and `get allUnitPrices` getters, respectively. TASK-ID: 4361605 --- addons/point_of_sale/__manifest__.py | 2 + .../static/src/app/models/pos_category.js | 8 + .../static/src/app/models/pos_order.js | 2 +- .../static/src/app/models/pos_order_line.js | 34 +- .../static/src/app/models/related_models.js | 76 ++-- .../screens/product_screen/product_screen.js | 71 ---- .../screens/product_screen/product_screen.xml | 4 +- .../screens/receipt_screen/receipt_screen.js | 2 +- .../static/src/app/services/pos_store.js | 70 +++- .../point_of_sale/static/src/lazy_getter.js | 102 ++++++ addons/point_of_sale/static/src/proxy_trap.js | 39 ++ .../static/tests/unit/lazy_getter.test.js | 340 ++++++++++++++++++ .../static/src/app/models/restaurant_table.js | 2 +- .../static/src/app/services/pos_store.js | 10 +- addons/pos_self_order/__manifest__.py | 2 + 15 files changed, 623 insertions(+), 141 deletions(-) create mode 100644 addons/point_of_sale/static/src/lazy_getter.js create mode 100644 addons/point_of_sale/static/src/proxy_trap.js create mode 100644 addons/point_of_sale/static/tests/unit/lazy_getter.test.js diff --git a/addons/point_of_sale/__manifest__.py b/addons/point_of_sale/__manifest__.py index 5c5b75e7d7ea3..314187c3d4644 100644 --- a/addons/point_of_sale/__manifest__.py +++ b/addons/point_of_sale/__manifest__.py @@ -90,6 +90,8 @@ 'point_of_sale/static/src/app/models/utils/indexed_db.js', 'point_of_sale/static/src/app/models/data_service_options.js', 'point_of_sale/static/src/utils.js', + 'point_of_sale/static/src/proxy_trap.js', + 'point_of_sale/static/src/lazy_getter.js', 'point_of_sale/static/src/app/services/data_service.js', 'point_of_sale/static/tests/unit/**/*', ], diff --git a/addons/point_of_sale/static/src/app/models/pos_category.js b/addons/point_of_sale/static/src/app/models/pos_category.js index e058b73b2e702..02a0fe585d374 100644 --- a/addons/point_of_sale/static/src/app/models/pos_category.js +++ b/addons/point_of_sale/static/src/app/models/pos_category.js @@ -30,6 +30,14 @@ export class PosCategory extends Base { return parents.reverse(); } + get associatedProducts() { + const allCategoryIds = this.getAllChildren().map((cat) => cat.id); + const products = allCategoryIds.flatMap( + (catId) => this.models["product.template"].getBy("pos_categ_ids", catId) || [] + ); + // Remove duplicates since owl doesn't like them. + return Array.from(new Set(products)); + } } registry.category("pos_available_models").add(PosCategory.pythonModel, PosCategory); diff --git a/addons/point_of_sale/static/src/app/models/pos_order.js b/addons/point_of_sale/static/src/app/models/pos_order.js index 0641d36a3c116..f5c9ad6a1d29d 100644 --- a/addons/point_of_sale/static/src/app/models/pos_order.js +++ b/addons/point_of_sale/static/src/app/models/pos_order.js @@ -673,7 +673,7 @@ export class PosOrder extends Base { getTaxDetails() { const taxDetails = {}; for (const line of this.lines) { - for (const taxData of line.getAllPrices().taxesData) { + for (const taxData of line.allPrices.taxesData) { const taxId = taxData.id; if (!taxDetails[taxId]) { taxDetails[taxId] = Object.assign({}, taxData, { diff --git a/addons/point_of_sale/static/src/app/models/pos_order_line.js b/addons/point_of_sale/static/src/app/models/pos_order_line.js index 8e0d9353cbca3..30fe1418f125c 100644 --- a/addons/point_of_sale/static/src/app/models/pos_order_line.js +++ b/addons/point_of_sale/static/src/app/models/pos_order_line.js @@ -361,19 +361,19 @@ export class PosOrderline extends Base { return window.parseFloat(roundDecimals(this.price_unit || 0, digits).toFixed(digits)); } - getUnitDisplayPrice() { + get unitDisplayPrice() { if (this.config.iface_tax_included === "total") { - return this.getAllPrices(1).priceWithTax; + return this.allUnitPrices.priceWithTax; } else { - return this.getAllPrices(1).priceWithoutTax; + return this.allUnitPrices.priceWithoutTax; } } getUnitDisplayPriceBeforeDiscount() { if (this.config.iface_tax_included === "total") { - return this.getAllPrices(1).priceWithTaxBeforeDiscount; + return this.allUnitPrices.priceWithTaxBeforeDiscount; } else { - return this.getAllPrices(1).priceWithoutTaxBeforeDiscount; + return this.allUnitPrices.priceWithoutTaxBeforeDiscount; } } getBasePrice() { @@ -422,19 +422,19 @@ export class PosOrderline extends Base { } getPriceWithoutTax() { - return this.getAllPrices().priceWithoutTax; + return this.allPrices.priceWithoutTax; } getPriceWithTax() { - return this.getAllPrices().priceWithTax; + return this.allPrices.priceWithTax; } getTax() { - return this.getAllPrices().tax; + return this.allPrices.tax; } getTaxDetails() { - return this.getAllPrices().taxDetails; + return this.allPrices.taxDetails; } getTotalTaxesIncludedInPrice() { @@ -516,6 +516,14 @@ export class PosOrderline extends Base { }; } + get allPrices() { + return this.getAllPrices(); + } + + get allUnitPrices() { + return this.getAllPrices(1); + } + displayDiscountPolicy() { // Sales dropped `discount_policy`, and we only show discount if applied pricelist rule // is a percentage discount. However we don't have that information in pos @@ -584,17 +592,17 @@ export class PosOrderline extends Base { getComboTotalPrice() { const allLines = this.getAllLinesInCombo(); - return allLines.reduce((total, line) => total + line.getAllPrices(1).priceWithTax, 0); + return allLines.reduce((total, line) => total + line.allUnitPrices.priceWithTax, 0); } getComboTotalPriceWithoutTax() { const allLines = this.getAllLinesInCombo(); - return allLines.reduce((total, line) => total + line.getAllPrices(1).priceWithoutTax, 0); + return allLines.reduce((total, line) => total + line.allUnitPrices.priceWithoutTax, 0); } getOldUnitDisplayPrice() { return ( this.displayDiscountPolicy() === "without_discount" && - roundCurrency(this.getUnitDisplayPrice(), this.currency) < + roundCurrency(this.unitDisplayPrice, this.currency) < roundCurrency(this.getTaxedlstUnitPrice(), this.currency) && this.getTaxedlstUnitPrice() ); @@ -616,7 +624,7 @@ export class PosOrderline extends Base { price: this.getPriceString(), qty: this.getQuantityStr(), unit: this.product_id.uom_id ? this.product_id.uom_id.name : "", - unitPrice: formatCurrency(this.getUnitDisplayPrice(), this.currency), + unitPrice: formatCurrency(this.unitDisplayPrice, this.currency), oldUnitPrice: this.getOldUnitDisplayPrice() ? formatCurrency(this.getOldUnitDisplayPrice(), this.currency) : "", diff --git a/addons/point_of_sale/static/src/app/models/related_models.js b/addons/point_of_sale/static/src/app/models/related_models.js index d9e5dff20e696..ac669426a0130 100644 --- a/addons/point_of_sale/static/src/app/models/related_models.js +++ b/addons/point_of_sale/static/src/app/models/related_models.js @@ -1,5 +1,7 @@ import { reactive, toRaw } from "@odoo/owl"; import { uuidv4 } from "@point_of_sale/utils"; +import { TrapDisabler } from "@point_of_sale/proxy_trap"; +import { WithLazyGetterTrap } from "@point_of_sale/lazy_getter"; const ID_CONTAINER = {}; @@ -139,12 +141,12 @@ function processModelDefs(modelDefs) { return [inverseMap, modelDefs]; } -export class Base { - constructor({ models, records, model, proxyTrap }) { +export class Base extends WithLazyGetterTrap { + constructor({ models, records, model, traps }) { + super({ traps }); this.models = models; this.records = records; this.model = model; - return new Proxy(this, proxyTrap); } /** * Called during instantiation when the instance is fully-populated with field values. @@ -433,19 +435,6 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) { return records[model].has(id); } - // If value is more than 0, then the proxy trap is disabled. - let proxyTrapDisabled = 0; - function withoutProxyTrap(fn) { - return function (...args) { - try { - proxyTrapDisabled += 1; - return fn(...args); - } finally { - proxyTrapDisabled -= 1; - } - }; - } - /** * This check assumes that if the first element is a command, then the rest are commands. */ @@ -457,30 +446,39 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) { ); } - const proxyTraps = {}; - function getProxyTrap(model) { - if (model in proxyTraps) { - return proxyTraps[model]; - } + const disabler = new TrapDisabler(); + function withoutProxyTrap(fn) { + return (...args) => disabler.call(fn, ...args); + } + + const setTrapsCache = {}; + function instantiateModel(model, { models, records }) { const fields = getFields(model); - const proxyTrap = { - set(target, prop, value) { - if (proxyTrapDisabled || !(prop in fields)) { - return Reflect.set(target, prop, value); + const Model = modelClasses[model] || Base; + if (!(model in setTrapsCache)) { + setTrapsCache[model] = function setTrap(target, prop, value, receiver) { + if (disabler.isDisabled() || !(prop in fields)) { + return Reflect.set(target, prop, value, receiver); } - const field = fields[prop]; - if (field && X2MANY_TYPES.has(field.type)) { - if (!isX2ManyCommands(value)) { - value = [["clear"], ["link", ...value]]; + return disabler.call(() => { + const field = fields[prop]; + if (field && X2MANY_TYPES.has(field.type)) { + if (!isX2ManyCommands(value)) { + value = [["clear"], ["link", ...value]]; + } } - } - target.update({ [prop]: value }); - target.model.triggerEvents("update", { field: prop, value, id: target.id }); - return true; - }, - }; - proxyTraps[model] = proxyTrap; - return proxyTrap; + receiver.update({ [prop]: value }); + target.model.triggerEvents("update", { field: prop, value, id: target.id }); + return true; + }); + }; + } + return new Model({ + models, + records, + model: models[model], + traps: { set: setTrapsCache[model] }, + }); } const create = withoutProxyTrap(_create); @@ -495,9 +493,7 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) { vals["id"] = uuid(model); } - const Model = modelClasses[model] || Base; - const proxyTrap = getProxyTrap(model); - let record = new Model({ models, records, model: models[model], proxyTrap }); + let record = instantiateModel(model, { models, records }); const id = vals["id"]; record.id = id; diff --git a/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.js b/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.js index d5e8e50cf0bef..2e65629946186 100644 --- a/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.js +++ b/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.js @@ -17,13 +17,11 @@ import { Orderline } from "@point_of_sale/app/components/orderline/orderline"; import { OrderWidget } from "@point_of_sale/app/components/order_widget/order_widget"; import { OrderSummary } from "@point_of_sale/app/screens/product_screen/order_summary/order_summary"; import { ProductInfoPopup } from "@point_of_sale/app/components/popups/product_info_popup/product_info_popup"; -import { fuzzyLookup } from "@web/core/utils/search"; import { ProductCard } from "@point_of_sale/app/components/product_card/product_card"; import { ControlButtons, ControlButtonsPopup, } from "@point_of_sale/app/screens/product_screen/control_buttons/control_buttons"; -import { unaccent } from "@web/core/utils/strings"; import { CameraBarcodeScanner } from "@point_of_sale/app/screens/product_screen/camera_barcode_scanner"; export class ProductScreen extends Component { @@ -287,75 +285,6 @@ export class ProductScreen extends Component { return this.pos.searchProductWord.trim(); } - get products() { - return this.pos.models["product.template"].getAll(); - } - - get productsToDisplay() { - let list = []; - - if (this.searchWord !== "") { - list = this.addMainProductsToDisplay(this.getProductsBySearchWord(this.searchWord)); - } else if (this.pos.selectedCategory?.id) { - list = this.getProductsByCategory(this.pos.selectedCategory); - } else { - list = this.products; - } - - if (!list || list.length === 0) { - return []; - } - - const excludedProductIds = [ - this.pos.config.tip_product_id?.id, - ...this.pos.hiddenProductIds, - ...this.pos.session._pos_special_products_ids, - ]; - - list = list - .filter( - (product) => !excludedProductIds.includes(product.id) && product.available_in_pos - ) - .slice(0, 100); - - return this.searchWord !== "" - ? list.sort((a, b) => b.is_favorite - a.is_favorite) - : list.sort((a, b) => { - if (b.is_favorite !== a.is_favorite) { - return b.is_favorite - a.is_favorite; - } - return a.display_name.localeCompare(b.display_name); - }); - } - - getProductsBySearchWord(searchWord) { - return fuzzyLookup(unaccent(searchWord, false), this.products, (product) => - unaccent(product.searchString, false) - ); - } - - addMainProductsToDisplay(products) { - const uniqueProductsMap = new Map(); - for (const product of products) { - if (product.id in this.pos.mainProductVariant) { - const mainProduct = this.pos.mainProductVariant[product.id]; - uniqueProductsMap.set(mainProduct.id, mainProduct); - } else { - uniqueProductsMap.set(product.id, product); - } - } - return Array.from(uniqueProductsMap.values()); - } - - getProductsByCategory(category) { - const allCategoryIds = category.getAllChildren().map((cat) => cat.id); - const products = allCategoryIds.flatMap( - (catId) => this.pos.models["product.template"].getBy("pos_categ_ids", catId) || [] - ); - // Remove duplicates since owl doesn't like it. - return Array.from(new Set(products)); - } - async onPressEnterKey() { const { searchProductWord } = this.pos; if (!searchProductWord) { diff --git a/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.xml b/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.xml index 85afa17efbfe5..9f3f65191d426 100644 --- a/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.xml +++ b/addons/point_of_sale/static/src/app/screens/product_screen/product_screen.xml @@ -25,9 +25,9 @@
-
+
tip_product_id && line.product_id.id === tip_product_id); - const tipAmount = tipLine ? tipLine.getAllPrices().priceWithTax : 0; + const tipAmount = tipLine ? tipLine.allPrices.priceWithTax : 0; const orderAmountStr = this.env.utils.formatCurrency(orderTotalAmount - tipAmount); if (!tipAmount) { return orderAmountStr; diff --git a/addons/point_of_sale/static/src/app/services/pos_store.js b/addons/point_of_sale/static/src/app/services/pos_store.js index 299b207e1837c..61ab381e94d07 100644 --- a/addons/point_of_sale/static/src/app/services/pos_store.js +++ b/addons/point_of_sale/static/src/app/services/pos_store.js @@ -6,7 +6,6 @@ import { floatIsZero } from "@web/core/utils/numbers"; import { registry } from "@web/core/registry"; import { AlertDialog } from "@web/core/confirmation_dialog/confirmation_dialog"; import { deduceUrl, random5Chars, uuidv4, getOnNotified, Counter } from "@point_of_sale/utils"; -import { Reactive } from "@web/core/utils/reactive"; import { HWPrinter } from "@point_of_sale/app/utils/printer/hw_printer"; import { ConnectionAbortedError, ConnectionLostError, RPCError } from "@web/core/network/rpc"; import { OrderReceipt } from "@point_of_sale/app/screens/receipt_screen/receipt/order_receipt"; @@ -34,8 +33,11 @@ import { FormViewDialog } from "@web/views/view_dialogs/form_view_dialog"; import { CashMovePopup } from "@point_of_sale/app/components/popups/cash_move_popup/cash_move_popup"; import { ClosePosPopup } from "@point_of_sale/app/components/popups/closing_popup/closing_popup"; import { user } from "@web/core/user"; +import { fuzzyLookup } from "@web/core/utils/search"; +import { unaccent } from "@web/core/utils/strings"; +import { WithLazyGetterTrap } from "@point_of_sale/lazy_getter"; -export class PosStore extends Reactive { +export class PosStore extends WithLazyGetterTrap { loadingSkipButtonIsShown = false; mainScreen = { name: null, component: null }; @@ -53,9 +55,9 @@ export class PosStore extends Reactive { "alert", "mail.sound_effects", ]; - constructor() { - super(); - this.ready = this.setup(...arguments).then(() => this); + constructor({ traps, env, deps }) { + super({ traps }); + this.ready = this.setup(env, deps).then(() => this); } // use setup instead of constructor because setup can be patched. async setup( @@ -1890,6 +1892,62 @@ export class PosStore extends Reactive { getDisplayDeviceIP() { return this.config.proxy_ip; } + + addMainProductsToDisplay(products) { + const uniqueProductsMap = new Map(); + for (const product of products) { + if (product.id in this.mainProductVariant) { + const mainProduct = this.mainProductVariant[product.id]; + uniqueProductsMap.set(mainProduct.id, mainProduct); + } else { + uniqueProductsMap.set(product.id, product); + } + } + return Array.from(uniqueProductsMap.values()); + } + + get productsToDisplay() { + const searchWord = this.searchProductWord.trim(); + const allProducts = this.models["product.template"].getAll(); + let list = []; + + if (searchWord !== "") { + list = this.addMainProductsToDisplay( + fuzzyLookup(unaccent(searchWord, false), allProducts, (product) => + unaccent(product.searchString, false) + ) + ); + } else if (this.selectedCategory?.id) { + list = this.selectedCategory.associatedProducts; + } else { + list = allProducts; + } + + if (!list || list.length === 0) { + return []; + } + + const excludedProductIds = [ + this.config.tip_product_id?.id, + ...this.hiddenProductIds, + ...this.session._pos_special_products_ids, + ]; + + list = list + .filter( + (product) => !excludedProductIds.includes(product.id) && product.available_in_pos + ) + .slice(0, 100); + + return searchWord !== "" + ? list.sort((a, b) => b.is_favorite - a.is_favorite) + : list.sort((a, b) => { + if (b.is_favorite !== a.is_favorite) { + return b.is_favorite - a.is_favorite; + } + return a.display_name.localeCompare(b.display_name); + }); + } } PosStore.prototype.electronic_payment_interfaces = {}; @@ -1914,7 +1972,7 @@ export function register_payment_method(use_payment_terminal, ImplementedPayment export const posService = { dependencies: PosStore.serviceDependencies, async start(env, deps) { - return new PosStore(env, deps).ready; + return new PosStore({ traps: {}, env, deps }).ready; }, }; diff --git a/addons/point_of_sale/static/src/lazy_getter.js b/addons/point_of_sale/static/src/lazy_getter.js new file mode 100644 index 0000000000000..8af5a790e1b9b --- /dev/null +++ b/addons/point_of_sale/static/src/lazy_getter.js @@ -0,0 +1,102 @@ +import { effect } from "@web/core/utils/reactive"; +import { getDisabler } from "./proxy_trap"; + +function getAllGetters(proto) { + const getterNames = new Set(); + const getters = new Set(); + while (proto !== null) { + const descriptors = Object.getOwnPropertyDescriptors(proto); + for (const [name, descriptor] of Object.entries(descriptors)) { + if (descriptor.get && !getterNames.has(name)) { + getterNames.add(name); + getters.add([name, descriptor.get]); + } + } + proto = Object.getPrototypeOf(proto); + } + return getters; +} + +const classGetters = new Map(); + +export function clearGettersCache() { + classGetters.clear(); +} + +function getGetters(Class) { + if (!classGetters.has(Class)) { + const getters = new Map(); + for (const [name, func] of getAllGetters(Class.prototype)) { + if (name.startsWith("__") && name.endsWith("__")) { + continue; + } + getters.set(name, [ + `__lazy_${name}`, + (obj) => { + return func.call(obj); + }, + ]); + } + classGetters.set(Class, getters); + } + return classGetters.get(Class); +} + +function defineLazyGetterTrap(Class) { + const getters = getGetters(Class); + return function get(target, prop, receiver) { + const disabler = getDisabler(target, prop); + if (disabler.isDisabled() || !getters.has(prop)) { + return Reflect.get(target, prop, receiver); + } + return disabler.call(() => { + const [lazyName] = getters.get(prop); + // For a getter, we should get the value from the receiver. + // Because the receiver is linked to the reactivity. + // We want to read the getter from it to make sure that the getter + // is part of the reactivity as well. + // To avoid infinite recursion, we disable this proxy trap + // during the time the lazy getter is accessed. + return receiver[lazyName]; + }); + }; +} + +function lazyComputed(obj, propName, compute) { + const key = Symbol(propName); + Object.defineProperty(obj, propName, { + get() { + return this[key](); + }, + configurable: true, + }); + + /** + * - `recompute` depends on the dependencies of `compute`. + * - When one of the dependencies of `compute` changed, `recompute` invalidates the cache of the `compute`. + * - The cache of `compute` is saved in `value`. + */ + effect( + function recompute(obj) { + const value = []; + obj[key] = () => { + if (!value.length) { + value.push(compute(obj)); + } + return value[0]; + }; + }, + [obj] + ); +} + +export class WithLazyGetterTrap { + constructor({ traps }) { + const Class = this.constructor; + const instance = new Proxy(this, { get: defineLazyGetterTrap(Class), ...traps }); + for (const [lazyName, func] of getGetters(Class).values()) { + lazyComputed(instance, lazyName, func); + } + return instance; + } +} diff --git a/addons/point_of_sale/static/src/proxy_trap.js b/addons/point_of_sale/static/src/proxy_trap.js new file mode 100644 index 0000000000000..92a3480c143bf --- /dev/null +++ b/addons/point_of_sale/static/src/proxy_trap.js @@ -0,0 +1,39 @@ +/** + * Instance of this class is useful in a context of a proxy trap handler. + * A trap handler's main purpose is to introduce a new behavior which is prone to + * being recursive. In this case, to avoid infinite recursion, we should be able to track the + * stack of calls and allow the handler to call the default behavior via the Reflect API. + * + * The idea is that when a block of code is called via the `call` method, we increment a counter. + * If the counter is greater than 0, then we know that we are in a recursive call. We can then + * use the `isDisabled` method to check if we should call the default behavior or not. + */ +export class TrapDisabler { + constructor() { + this.disabled = 0; + } + isDisabled() { + return this.disabled > 0; + } + call(fn, ...args) { + try { + this.disabled += 1; + return fn(...args); + } finally { + this.disabled -= 1; + } + } +} + +const disablerCaches = new WeakMap(); + +export function getDisabler(target, prop) { + if (!disablerCaches.has(target)) { + disablerCaches.set(target, new Map()); + } + const disablerCache = disablerCaches.get(target); + if (!disablerCache.has(prop)) { + disablerCache.set(prop, new TrapDisabler()); + } + return disablerCache.get(prop); +} diff --git a/addons/point_of_sale/static/tests/unit/lazy_getter.test.js b/addons/point_of_sale/static/tests/unit/lazy_getter.test.js new file mode 100644 index 0000000000000..40cb942d537be --- /dev/null +++ b/addons/point_of_sale/static/tests/unit/lazy_getter.test.js @@ -0,0 +1,340 @@ +import { describe, expect, mountOnFixture, test } from "@odoo/hoot"; +import { Component, onWillRender, reactive, useState, xml } from "@odoo/owl"; +import { animationFrame } from "@odoo/hoot-mock"; +import { WithLazyGetterTrap, clearGettersCache } from "@point_of_sale/lazy_getter"; +import { patch } from "@web/core/utils/patch"; +import { zip } from "@web/core/utils/arrays"; + +/** + * This returns an object which provides a custom `step` and `verifySteps` behavior. + * See the definition of each method for more details. + */ +function makeUnorderedVerifySteps() { + const steps = new Set(); + return { + /** + * It throws an error if the val already exists. + * @param {any} val + */ + step(val) { + if (steps.has(val)) { + throw new Error(`Step ${val} already exists`); + } + steps.add(val); + }, + /** + * Makes multiple assertions: + * - Are all items in `vals` in steps? + * - Are the items in `steps` ordered according to each item in `orderedValsArr`? + * Then it clears the `steps`. + * @param {any[]} vals + * @param {any[][]} [orderedValsArr] + */ + verifySteps(vals, orderedValsArr = []) { + vals.forEach((val) => expect(steps.has(val)).toBe(true)); + const valsSet = new Set(vals); + [...steps].forEach((val) => expect(valsSet.has(val)).toBe(true)); + + expect(steps.size).toBe(vals.length); + + const stepsArr = Array.from(steps); + orderedValsArr.forEach((orderedVals) => { + expect( + zip(orderedVals.slice(0, -1), orderedVals.slice(1)).reduce((acc, [a, b]) => { + return acc && stepsArr.indexOf(a) < stepsArr.indexOf(b); + }, true) + ).toEqual(true); + }); + + steps.clear(); + }, + }; +} + +const unorderedExpect = makeUnorderedVerifySteps(); + +/** + * ```mermaid + * graph TD; + * a --> ab + * b --> ab + * b --> bc + * c --> abc + * c --> bc + * c --> cd + * d --> cd + * ab --> abc + * abc --> x + * bc --> x + * cd --> y + * x --> y + * ``` + */ +class AppStore extends WithLazyGetterTrap { + constructor() { + super({ traps: {} }); + this.a = 0; + this.b = 0; + this.c = 0; + this.d = 0; + } + get ab() { + return this.a + this.b; + } + get abc() { + let result = 0; + for (let i = 0; i < 10; i++) { + result += this.ab; + } + return result + this.c; + } + get bc() { + return this.b + this.c; + } + get cd() { + return this.c + this.d; + } + get x() { + return this.abc + this.bc; + } + get y() { + return this.cd + this.x; + } +} + +class WithStore extends Component { + setup() { + this.store = useState(this.env.store); + onWillRender(() => this.onWillRender()); + } + onWillRender() {} +} + +class A extends WithStore { + static template = xml` + A: +`; +} + +class B extends WithStore { + static template = xml` + B: +`; +} + +class C extends WithStore { + static template = xml` + C: +`; +} + +class D extends WithStore { + static template = xml` + D: +`; +} + +class AB extends WithStore { + static template = xml` + AB: +`; +} + +class ABC extends WithStore { + static template = xml` + ABC: +`; +} + +class BC extends WithStore { + static template = xml` + BC: +`; +} + +class CD extends WithStore { + static template = xml` + CD: +`; +} + +class Root extends Component { + static components = { A, B, C, D, AB, ABC, BC, CD }; + static template = xml` + +`; +} + +describe("lazy getters", () => { + test("each getter should only be called once and only when needed", async () => { + const unpatch = patch(AppStore.prototype, { + get ab() { + unorderedExpect.step("ab"); + return super.ab; + }, + get abc() { + unorderedExpect.step("abc"); + return super.abc; + }, + get bc() { + unorderedExpect.step("bc"); + return super.bc; + }, + get cd() { + unorderedExpect.step("cd"); + return super.cd; + }, + }); + + const store = reactive(new AppStore()); + + await mountOnFixture(Root, { env: { store }, warnIfNoStaticProps: false }); + + unorderedExpect.verifySteps(["ab", "abc", "bc", "cd"]); + + store.a = 1; + + // Before rerendering, the getters should not be called + unorderedExpect.verifySteps([]); + + await animationFrame(); + // Only during rerendering that the getters are called + unorderedExpect.verifySteps(["ab", "abc"]); + + store.b = 1; + unorderedExpect.verifySteps([]); + await animationFrame(); + unorderedExpect.verifySteps(["bc", "ab", "abc"]); + + store.c = 1; + unorderedExpect.verifySteps([]); + await animationFrame(); + unorderedExpect.verifySteps(["cd", "bc", "abc"]); + + store.d = 1; + unorderedExpect.verifySteps([]); + await animationFrame(); + unorderedExpect.verifySteps(["cd"]); + + unpatch(); + clearGettersCache(); + }); + + test("only dependent components rerender", async () => { + const unpatches = [A, B, C, D, AB, ABC, CD, BC].map((Class) => { + return patch(Class.prototype, { + onWillRender() { + unorderedExpect.step(Class); + return super.onWillRender(); + }, + }); + }); + + const store = reactive(new AppStore()); + await mountOnFixture(Root, { env: { store }, warnIfNoStaticProps: false }); + unorderedExpect.verifySteps([A, B, C, D, AB, ABC, BC, CD]); + + store.a = 1; + await animationFrame(); + unorderedExpect.verifySteps([A, AB, ABC]); + + store.b = 1; + await animationFrame(); + unorderedExpect.verifySteps([B, AB, ABC, BC]); + + store.c = 1; + await animationFrame(); + unorderedExpect.verifySteps([C, ABC, BC, CD]); + + store.d = 1; + await animationFrame(); + unorderedExpect.verifySteps([D, CD]); + + for (const unpatch of unpatches) { + unpatch(); + } + clearGettersCache(); + }); + + test("only dependent getters are called and in correct order", () => { + clearGettersCache(); + + const unpatch = patch(AppStore.prototype, { + get ab() { + const result = super.ab; + unorderedExpect.step("ab"); + return result; + }, + get abc() { + const result = super.abc; + unorderedExpect.step("abc"); + return result; + }, + get bc() { + const result = super.bc; + unorderedExpect.step("bc"); + return result; + }, + get cd() { + const result = super.cd; + unorderedExpect.step("cd"); + return result; + }, + get x() { + const result = super.x; + unorderedExpect.step("x"); + return result; + }, + get y() { + const result = super.y; + unorderedExpect.step("y"); + return result; + }, + }); + const store = reactive(new AppStore()); + + expect(store.y).toBe(0); + unorderedExpect.verifySteps(["ab", "bc", "cd", "abc", "x", "y"], [["ab", "abc", "x", "y"]]); + + store.a = 1; + expect(store.y).toBe(10); + unorderedExpect.verifySteps(["ab", "abc", "x", "y"], [["ab", "abc", "x", "y"]]); + + store.b = 1; + expect(store.y).toBe(21); + unorderedExpect.verifySteps( + ["ab", "bc", "abc", "x", "y"], + [ + ["ab", "abc", "x", "y"], + ["bc", "x", "y"], + ] + ); + + store.c = 1; + expect(store.y).toBe(24); + unorderedExpect.verifySteps( + ["abc", "bc", "cd", "x", "y"], + [ + ["abc", "x", "y"], + ["bc", "x", "y"], + ["cd", "y"], + ] + ); + + store.d = 1; + expect(store.y).toBe(25); + unorderedExpect.verifySteps(["cd", "y"], [["cd", "y"]]); + + unpatch(); + clearGettersCache(); + }); +}); diff --git a/addons/pos_restaurant/static/src/app/models/restaurant_table.js b/addons/pos_restaurant/static/src/app/models/restaurant_table.js index 213605198da04..91d1b40429d3a 100644 --- a/addons/pos_restaurant/static/src/app/models/restaurant_table.js +++ b/addons/pos_restaurant/static/src/app/models/restaurant_table.js @@ -62,7 +62,7 @@ export class RestaurantTable extends Base { y: this.getY() + this.height / 2, }; } - get orders() { + getOrders() { return this.models["pos.order"].filter( (o) => o.table_id?.id === this.id && diff --git a/addons/pos_restaurant/static/src/app/services/pos_store.js b/addons/pos_restaurant/static/src/app/services/pos_store.js index 61d1bf0f4f885..4ae83d6cfbe94 100644 --- a/addons/pos_restaurant/static/src/app/services/pos_store.js +++ b/addons/pos_restaurant/static/src/app/services/pos_store.js @@ -219,11 +219,9 @@ patch(PosStore.prototype, { async setTable(table, orderUuid = null) { this.loadingOrderState = true; - const tableOrders = table.orders; - - let currentOrder = tableOrders.find((order) => - orderUuid ? order.uuid === orderUuid : !order.finalized - ); + let currentOrder = table + .getOrders() + .find((order) => (orderUuid ? order.uuid === orderUuid : !order.finalized)); if (currentOrder) { this.setOrder(currentOrder); @@ -244,7 +242,7 @@ patch(PosStore.prototype, { this.loadingOrderState = true; const orders = await this.syncAllOrders({ throw: true }); const orderUuids = orders.map((order) => order.uuid); - for (const order of table.orders) { + for (const order of table.getOrders()) { if ( !orderUuids.includes(order.uuid) && typeof order.id === "number" && diff --git a/addons/pos_self_order/__manifest__.py b/addons/pos_self_order/__manifest__.py index a218e073c7c32..3052a97ae3f65 100644 --- a/addons/pos_self_order/__manifest__.py +++ b/addons/pos_self_order/__manifest__.py @@ -38,6 +38,8 @@ 'web/static/src/core/currency.js', 'barcodes/static/src/barcode_service.js', 'point_of_sale/static/src/utils.js', + 'point_of_sale/static/src/proxy_trap.js', + 'point_of_sale/static/src/lazy_getter.js', 'web/static/lib/bootstrap/js/dist/util/index.js', 'web/static/lib/bootstrap/js/dist/dom/data.js', 'web/static/lib/bootstrap/js/dist/dom/event-handler.js',