From 3a1cf2212ac3d92eebee1bbea07e8731e53e4c72 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Thu, 10 Apr 2025 17:55:35 +0200 Subject: [PATCH] chore: Cache available price rule attributes (#12144) **What** We found out that the pricing context from the cart always contains the entire cart, even though it is kind of wrong. The issue is that even though we improve the performances of the query, it will cost a lot to have hundreds of constraint for nothing potentially. For that reason, we cache the attributes in memory with the best possible query we can do to gather them and we renew them when we perform a calculate prices if it has been reset. That way, we ensure we don't have unnecessary checks on attributes that does not have rules. Since we don't have the type table anymore which was doing that for us and until we have a proper caching layer it would do IMO. But the rules type table was very useful for these attributes findings --- .changeset/mighty-stingrays-protect.md | 5 + .../pricing/src/repositories/pricing.ts | 44 ++++- .../pricing/src/services/pricing-module.ts | 163 ++++++++++++++---- 3 files changed, 176 insertions(+), 36 deletions(-) create mode 100644 .changeset/mighty-stingrays-protect.md diff --git a/.changeset/mighty-stingrays-protect.md b/.changeset/mighty-stingrays-protect.md new file mode 100644 index 0000000000..715b6d7e21 --- /dev/null +++ b/.changeset/mighty-stingrays-protect.md @@ -0,0 +1,5 @@ +--- +"@medusajs/pricing": patch +--- + +Chore/pricing cache available keys diff --git a/packages/modules/pricing/src/repositories/pricing.ts b/packages/modules/pricing/src/repositories/pricing.ts index 44f59c55bc..43bee3505b 100644 --- a/packages/modules/pricing/src/repositories/pricing.ts +++ b/packages/modules/pricing/src/repositories/pricing.ts @@ -19,12 +19,46 @@ export class PricingRepository extends MikroOrmBase implements PricingRepositoryService { + #availableAttributes: Set = new Set() + constructor() { // @ts-ignore // eslint-disable-next-line prefer-rest-params super(...arguments) } + clearAvailableAttributes() { + this.#availableAttributes.clear() + } + + async #cacheAvailableAttributes() { + const manager = this.getActiveManager() + const knex = manager.getKnex() + + const { rows } = await knex.raw( + ` + SELECT DISTINCT attribute + FROM ( + SELECT attribute + FROM price_rule + UNION ALL + SELECT attribute + FROM price_list_rule + ) as combined_rules_attributes + ` + ) + this.#availableAttributes.clear() + rows.forEach(({ attribute }) => { + this.#availableAttributes.add(attribute) + }) + } + + async #cacheAvailableAttributesIfNecessary() { + if (this.#availableAttributes.size === 0) { + await this.#cacheAvailableAttributes() + } + } + async calculatePrices( pricingFilters: PricingFilters, pricingContext: PricingContext = { context: {} }, @@ -53,7 +87,7 @@ export class PricingRepository const flattenedKeyValuePairs = flattenObjectToKeyValuePairs(context) // First filter by value presence - const flattenedContext = Object.entries(flattenedKeyValuePairs).filter( + let flattenedContext = Object.entries(flattenedKeyValuePairs).filter( ([, value]) => { const isValuePresent = !Array.isArray(value) && isPresent(value) const isArrayPresent = Array.isArray(value) && value.flat(1).length @@ -62,6 +96,13 @@ export class PricingRepository } ) + if (flattenedContext.length > 10) { + await this.#cacheAvailableAttributesIfNecessary() + flattenedContext = flattenedContext.filter(([key]) => + this.#availableAttributes.has(key) + ) + } + const hasComplexContext = flattenedContext.length > 0 const query = knex @@ -227,7 +268,6 @@ export class PricingRepository .orderBy("pl.id", "asc") .orderBy("price.amount", "asc") - console.log(query.toString()) return await query } } diff --git a/packages/modules/pricing/src/services/pricing-module.ts b/packages/modules/pricing/src/services/pricing-module.ts index aa19b9d305..47673d4eb6 100644 --- a/packages/modules/pricing/src/services/pricing-module.ts +++ b/packages/modules/pricing/src/services/pricing-module.ts @@ -77,24 +77,28 @@ const generateMethodForModels = { PricePreference, } +const BaseClass = ModulesSdkUtils.MedusaService<{ + PriceSet: { dto: PricingTypes.PriceSetDTO } + Price: { dto: PricingTypes.PriceDTO } + PriceRule: { + dto: PricingTypes.PriceRuleDTO + create: PricingTypes.CreatePriceRuleDTO + update: PricingTypes.UpdatePriceRuleDTO + } + PriceList: { dto: PricingTypes.PriceListDTO } + PriceListRule: { dto: PricingTypes.PriceListRuleDTO } + // PricePreference: { dto: PricingTypes.PricePreferenceDTO } + PricePreference: { dto: any } +}>(generateMethodForModels) + export default class PricingModuleService - extends ModulesSdkUtils.MedusaService<{ - PriceSet: { dto: PricingTypes.PriceSetDTO } - Price: { dto: PricingTypes.PriceDTO } - PriceRule: { - dto: PricingTypes.PriceRuleDTO - create: PricingTypes.CreatePriceRuleDTO - update: PricingTypes.UpdatePriceRuleDTO - } - PriceList: { dto: PricingTypes.PriceListDTO } - PriceListRule: { dto: PricingTypes.PriceListRuleDTO } - // PricePreference: { dto: PricingTypes.PricePreferenceDTO } - PricePreference: { dto: any } - }>(generateMethodForModels) + extends BaseClass implements PricingTypes.IPricingModuleService { protected baseRepository_: DAL.RepositoryService - protected readonly pricingRepository_: PricingRepositoryService + protected readonly pricingRepository_: PricingRepositoryService & { + clearAvailableAttributes?: () => Promise + } protected readonly priceSetService_: ModulesSdkTypes.IMedusaInternalService< InferEntityType > @@ -164,6 +168,52 @@ export default class PricingModuleService return pricingContext } + // @ts-expect-error + async createPriceRules( + ...args: Parameters + ): Promise { + try { + return await super.createPriceRules(...args) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } + } + + // @ts-expect-error + async updatePriceRules( + ...args: Parameters + ): Promise { + try { + return await super.updatePriceRules(...args) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } + } + + // @ts-expect-error + async createPriceListRules( + ...args: any[] + ): Promise { + try { + // @ts-ignore + return await super.createPriceListRules(...args) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } + } + + // @ts-expect-error + async updatePriceListRules( + ...args: any[] + ): Promise { + try { + // @ts-ignore + return await super.updatePriceListRules(...args) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } + } + @InjectManager() // @ts-expect-error async retrievePriceSet( @@ -458,9 +508,13 @@ export default class PricingModuleService return dbPriceSets.find((p) => p.id === priceSet.id)! }) - return await this.baseRepository_.serialize( - Array.isArray(data) ? results : results[0] - ) + try { + return await this.baseRepository_.serialize( + Array.isArray(data) ? results : results[0] + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } async upsertPriceSets( @@ -496,9 +550,14 @@ export default class PricingModuleService } const result = (await promiseAll(operations)).flat() - return await this.baseRepository_.serialize( - Array.isArray(data) ? result : result[0] - ) + + try { + return await this.baseRepository_.serialize( + Array.isArray(data) ? result : result[0] + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } // @ts-expect-error @@ -548,7 +607,11 @@ export default class PricingModuleService PriceSetDTO[] | PriceSetDTO >(updateResult) - return isString(idOrSelector) ? priceSets[0] : priceSets + try { + return isString(idOrSelector) ? priceSets[0] : priceSets + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectTransactionManager() @@ -744,7 +807,11 @@ export default class PricingModuleService return dbPrices.find((p) => p.id === inputItem.priceSetId)! }) - return Array.isArray(data) ? orderedPriceSets : orderedPriceSets[0] + try { + return Array.isArray(data) ? orderedPriceSets : orderedPriceSets[0] + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager() @@ -756,9 +823,13 @@ export default class PricingModuleService ): Promise { const priceLists = await this.createPriceLists_(data, sharedContext) - return await this.baseRepository_.serialize( - priceLists - ) + try { + return await this.baseRepository_.serialize( + priceLists + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectTransactionManager() @@ -770,9 +841,13 @@ export default class PricingModuleService ): Promise { const priceLists = await this.updatePriceLists_(data, sharedContext) - return await this.baseRepository_.serialize( - priceLists - ) + try { + return await this.baseRepository_.serialize( + priceLists + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager() @@ -783,7 +858,13 @@ export default class PricingModuleService ): Promise { const prices = await this.updatePriceListPrices_(data, sharedContext) - return await this.baseRepository_.serialize(prices) + try { + return await this.baseRepository_.serialize( + prices + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager() @@ -792,7 +873,11 @@ export default class PricingModuleService ids: string[], @MedusaContext() sharedContext: Context = {} ): Promise { - await this.removePrices_(ids, sharedContext) + try { + await this.removePrices_(ids, sharedContext) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager() @@ -803,7 +888,13 @@ export default class PricingModuleService ): Promise { const prices = await this.addPriceListPrices_(data, sharedContext) - return await this.baseRepository_.serialize(prices) + try { + return await this.baseRepository_.serialize( + prices + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager() @@ -814,9 +905,13 @@ export default class PricingModuleService ): Promise { const [priceList] = await this.setPriceListRules_([data], sharedContext) - return await this.baseRepository_.serialize( - priceList - ) + try { + return await this.baseRepository_.serialize( + priceList + ) + } finally { + this.pricingRepository_.clearAvailableAttributes?.() + } } @InjectManager()