From 71f3eab1a23b02fa42e6570d1c71c9deedd5fec5 Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Fri, 13 Dec 2019 11:51:38 +0100 Subject: [PATCH] perf(core): Optimize OrderCalculator logic to improve performance Relates to #226. This commit introduces a number of performance improvements when modifying Orders and subsequently re-calculating the totals, including taxes and promotions. The logic is now a little more complex because more work is done to track changes and therefore limit DB updates to only the needed OrderItems, whereas previously any change to the Order resulted in updates to every single OrderItem. BREAKING CHANGE: The `Order` entity now has a new column, `taxZoneId`. This is used to more efficiently track changes to the active tax zone, and therefore reduce the number of tax calculations to be performed on an Order. This change will require a migration which should be routine. --- .../entity/order-item/order-item.entity.ts | 6 + .../core/src/entity/order/order.entity.ts | 5 +- .../order-calculator/order-calculator.spec.ts | 10 +- .../order-calculator/order-calculator.ts | 143 ++++++++++++++---- .../src/service/services/order.service.ts | 19 ++- 5 files changed, 140 insertions(+), 43 deletions(-) diff --git a/packages/core/src/entity/order-item/order-item.entity.ts b/packages/core/src/entity/order-item/order-item.entity.ts index e1a651d5ba..3a8032da1f 100644 --- a/packages/core/src/entity/order-item/order-item.entity.ts +++ b/packages/core/src/entity/order-item/order-item.entity.ts @@ -65,6 +65,9 @@ export class OrderItem extends VendureEntity { */ @Calculated() get adjustments(): Adjustment[] { + if (!this.pendingAdjustments) { + return []; + } if (this.unitPriceIncludesTax) { return this.pendingAdjustments; } else { @@ -106,6 +109,9 @@ export class OrderItem extends VendureEntity { } get promotionAdjustmentsTotal(): number { + if (!this.pendingAdjustments) { + return 0; + } return this.pendingAdjustments .filter(a => a.type === AdjustmentType.PROMOTION) .reduce((total, a) => total + a.amount, 0); diff --git a/packages/core/src/entity/order/order.entity.ts b/packages/core/src/entity/order/order.entity.ts index e17cc77651..6b6ce3cbf0 100644 --- a/packages/core/src/entity/order/order.entity.ts +++ b/packages/core/src/entity/order/order.entity.ts @@ -91,6 +91,9 @@ export class Order extends VendureEntity implements HasCustomFields { @Column(type => CustomOrderFields) customFields: CustomOrderFields; + @EntityId({ nullable: true }) + taxZoneId?: ID; + @Calculated() get totalBeforeTax(): number { return this.subTotalBeforeTax + this.promotionAdjustmentsTotal + (this.shipping || 0); @@ -103,7 +106,7 @@ export class Order extends VendureEntity implements HasCustomFields { @Calculated() get adjustments(): Adjustment[] { - return this.pendingAdjustments; + return this.pendingAdjustments || []; } get promotionAdjustmentsTotal(): number { diff --git a/packages/core/src/service/helpers/order-calculator/order-calculator.spec.ts b/packages/core/src/service/helpers/order-calculator/order-calculator.spec.ts index b43571c977..ba5b8dba2b 100644 --- a/packages/core/src/service/helpers/order-calculator/order-calculator.spec.ts +++ b/packages/core/src/service/helpers/order-calculator/order-calculator.spec.ts @@ -458,10 +458,12 @@ describe('OrderCalculator', () => { // bring the order total below the threshold for the second promotion. order.lines[0].items.push(new OrderItem({ unitPrice: 42 })); - await orderCalculator.applyPriceAdjustments(ctx, order, [ - buy3Get10pcOffOrder, - spend100Get10pcOffOrder, - ]); + await orderCalculator.applyPriceAdjustments( + ctx, + order, + [buy3Get10pcOffOrder, spend100Get10pcOffOrder], + order.lines[0], + ); expect(order.subTotal).toBe(150); expect(order.adjustments.length).toBe(1); diff --git a/packages/core/src/service/helpers/order-calculator/order-calculator.ts b/packages/core/src/service/helpers/order-calculator/order-calculator.ts index 0c4b1b7f1f..945a896a8d 100644 --- a/packages/core/src/service/helpers/order-calculator/order-calculator.ts +++ b/packages/core/src/service/helpers/order-calculator/order-calculator.ts @@ -10,7 +10,7 @@ import { RequestContext } from '../../../api/common/request-context'; import { idsAreEqual } from '../../../common/utils'; import { PromotionUtils, ShippingCalculationResult } from '../../../config'; import { ConfigService } from '../../../config/config.service'; -import { OrderLine, ProductVariant } from '../../../entity'; +import { OrderItem, OrderLine, ProductVariant, TaxCategory, TaxRate } from '../../../entity'; import { Order } from '../../../entity/order/order.entity'; import { Promotion } from '../../../entity/promotion/promotion.entity'; import { ShippingMethod } from '../../../entity/shipping-method/shipping-method.entity'; @@ -33,76 +33,145 @@ export class OrderCalculator { /** * Applies taxes and promotions to an Order. Mutates the order object. + * Returns an array of any OrderItems which had new adjustments + * applied, either tax or promotions. */ - async applyPriceAdjustments(ctx: RequestContext, order: Order, promotions: Promotion[]): Promise { + async applyPriceAdjustments( + ctx: RequestContext, + order: Order, + promotions: Promotion[], + updatedOrderLine?: OrderLine, + ): Promise { const { taxZoneStrategy } = this.configService.taxOptions; const zones = this.zoneService.findAll(ctx); const activeTaxZone = taxZoneStrategy.determineTaxZone(zones, ctx.channel, order); + let taxZoneChanged = false; + if (!order.taxZoneId || !idsAreEqual(order.taxZoneId, activeTaxZone.id)) { + order.taxZoneId = activeTaxZone.id; + taxZoneChanged = true; + } + const updatedOrderItems = new Set(); + if (updatedOrderLine) { + this.applyTaxesToOrderLine( + ctx, + updatedOrderLine, + activeTaxZone, + this.createTaxRateGetter(activeTaxZone), + ); + updatedOrderLine.activeItems.forEach(item => updatedOrderItems.add(item)); + } order.clearAdjustments(); + this.calculateOrderTotals(order); if (order.lines.length) { - // First apply taxes to the non-discounted prices - this.applyTaxes(ctx, order, activeTaxZone); + if (taxZoneChanged) { + // First apply taxes to the non-discounted prices + this.applyTaxes(ctx, order, activeTaxZone); + } + // Then test and apply promotions - await this.applyPromotions(order, promotions); - // Finally, re-calculate taxes because the promotions may have - // altered the unit prices, which in turn will alter the tax payable. - this.applyTaxes(ctx, order, activeTaxZone); + const totalBeforePromotions = order.total; + const itemsModifiedByPromotions = await this.applyPromotions(order, promotions); + itemsModifiedByPromotions.forEach(item => updatedOrderItems.add(item)); + + if (order.total !== totalBeforePromotions || itemsModifiedByPromotions.length) { + // Finally, re-calculate taxes because the promotions may have + // altered the unit prices, which in turn will alter the tax payable. + this.applyTaxes(ctx, order, activeTaxZone); + } await this.applyShipping(ctx, order); - } else { - this.calculateOrderTotals(order); } - return order; + this.calculateOrderTotals(order); + return taxZoneChanged ? order.getOrderItems() : Array.from(updatedOrderItems); } /** * Applies the correct TaxRate to each OrderItem in the order. */ private applyTaxes(ctx: RequestContext, order: Order, activeZone: Zone) { + const getTaxRate = this.createTaxRateGetter(activeZone); for (const line of order.lines) { - line.clearAdjustments(AdjustmentType.TAX); + this.applyTaxesToOrderLine(ctx, line, activeZone, getTaxRate); + } + this.calculateOrderTotals(order); + } - const applicableTaxRate = this.taxRateService.getApplicableTaxRate(activeZone, line.taxCategory); - const { price, priceIncludesTax, priceWithTax, priceWithoutTax } = this.taxCalculator.calculate( - line.unitPrice, - line.taxCategory, - activeZone, - ctx, - ); + /** + * Applies the correct TaxRate to an OrderLine + */ + private applyTaxesToOrderLine( + ctx: RequestContext, + line: OrderLine, + activeZone: Zone, + getTaxRate: (taxCategory: TaxCategory) => TaxRate, + ) { + line.clearAdjustments(AdjustmentType.TAX); - line.setUnitPriceIncludesTax(priceIncludesTax); - line.setTaxRate(applicableTaxRate.value); + const applicableTaxRate = getTaxRate(line.taxCategory); + const { price, priceIncludesTax, priceWithTax, priceWithoutTax } = this.taxCalculator.calculate( + line.unitPrice, + line.taxCategory, + activeZone, + ctx, + ); + for (const item of line.activeItems) { + item.unitPriceIncludesTax = priceIncludesTax; + item.taxRate = applicableTaxRate.value; if (!priceIncludesTax) { - for (const item of line.items) { - item.pendingAdjustments = item.pendingAdjustments.concat( - applicableTaxRate.apply(item.unitPriceWithPromotions), - ); - } + item.pendingAdjustments = item.pendingAdjustments.concat( + applicableTaxRate.apply(item.unitPriceWithPromotions), + ); } - this.calculateOrderTotals(order); } } /** - * Applies any eligible promotions to each OrderItem in the order. + * Returns a memoized function for performing an efficient + * lookup of the correct TaxRate for a given TaxCategory. + */ + private createTaxRateGetter(activeZone: Zone): (taxCategory: TaxCategory) => TaxRate { + const taxRateCache = new Map(); + + return (taxCategory: TaxCategory): TaxRate => { + const cached = taxRateCache.get(taxCategory); + if (cached) { + return cached; + } + const rate = this.taxRateService.getApplicableTaxRate(activeZone, taxCategory); + taxRateCache.set(taxCategory, rate); + return rate; + }; + } + + /** + * Applies any eligible promotions to each OrderItem in the order. Returns an array of + * any OrderItems which had their Adjustments modified. */ - private async applyPromotions(order: Order, promotions: Promotion[]) { + private async applyPromotions(order: Order, promotions: Promotion[]): Promise { const utils = this.createPromotionUtils(); - await this.applyOrderItemPromotions(order, promotions, utils); + const updatedItems = await this.applyOrderItemPromotions(order, promotions, utils); await this.applyOrderPromotions(order, promotions, utils); + return updatedItems; } private async applyOrderItemPromotions(order: Order, promotions: Promotion[], utils: PromotionUtils) { + const updatedOrderItems = new Set(); + for (const line of order.lines) { // Must be re-calculated for each line, since the previous lines may have triggered promotions // which affected the order price. const applicablePromotions = await filterAsync(promotions, p => p.test(order, utils)); - line.clearAdjustments(AdjustmentType.PROMOTION); - for (const promotion of applicablePromotions) { + let priceAdjusted = false; if (await promotion.test(order, utils)) { for (const item of line.items) { + const itemHasPromotions = + item.pendingAdjustments && + !!item.pendingAdjustments.find(a => a.type === AdjustmentType.PROMOTION); + if (itemHasPromotions) { + item.clearAdjustments(AdjustmentType.PROMOTION); + } if (applicablePromotions) { const adjustment = await promotion.apply({ orderItem: item, @@ -111,13 +180,21 @@ export class OrderCalculator { }); if (adjustment) { item.pendingAdjustments = item.pendingAdjustments.concat(adjustment); + priceAdjusted = true; + updatedOrderItems.add(item); + } else if (itemHasPromotions) { + updatedOrderItems.add(item); } } } + if (priceAdjusted) { + this.calculateOrderTotals(order); + priceAdjusted = false; + } } - this.calculateOrderTotals(order); } } + return Array.from(updatedOrderItems.values()); } private async applyOrderPromotions(order: Order, promotions: Promotion[], utils: PromotionUtils) { diff --git a/packages/core/src/service/services/order.service.ts b/packages/core/src/service/services/order.service.ts index f56c6f8d8a..e91e738697 100644 --- a/packages/core/src/service/services/order.service.ts +++ b/packages/core/src/service/services/order.service.ts @@ -99,6 +99,7 @@ export class OrderService { .leftJoinAndSelect('lines.productVariant', 'productVariant') .leftJoinAndSelect('productVariant.taxCategory', 'prodVariantTaxCategory') .leftJoinAndSelect('productVariant.productVariantPrices', 'prices') + .leftJoinAndSelect('productVariant.translations', 'translations') .leftJoinAndSelect('lines.featuredAsset', 'featuredAsset') .leftJoinAndSelect('lines.items', 'items') .leftJoinAndSelect('items.fulfillment', 'fulfillment') @@ -283,7 +284,7 @@ export class OrderService { orderLine.customFields = customFields; } await this.connection.getRepository(OrderLine).save(orderLine, { reload: false }); - return this.applyPriceAdjustments(ctx, order); + return this.applyPriceAdjustments(ctx, order, orderLine); } async removeItemFromOrder(ctx: RequestContext, orderId: ID, orderLineId: ID): Promise { @@ -788,15 +789,23 @@ export class OrderService { /** * Applies promotions, taxes and shipping to the Order. */ - private async applyPriceAdjustments(ctx: RequestContext, order: Order): Promise { + private async applyPriceAdjustments( + ctx: RequestContext, + order: Order, + updatedOrderLine?: OrderLine, + ): Promise { const promotions = await this.connection.getRepository(Promotion).find({ where: { enabled: true, deletedAt: null }, order: { priorityScore: 'ASC' }, }); - order = await this.orderCalculator.applyPriceAdjustments(ctx, order, promotions); + const updatedItems = await this.orderCalculator.applyPriceAdjustments( + ctx, + order, + promotions, + updatedOrderLine, + ); await this.connection.getRepository(Order).save(order, { reload: false }); - await this.connection.getRepository(OrderItem).save(order.getOrderItems(), { reload: false }); - await this.connection.getRepository(OrderLine).save(order.lines, { reload: false }); + await this.connection.getRepository(OrderItem).save(updatedItems, { reload: false }); return order; }