chore(pricing): Pricing retrieval improvements (#12128)
**What** I have removed the check for the context key where it was fetching all attributes available and then stripping out the one that does not exists.. On big dataset these would remove multiple hundreds of ms of query execution
This commit is contained in:
1 parent
6ae1e7b708
commit
07252691c5
8 files changed
+671
-127
No files matched your search
+165
@@ -286,6 +286,171 @@ moduleIntegrationTestRunner<IPricingModuleService>({
|
||||
})
|
||||
})
|
||||
|
||||
it("should successfully calculate prices with complex context", async () => {
|
||||
const context = {
|
||||
id: "cart_01JRDH08QD8CZ0KJDVE410KM1J",
|
||||
currency_code: "PLN",
|
||||
email: "tony@stark-industries.com",
|
||||
region_id: "reg_01JRDH08ENY3276P6133BVXGWJ",
|
||||
created_at: "2025-04-09T14:59:24.526Z",
|
||||
updated_at: "2025-04-09T14:59:24.526Z",
|
||||
completed_at: null,
|
||||
total: 1500,
|
||||
subtotal: 1428.5714285714287,
|
||||
tax_total: 71.42857142857143,
|
||||
discount_total: 0,
|
||||
discount_subtotal: 0,
|
||||
discount_tax_total: 0,
|
||||
original_total: 1500,
|
||||
original_tax_total: 71.42857142857143,
|
||||
item_total: 1500,
|
||||
item_subtotal: 1428.5714285714287,
|
||||
item_tax_total: 71.42857142857143,
|
||||
original_item_total: 1500,
|
||||
original_item_subtotal: 1428.5714285714287,
|
||||
original_item_tax_total: 71.42857142857143,
|
||||
shipping_total: 0,
|
||||
shipping_subtotal: 0,
|
||||
shipping_tax_total: 0,
|
||||
original_shipping_tax_total: 0,
|
||||
original_shipping_subtotal: 0,
|
||||
original_shipping_total: 0,
|
||||
credit_line_subtotal: 0,
|
||||
credit_line_tax_total: 0,
|
||||
credit_line_total: 0,
|
||||
metadata: null,
|
||||
sales_channel_id: "sc_01JRDH08KWX1AR5SB0A3THWWQQ",
|
||||
shipping_address_id: "caaddr_01JRDH08QDXHV9SJXKHT04TXK0",
|
||||
customer_id: "cus_01JRDH08ATYB5AMFEZDTWCQWNK",
|
||||
items: [
|
||||
{
|
||||
id: "cali_01JRDH08QDQH3CB1DE4S79HREC",
|
||||
thumbnail: null,
|
||||
variant_id: "variant_01JRDH08GJCZQB4GZCDDTYMD1V",
|
||||
product_id: "prod_01JRDH08FPZ6QBZQ096B310RM7",
|
||||
product_type_id: null,
|
||||
product_title: "Medusa T-Shirt",
|
||||
product_description: null,
|
||||
product_subtitle: null,
|
||||
product_type: null,
|
||||
product_collection: null,
|
||||
product_handle: "t-shirt",
|
||||
variant_sku: "SHIRT-S-BLACK",
|
||||
variant_barcode: null,
|
||||
variant_title: "S / Black",
|
||||
requires_shipping: true,
|
||||
metadata: {},
|
||||
created_at: "2025-04-09T14:59:24.526Z",
|
||||
updated_at: "2025-04-09T14:59:24.526Z",
|
||||
title: "S / Black",
|
||||
quantity: 1,
|
||||
unit_price: 1500,
|
||||
compare_at_unit_price: null,
|
||||
is_tax_inclusive: true,
|
||||
tax_lines: [
|
||||
{
|
||||
id: "calitxl_01JRDH08RJEQ4WXXDTJYWV7B4M",
|
||||
description: "CA Default Rate",
|
||||
code: "CADEFAULT",
|
||||
rate: 5,
|
||||
provider_id: "system",
|
||||
},
|
||||
],
|
||||
adjustments: [],
|
||||
product: {
|
||||
id: "prod_01JRDH08FPZ6QBZQ096B310RM7",
|
||||
collection_id: null,
|
||||
type_id: null,
|
||||
categories: [],
|
||||
tags: [],
|
||||
},
|
||||
},
|
||||
],
|
||||
shipping_methods: [],
|
||||
shipping_address: {
|
||||
id: "caaddr_01JRDH08QDXHV9SJXKHT04TXK0",
|
||||
first_name: null,
|
||||
last_name: null,
|
||||
company: null,
|
||||
address_1: "test address 1",
|
||||
address_2: "test address 2",
|
||||
city: "SF",
|
||||
postal_code: "94016",
|
||||
country_code: "US",
|
||||
province: "CA",
|
||||
phone: null,
|
||||
},
|
||||
billing_address: null,
|
||||
credit_lines: [],
|
||||
customer: {
|
||||
id: "cus_01JRDH08ATYB5AMFEZDTWCQWNK",
|
||||
email: "tony@stark-industries.com",
|
||||
groups: [],
|
||||
},
|
||||
region: {
|
||||
id: "reg_01JRDH08ENY3276P6133BVXGWJ",
|
||||
name: "US",
|
||||
currency_code: "usd",
|
||||
automatic_taxes: true,
|
||||
countries: [
|
||||
{
|
||||
iso_2: "us",
|
||||
iso_3: "usa",
|
||||
num_code: "840",
|
||||
name: "UNITED STATES",
|
||||
display_name: "United States",
|
||||
region_id: "reg_01JRDH08ENY3276P6133BVXGWJ",
|
||||
metadata: null,
|
||||
created_at: "2025-04-09T14:59:20.275Z",
|
||||
updated_at: "2025-04-09T14:59:24.250Z",
|
||||
deleted_at: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
promotions: [],
|
||||
}
|
||||
|
||||
const calculatedPrice = await service.calculatePrices(
|
||||
{ id: ["price-set-EUR", "price-set-PLN"] },
|
||||
{ context: context as any }
|
||||
)
|
||||
|
||||
expect(calculatedPrice).toEqual([
|
||||
{
|
||||
id: "price-set-PLN",
|
||||
is_calculated_price_price_list: false,
|
||||
is_calculated_price_tax_inclusive: false,
|
||||
calculated_amount: 1000,
|
||||
raw_calculated_amount: {
|
||||
value: "1000",
|
||||
precision: 20,
|
||||
},
|
||||
is_original_price_price_list: false,
|
||||
is_original_price_tax_inclusive: false,
|
||||
original_amount: 1000,
|
||||
raw_original_amount: {
|
||||
value: "1000",
|
||||
precision: 20,
|
||||
},
|
||||
currency_code: "PLN",
|
||||
calculated_price: {
|
||||
id: "price-PLN",
|
||||
price_list_id: null,
|
||||
price_list_type: null,
|
||||
min_quantity: 1,
|
||||
max_quantity: 10,
|
||||
},
|
||||
original_price: {
|
||||
id: "price-PLN",
|
||||
price_list_id: null,
|
||||
price_list_type: null,
|
||||
min_quantity: 1,
|
||||
max_quantity: 10,
|
||||
},
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
it("should throw an error when currency code is not set", async () => {
|
||||
let result = service.calculatePrices(
|
||||
{ id: ["price-set-EUR", "price-set-PLN"] },
|
||||
|
||||
@@ -795,6 +795,15 @@
|
||||
"unique": false,
|
||||
"expression": "CREATE UNIQUE INDEX IF NOT EXISTS \"IDX_price_rule_price_id_attribute_operator_unique\" ON \"price_rule\" (price_id, attribute, operator) WHERE deleted_at IS NULL"
|
||||
},
|
||||
{
|
||||
"keyName": "IDX_price_rule_attribute",
|
||||
"columnNames": [],
|
||||
"composite": false,
|
||||
"constraint": false,
|
||||
"primary": false,
|
||||
"unique": false,
|
||||
"expression": "CREATE INDEX IF NOT EXISTS \"IDX_price_rule_attribute\" ON \"price_rule\" (attribute) WHERE deleted_at IS NULL"
|
||||
},
|
||||
{
|
||||
"keyName": "IDX_price_rule_attribute_value",
|
||||
"columnNames": [],
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
import { Migration } from '@mikro-orm/migrations';
|
||||
|
||||
export class Migration20250409122219 extends Migration {
|
||||
|
||||
override async up(): Promise<void> {
|
||||
this.addSql(`CREATE INDEX IF NOT EXISTS "IDX_price_rule_attribute" ON "price_rule" (attribute) WHERE deleted_at IS NULL;`);
|
||||
}
|
||||
|
||||
override async down(): Promise<void> {
|
||||
this.addSql(`drop index if exists "IDX_price_rule_attribute";`);
|
||||
}
|
||||
|
||||
}
|
||||
@@ -18,6 +18,10 @@ const PriceRule = model
|
||||
where: "deleted_at IS NULL",
|
||||
unique: true,
|
||||
},
|
||||
{
|
||||
on: ["attribute"],
|
||||
where: "deleted_at IS NULL",
|
||||
},
|
||||
{
|
||||
on: ["attribute", "value"],
|
||||
where: "deleted_at IS NULL",
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
MedusaError,
|
||||
MikroOrmBase,
|
||||
PriceListStatus,
|
||||
promiseAll,
|
||||
} from "@medusajs/framework/utils"
|
||||
|
||||
import {
|
||||
@@ -16,8 +15,6 @@ import {
|
||||
} from "@medusajs/framework/types"
|
||||
import { Knex, SqlEntityManager } from "@mikro-orm/postgresql"
|
||||
|
||||
// Simple cache implementation
|
||||
|
||||
export class PricingRepository
|
||||
extends MikroOrmBase
|
||||
implements PricingRepositoryService
|
||||
@@ -53,31 +50,20 @@ export class PricingRepository
|
||||
}
|
||||
|
||||
// Generate flatten key-value pairs for rule matching
|
||||
const [ruleAttributes, priceListRuleAttributes] =
|
||||
await this.getAttributesFromRuleTables(knex)
|
||||
|
||||
const allowedRuleAttributes = [
|
||||
...ruleAttributes,
|
||||
...priceListRuleAttributes,
|
||||
]
|
||||
|
||||
const flattenedKeyValuePairs = flattenObjectToKeyValuePairs(context)
|
||||
|
||||
// First filter by value presence
|
||||
const flattenedContext = Object.entries(flattenedKeyValuePairs).filter(
|
||||
([key, value]) => {
|
||||
([, value]) => {
|
||||
const isValuePresent = !Array.isArray(value) && isPresent(value)
|
||||
const isArrayPresent = Array.isArray(value) && value.flat(1).length
|
||||
|
||||
return (
|
||||
allowedRuleAttributes.includes(key) &&
|
||||
(isValuePresent || isArrayPresent)
|
||||
)
|
||||
return isValuePresent || isArrayPresent
|
||||
}
|
||||
)
|
||||
|
||||
const hasComplexContext = flattenedContext.length > 0
|
||||
|
||||
// Base query with efficient index lookups
|
||||
const query = knex
|
||||
.select({
|
||||
id: "price.id",
|
||||
@@ -97,7 +83,6 @@ export class PricingRepository
|
||||
.andWhere("price.currency_code", currencyCode)
|
||||
.whereNull("price.deleted_at")
|
||||
|
||||
// Apply quantity filter
|
||||
if (quantity !== undefined) {
|
||||
query.andWhere(function (this: Knex.QueryBuilder) {
|
||||
this.where(function (this: Knex.QueryBuilder) {
|
||||
@@ -118,7 +103,6 @@ export class PricingRepository
|
||||
})
|
||||
}
|
||||
|
||||
// Efficient price list join with index usage
|
||||
query.leftJoin("price_list as pl", function (this: Knex.JoinClause) {
|
||||
this.on("pl.id", "=", "price.price_list_id")
|
||||
.andOn("pl.status", "=", knex.raw("?", [PriceListStatus.ACTIVE]))
|
||||
@@ -133,9 +117,7 @@ export class PricingRepository
|
||||
})
|
||||
})
|
||||
|
||||
// OPTIMIZATION: Only add complex rule filtering when necessary
|
||||
if (hasComplexContext) {
|
||||
// For price rules - direct check that ALL rules match
|
||||
const priceRuleConditions = knex.raw(
|
||||
`
|
||||
(
|
||||
@@ -188,7 +170,6 @@ export class PricingRepository
|
||||
})
|
||||
)
|
||||
|
||||
// For price list rules - direct check that ALL rules match
|
||||
const priceListRuleConditions = knex.raw(
|
||||
`
|
||||
(
|
||||
@@ -231,7 +212,6 @@ export class PricingRepository
|
||||
})
|
||||
})
|
||||
} else {
|
||||
// Simple case - just get prices with no rules or price lists with no rules
|
||||
query.where(function (this: Knex.QueryBuilder) {
|
||||
this.where("price.rules_count", 0).orWhere(function (
|
||||
this: Knex.QueryBuilder
|
||||
@@ -241,31 +221,13 @@ export class PricingRepository
|
||||
})
|
||||
}
|
||||
|
||||
// Optimized ordering to help query planner and preserve price list precedence
|
||||
query
|
||||
.orderByRaw("price.price_list_id IS NOT NULL DESC")
|
||||
.orderByRaw("price.rules_count + COALESCE(pl.rules_count, 0) DESC") // More specific rules first
|
||||
.orderBy("pl.id", "asc") // Order by price list ID to ensure first created price list takes precedence
|
||||
.orderBy("price.amount", "asc") // For non-price list prices, cheaper ones first
|
||||
.orderByRaw("price.rules_count + COALESCE(pl.rules_count, 0) DESC")
|
||||
.orderBy("pl.id", "asc")
|
||||
.orderBy("price.amount", "asc")
|
||||
|
||||
// Execute the optimized query
|
||||
console.log(query.toString())
|
||||
return await query
|
||||
}
|
||||
|
||||
// Helper method to get attributes from rule tables
|
||||
private async getAttributesFromRuleTables(knex: Knex) {
|
||||
// Using distinct queries for better performance
|
||||
const priceRuleAttributesQuery = knex("price_rule")
|
||||
.distinct("attribute")
|
||||
.pluck("attribute")
|
||||
|
||||
const priceListRuleAttributesQuery = knex("price_list_rule")
|
||||
.distinct("attribute")
|
||||
.pluck("attribute")
|
||||
|
||||
return await promiseAll([
|
||||
priceRuleAttributesQuery,
|
||||
priceListRuleAttributesQuery,
|
||||
])
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user