Skip to content

Commit

Permalink
perf(core): Optimize order operations
Browse files Browse the repository at this point in the history
This commit introduces a number of small optimizations to order-related code paths:

- Relations decorator added to `addItemToOrder` and `adjustOrderLine`
- Relations array passed down through key OrderService methods
- Ids used for tax calculations, so entire entities do not need to be joined
  • Loading branch information
michaelbromley committed Sep 9, 2024
1 parent d7bd446 commit e3d6c21
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 26 deletions.
17 changes: 13 additions & 4 deletions packages/core/src/api/resolvers/shop/shop-order.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export class ShopOrderResolver {
async order(
@Ctx() ctx: RequestContext,
@Args() args: QueryOrderArgs,
@Relations(Order) relations: RelationPaths<Order>,
@Relations({ entity: Order, omit: ['aggregateOrder', 'sellerOrders'] })
relations: RelationPaths<Order>,
): Promise<Order | undefined> {
const requiredRelations: RelationPaths<Order> = ['customer', 'customer.user'];
const order = await this.orderService.findOne(
Expand All @@ -98,7 +99,8 @@ export class ShopOrderResolver {
@Allow(Permission.Owner)
async activeOrder(
@Ctx() ctx: RequestContext,
@Relations(Order) relations: RelationPaths<Order>,
@Relations({ entity: Order, omit: ['aggregateOrder', 'sellerOrders'] })
relations: RelationPaths<Order>,
@Args() args: ActiveOrderArgs,
): Promise<Order | undefined> {
if (ctx.authorizedAsOwnerOnly) {
Expand All @@ -107,7 +109,7 @@ export class ShopOrderResolver {
args[ACTIVE_ORDER_INPUT_FIELD_NAME],
);
if (sessionOrder) {
return this.orderService.findOne(ctx, sessionOrder.id);
return this.orderService.findOne(ctx, sessionOrder.id, relations);
} else {
return;
}
Expand All @@ -119,7 +121,8 @@ export class ShopOrderResolver {
async orderByCode(
@Ctx() ctx: RequestContext,
@Args() args: QueryOrderByCodeArgs,
@Relations(Order) relations: RelationPaths<Order>,
@Relations({ entity: Order, omit: ['aggregateOrder', 'sellerOrders'] })
relations: RelationPaths<Order>,
): Promise<Order | undefined> {
if (ctx.authorizedAsOwnerOnly) {
const requiredRelations: RelationPaths<Order> = ['customer', 'customer.user'];
Expand Down Expand Up @@ -294,6 +297,8 @@ export class ShopOrderResolver {
async addItemToOrder(
@Ctx() ctx: RequestContext,
@Args() args: MutationAddItemToOrderArgs & ActiveOrderArgs,
@Relations({ entity: Order, omit: ['aggregateOrder', 'sellerOrders'] })
relations: RelationPaths<Order>,
): Promise<ErrorResultUnion<UpdateOrderItemsResult, Order>> {
const order = await this.activeOrderService.getActiveOrder(
ctx,
Expand All @@ -306,6 +311,7 @@ export class ShopOrderResolver {
args.productVariantId,
args.quantity,
(args as any).customFields,
relations,
);
}

Expand All @@ -315,6 +321,8 @@ export class ShopOrderResolver {
async adjustOrderLine(
@Ctx() ctx: RequestContext,
@Args() args: MutationAdjustOrderLineArgs & ActiveOrderArgs,
@Relations({ entity: Order, omit: ['aggregateOrder', 'sellerOrders'] })
relations: RelationPaths<Order>,
): Promise<ErrorResultUnion<UpdateOrderItemsResult, Order>> {
if (args.quantity === 0) {
return this.removeOrderLine(ctx, { orderLineId: args.orderLineId });
Expand All @@ -330,6 +338,7 @@ export class ShopOrderResolver {
args.orderLineId,
args.quantity,
(args as any).customFields,
relations,
);
}

Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/entity/order-line/order-line.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ export class OrderLine extends VendureEntity implements HasCustomFields {
@ManyToOne(type => TaxCategory)
taxCategory: TaxCategory;

@EntityId({ nullable: true })
taxCategoryId: ID;

@Index()
@ManyToOne(type => Asset, asset => asset.featuredInVariants, { onDelete: 'SET NULL' })
featuredAsset: Asset;
Expand Down
19 changes: 16 additions & 3 deletions packages/core/src/entity/tax-rate/tax-rate.entity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TaxLine } from '@vendure/common/lib/generated-types';
import { DeepPartial } from '@vendure/common/lib/shared-types';
import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
import { Column, Entity, Index, ManyToOne } from 'typeorm';

import { grossPriceOf, netPriceOf, taxComponentOf, taxPayableOn } from '../../common/tax-utils';
Expand All @@ -8,6 +8,7 @@ import { HasCustomFields } from '../../config/custom-field/custom-field-types';
import { VendureEntity } from '../base/base.entity';
import { CustomTaxRateFields } from '../custom-entity-fields';
import { CustomerGroup } from '../customer-group/customer-group.entity';
import { EntityId } from '../entity-id.decorator';
import { TaxCategory } from '../tax-category/tax-category.entity';
import { DecimalTransformer } from '../value-transformers';
import { Zone } from '../zone/zone.entity';
Expand Down Expand Up @@ -38,10 +39,16 @@ export class TaxRate extends VendureEntity implements HasCustomFields {
@ManyToOne(type => TaxCategory, taxCategory => taxCategory.taxRates)
category: TaxCategory;

@EntityId({ nullable: true })
categoryId: ID;

@Index()
@ManyToOne(type => Zone, zone => zone.taxRates)
zone: Zone;

@EntityId({ nullable: true })
zoneId: ID;

@Index()
@ManyToOne(type => CustomerGroup, customerGroup => customerGroup.taxRates, { nullable: true })
customerGroup?: CustomerGroup;
Expand Down Expand Up @@ -84,7 +91,13 @@ export class TaxRate extends VendureEntity implements HasCustomFields {
};
}

test(zone: Zone, taxCategory: TaxCategory): boolean {
return idsAreEqual(taxCategory.id, this.category.id) && idsAreEqual(zone.id, this.zone.id);
test(zone: Zone | ID, taxCategory: TaxCategory | ID): boolean {
const taxCategoryId = this.isId(taxCategory) ? taxCategory : taxCategory.id;
const zoneId = this.isId(zone) ? zone : zone.id;
return idsAreEqual(taxCategoryId, this.categoryId) && idsAreEqual(zoneId, this.zoneId);
}

private isId<T>(entityOrId: T | ID): entityOrId is ID {
return typeof entityOrId === 'string' || typeof entityOrId === 'number';
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injectable } from '@nestjs/common';
import { filterAsync } from '@vendure/common/lib/filter-async';
import { AdjustmentType } from '@vendure/common/lib/generated-types';
import { ID } from '@vendure/common/lib/shared-types';

import { RequestContext } from '../../../api/common/request-context';
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
Expand Down Expand Up @@ -76,7 +77,6 @@ export class OrderCalculator {
ctx,
order,
updatedOrderLine,
activeTaxZone,
this.createTaxRateGetter(ctx, activeTaxZone),
);
}
Expand Down Expand Up @@ -113,7 +113,7 @@ export class OrderCalculator {
private async applyTaxes(ctx: RequestContext, order: Order, activeZone: Zone) {
const getTaxRate = this.createTaxRateGetter(ctx, activeZone);
for (const line of order.lines) {
await this.applyTaxesToOrderLine(ctx, order, line, activeZone, getTaxRate);
await this.applyTaxesToOrderLine(ctx, order, line, getTaxRate);
}
this.calculateOrderTotals(order);
}
Expand All @@ -126,10 +126,9 @@ export class OrderCalculator {
ctx: RequestContext,
order: Order,
line: OrderLine,
activeZone: Zone,
getTaxRate: (taxCategory: TaxCategory) => Promise<TaxRate>,
getTaxRate: (taxCategoryId: ID) => Promise<TaxRate>,
) {
const applicableTaxRate = await getTaxRate(line.taxCategory);
const applicableTaxRate = await getTaxRate(line.taxCategoryId);
const { taxLineCalculationStrategy } = this.configService.taxOptions;
line.taxLines = await taxLineCalculationStrategy.calculate({
ctx,
Expand All @@ -147,16 +146,16 @@ export class OrderCalculator {
private createTaxRateGetter(
ctx: RequestContext,
activeZone: Zone,
): (taxCategory: TaxCategory) => Promise<TaxRate> {
const taxRateCache = new Map<TaxCategory, TaxRate>();
): (taxCategoryId: ID) => Promise<TaxRate> {
const taxRateCache = new Map<ID, TaxRate>();

return async (taxCategory: TaxCategory): Promise<TaxRate> => {
const cached = taxRateCache.get(taxCategory);
return async (taxCategoryId: ID): Promise<TaxRate> => {
const cached = taxRateCache.get(taxCategoryId);
if (cached) {
return cached;
}
const rate = await this.taxRateService.getApplicableTaxRate(ctx, activeZone, taxCategory);
taxRateCache.set(taxCategory, rate);
const rate = await this.taxRateService.getApplicableTaxRate(ctx, activeZone, taxCategoryId);
taxRateCache.set(taxCategoryId, rate);
return rate;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class OrderModifier {
): Promise<OrderLine | undefined> {
for (const line of order.lines) {
const match =
idsAreEqual(line.productVariant.id, productVariantId) &&
idsAreEqual(line.productVariantId, productVariantId) &&
(await this.customFieldsAreEqual(ctx, line, customFields, line.customFields));
if (match) {
return line;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ export class OrderSplitter {
...pick(line, [
'quantity',
'productVariant',
'productVariantId',
'taxCategory',
'taxCategoryId',
'featuredAsset',
'shippingLine',
'shippingLineId',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export class OrderTestingService {
taxLines: [],
quantity: line.quantity,
taxCategory: productVariant.taxCategory,
taxCategoryId: productVariant.taxCategoryId,
});
mockOrder.lines.push(orderLine);

Expand Down
30 changes: 24 additions & 6 deletions packages/core/src/service/services/order.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ export class OrderService {
* @since 2.2.0
*/
async updateOrderCustomer(ctx: RequestContext, { customerId, orderId, note }: SetOrderCustomerInput) {
const order = await this.getOrderOrThrow(ctx, orderId);
const order = await this.getOrderOrThrow(ctx, orderId, ['channels', 'customer']);
const currentCustomer = order.customer;
if (currentCustomer?.id === customerId) {
// No change in customer, so just return the order as-is
Expand Down Expand Up @@ -539,6 +539,7 @@ export class OrderService {
productVariantId: ID,
quantity: number,
customFields?: { [key: string]: any },
relations?: RelationPaths<Order>,
): Promise<ErrorResultUnion<UpdateOrderItemsResult, Order>> {
const order = await this.getOrderOrThrow(ctx, orderId);
const existingOrderLine = await this.orderModifier.getExistingOrderLine(
Expand Down Expand Up @@ -597,7 +598,7 @@ export class OrderService {
await this.orderModifier.updateOrderLineQuantity(ctx, orderLine, correctedQuantity, order);
}
const quantityWasAdjustedDown = correctedQuantity < quantity;
const updatedOrder = await this.applyPriceAdjustments(ctx, order, [orderLine]);
const updatedOrder = await this.applyPriceAdjustments(ctx, order, [orderLine], relations);
if (quantityWasAdjustedDown) {
return new InsufficientStockError({ quantityAvailable: correctedQuantity, order: updatedOrder });
} else {
Expand All @@ -615,6 +616,7 @@ export class OrderService {
orderLineId: ID,
quantity: number,
customFields?: { [key: string]: any },
relations?: RelationPaths<Order>,
): Promise<ErrorResultUnion<UpdateOrderItemsResult, Order>> {
const order = await this.getOrderOrThrow(ctx, orderId);
const orderLine = this.getOrderLineOrThrow(order, orderLineId);
Expand Down Expand Up @@ -661,7 +663,7 @@ export class OrderService {
await this.orderModifier.updateOrderLineQuantity(ctx, orderLine, correctedQuantity, order);
}
const quantityWasAdjustedDown = correctedQuantity < quantity;
const updatedOrder = await this.applyPriceAdjustments(ctx, order, updatedOrderLines);
const updatedOrder = await this.applyPriceAdjustments(ctx, order, updatedOrderLines, relations);
if (quantityWasAdjustedDown) {
return new InsufficientStockError({ quantityAvailable: correctedQuantity, order: updatedOrder });
} else {
Expand Down Expand Up @@ -1664,8 +1666,23 @@ export class OrderService {
return order;
}

private async getOrderOrThrow(ctx: RequestContext, orderId: ID): Promise<Order> {
const order = await this.findOne(ctx, orderId);
private async getOrderOrThrow(
ctx: RequestContext,
orderId: ID,
relations?: RelationPaths<Order>,
): Promise<Order> {
const order = await this.findOne(
ctx,
orderId,
relations ?? [
'lines',
'lines.productVariant',
'lines.productVariant.productVariantPrices',
'shippingLines',
'surcharges',
'customer',
],
);
if (!order) {
throw new EntityNotFoundError('Order', orderId);
}
Expand Down Expand Up @@ -1731,6 +1748,7 @@ export class OrderService {
ctx: RequestContext,
order: Order,
updatedOrderLines?: OrderLine[],
relations?: RelationPaths<Order>,
): Promise<Order> {
const promotions = await this.promotionService.getActivePromotionsInChannel(ctx);
const activePromotionsPre = await this.promotionService.getActivePromotionsOnOrder(ctx, order.id);
Expand Down Expand Up @@ -1816,7 +1834,7 @@ export class OrderService {
await this.connection.getRepository(ctx, ShippingLine).save(order.shippingLines, { reload: false });
await this.promotionService.runPromotionSideEffects(ctx, order, activePromotionsPre);

return assertFound(this.findOne(ctx, order.id));
return assertFound(this.findOne(ctx, order.id, relations));
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/service/services/tax-rate.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,11 @@ export class TaxRateService {
* Returns the applicable TaxRate based on the specified Zone and TaxCategory. Used when calculating Order
* prices.
*/
async getApplicableTaxRate(ctx: RequestContext, zone: Zone, taxCategory: TaxCategory): Promise<TaxRate> {
async getApplicableTaxRate(
ctx: RequestContext,
zone: Zone | ID,
taxCategory: TaxCategory | ID,
): Promise<TaxRate> {
const rate = (await this.getActiveTaxRates(ctx)).find(r => r.test(zone, taxCategory));
return rate || this.defaultTaxRate;
}
Expand Down

0 comments on commit e3d6c21

Please sign in to comment.