feat(core): Add `shouldRunCheck` function to ShippingEligibilityChecker

Relates to #536. Allows one to optimize calls to the `check()` function in order to prevent
unnecessary expensive calls.

packages/core/e2e/order-promotion.e2e-spec.ts

@@ -86,7 +86,7 @@ describe('Promotions applied to Orders', () => {
         input => !!input.lines,
     );
 
-    let products: GetPromoProducts.Items[];
+    let products: GetProductsWithVariantPrices.Items[];
 
     beforeAll(async () => {
         await server.init({

packages/core/e2e/shipping-method-eligibility.e2e-spec.ts

@@ -19,6 +19,7 @@ import {
     GetShippingMethods,
     RemoveItemFromOrder,
     SetCustomerForOrder,
+    SetShippingAddress,
     SetShippingMethod,
     TestOrderFragmentFragment,
     UpdatedOrderFragment,
@@ -31,6 +32,7 @@ import {
     GET_ELIGIBLE_SHIPPING_METHODS,
     REMOVE_ITEM_FROM_ORDER,
     SET_CUSTOMER,
+    SET_SHIPPING_ADDRESS,
     SET_SHIPPING_METHOD,
 } from './graphql/shop-definitions';
 
@@ -56,6 +58,20 @@ const checker2 = new ShippingEligibilityChecker({
     },
 });
 
+const check3Spy = jest.fn();
+const checker3 = new ShippingEligibilityChecker({
+    code: 'checker3',
+    description: [],
+    args: {},
+    check: (ctx, order) => {
+        check3Spy();
+        return order.lines.length === 3;
+    },
+    shouldRunCheck: (ctx, order) => {
+        return order.shippingAddress;
+    },
+});
+
 const calculator = new ShippingCalculator({
     code: 'calculator',
     description: [],
@@ -72,7 +88,7 @@ describe('ShippingMethod resolver', () => {
     const { server, adminClient, shopClient } = createTestEnvironment({
         ...testConfig,
         shippingOptions: {
-            shippingEligibilityCheckers: [defaultShippingEligibilityChecker, checker1, checker2],
+            shippingEligibilityCheckers: [defaultShippingEligibilityChecker, checker1, checker2, checker3],
             shippingCalculators: [defaultShippingCalculator, calculator],
         },
     });
@@ -81,9 +97,9 @@ describe('ShippingMethod resolver', () => {
         UpdatedOrderFragment | TestOrderFragmentFragment
     > = createErrorResultGuard<UpdatedOrderFragment>(input => !!input.lines);
 
-    let order: UpdatedOrderFragment;
     let singleLineShippingMethod: ShippingMethodFragment;
     let multiLineShippingMethod: ShippingMethodFragment;
+    let optimizedShippingMethod: ShippingMethodFragment;
 
     beforeAll(async () => {
         await server.init({
@@ -133,160 +149,296 @@ describe('ShippingMethod resolver', () => {
             },
         });
         multiLineShippingMethod = result2.createShippingMethod;
+
+        const result3 = await adminClient.query<
+            CreateShippingMethod.Mutation,
+            CreateShippingMethod.Variables
+        >(CREATE_SHIPPING_METHOD, {
+            input: {
+                code: 'optimized',
+                description: 'Optimized with shouldRunCheck',
+                checker: {
+                    code: checker3.code,
+                    arguments: [],
+                },
+                calculator: {
+                    code: calculator.code,
+                    arguments: [],
+                },
+            },
+        });
+        optimizedShippingMethod = result3.createShippingMethod;
     }, TEST_SETUP_TIMEOUT_MS);
 
     afterAll(async () => {
         await server.destroy();
     });
 
-    it('Does not run checkers before a ShippingMethod is assigned to Order', async () => {
-        check1Spy.mockClear();
-        check2Spy.mockClear();
+    describe('default behavior', () => {
+        let order: UpdatedOrderFragment;
 
-        await shopClient.asUserWithCredentials('hayden.zieme12@hotmail.com', 'test');
+        it('Does not run checkers before a ShippingMethod is assigned to Order', async () => {
+            check1Spy.mockClear();
+            check2Spy.mockClear();
 
-        const { addItemToOrder } = await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(
-            ADD_ITEM_TO_ORDER,
-            {
+            await shopClient.asUserWithCredentials('hayden.zieme12@hotmail.com', 'test');
+
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
                 quantity: 1,
                 productVariantId: 'T_1',
-            },
-        );
-        orderGuard.assertSuccess(addItemToOrder);
+            });
+            orderGuard.assertSuccess(addItemToOrder);
 
-        expect(check1Spy).not.toHaveBeenCalled();
-        expect(check2Spy).not.toHaveBeenCalled();
+            expect(check1Spy).not.toHaveBeenCalled();
+            expect(check2Spy).not.toHaveBeenCalled();
 
-        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
-            ADJUST_ITEM_QUANTITY,
-            {
-                quantity: 2,
-                orderLineId: addItemToOrder.lines[0].id,
-            },
-        );
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 2,
+                    orderLineId: addItemToOrder.lines[0].id,
+                },
+            );
 
-        expect(check1Spy).not.toHaveBeenCalled();
-        expect(check2Spy).not.toHaveBeenCalled();
+            expect(check1Spy).not.toHaveBeenCalled();
+            expect(check2Spy).not.toHaveBeenCalled();
 
-        order = addItemToOrder;
-    });
+            order = addItemToOrder;
+        });
 
-    it('Runs checkers when querying for eligible ShippingMethods', async () => {
-        check1Spy.mockClear();
-        check2Spy.mockClear();
+        it('Runs checkers when querying for eligible ShippingMethods', async () => {
+            check1Spy.mockClear();
+            check2Spy.mockClear();
 
-        const { eligibleShippingMethods } = await shopClient.query<GetShippingMethods.Query>(
-            GET_ELIGIBLE_SHIPPING_METHODS,
-        );
+            const { eligibleShippingMethods } = await shopClient.query<GetShippingMethods.Query>(
+                GET_ELIGIBLE_SHIPPING_METHODS,
+            );
 
-        expect(check1Spy).toHaveBeenCalledTimes(1);
-        expect(check2Spy).toHaveBeenCalledTimes(1);
-    });
+            expect(check1Spy).toHaveBeenCalledTimes(1);
+            expect(check2Spy).toHaveBeenCalledTimes(1);
+        });
 
-    it('Runs checker of assigned method only', async () => {
-        check1Spy.mockClear();
-        check2Spy.mockClear();
+        it('Runs checker of assigned method only', async () => {
+            check1Spy.mockClear();
+            check2Spy.mockClear();
 
-        await shopClient.query<SetShippingMethod.Mutation, SetShippingMethod.Variables>(SET_SHIPPING_METHOD, {
-            id: singleLineShippingMethod.id,
-        });
+            await shopClient.query<SetShippingMethod.Mutation, SetShippingMethod.Variables>(
+                SET_SHIPPING_METHOD,
+                {
+                    id: singleLineShippingMethod.id,
+                },
+            );
+
+            // A check is done when assigning the method to ensure it
+            // is eligible, and again when calculating order adjustments
+            expect(check1Spy).toHaveBeenCalledTimes(2);
+            expect(check2Spy).not.toHaveBeenCalled();
+
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 3,
+                    orderLineId: order.lines[0].id,
+                },
+            );
 
-        // A check is done when assigning the method to ensure it
-        // is eligible, and again when calculating order adjustments
-        expect(check1Spy).toHaveBeenCalledTimes(2);
-        expect(check2Spy).not.toHaveBeenCalled();
+            expect(check1Spy).toHaveBeenCalledTimes(3);
+            expect(check2Spy).not.toHaveBeenCalled();
 
-        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
-            ADJUST_ITEM_QUANTITY,
-            {
-                quantity: 3,
-                orderLineId: order.lines[0].id,
-            },
-        );
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 4,
+                    orderLineId: order.lines[0].id,
+                },
+            );
 
-        expect(check1Spy).toHaveBeenCalledTimes(3);
-        expect(check2Spy).not.toHaveBeenCalled();
+            expect(check1Spy).toHaveBeenCalledTimes(4);
+            expect(check2Spy).not.toHaveBeenCalled();
+        });
 
-        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
-            ADJUST_ITEM_QUANTITY,
-            {
-                quantity: 4,
-                orderLineId: order.lines[0].id,
-            },
-        );
+        it('Prevents ineligible method from being assigned', async () => {
+            const { setOrderShippingMethod } = await shopClient.query<
+                SetShippingMethod.Mutation,
+                SetShippingMethod.Variables
+            >(SET_SHIPPING_METHOD, {
+                id: multiLineShippingMethod.id,
+            });
 
-        expect(check1Spy).toHaveBeenCalledTimes(4);
-        expect(check2Spy).not.toHaveBeenCalled();
-    });
+            orderGuard.assertErrorResult(setOrderShippingMethod);
 
-    it('Prevents ineligible method from being assigned', async () => {
-        const { setOrderShippingMethod } = await shopClient.query<
-            SetShippingMethod.Mutation,
-            SetShippingMethod.Variables
-        >(SET_SHIPPING_METHOD, {
-            id: multiLineShippingMethod.id,
+            expect(setOrderShippingMethod.errorCode).toBe(ErrorCode.INELIGIBLE_SHIPPING_METHOD_ERROR);
+            expect(setOrderShippingMethod.message).toBe(
+                'This Order is not eligible for the selected ShippingMethod',
+            );
         });
 
-        orderGuard.assertErrorResult(setOrderShippingMethod);
+        it('Runs checks when assigned method becomes ineligible', async () => {
+            check1Spy.mockClear();
+            check2Spy.mockClear();
 
-        expect(setOrderShippingMethod.errorCode).toBe(ErrorCode.INELIGIBLE_SHIPPING_METHOD_ERROR);
-        expect(setOrderShippingMethod.message).toBe(
-            'This Order is not eligible for the selected ShippingMethod',
-        );
+            // Adding a second OrderLine will make the singleLineShippingMethod
+            // ineligible
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
+                quantity: 1,
+                productVariantId: 'T_2',
+            });
+            orderGuard.assertSuccess(addItemToOrder);
+
+            // Checked once to see if still eligible (no)
+            expect(check1Spy).toHaveBeenCalledTimes(1);
+            // Checked once when looking for a fallback
+            expect(check2Spy).toHaveBeenCalledTimes(1);
+
+            const { activeOrder } = await shopClient.query<GetActiveOrder.Query>(GET_ACTIVE_ORDER);
+            // multiLineShippingMethod assigned as a fallback
+            expect(activeOrder?.shippingMethod?.id).toBe(multiLineShippingMethod.id);
+
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 2,
+                    orderLineId: addItemToOrder.lines[1].id,
+                },
+            );
+
+            // No longer called as singleLineShippingMethod not assigned
+            expect(check1Spy).toHaveBeenCalledTimes(1);
+            // Called on changes since multiLineShippingMethod is assigned
+            expect(check2Spy).toHaveBeenCalledTimes(2);
+
+            // Remove the second OrderLine and make multiLineShippingMethod ineligible
+            const { removeOrderLine } = await shopClient.query<
+                RemoveItemFromOrder.Mutation,
+                RemoveItemFromOrder.Variables
+            >(REMOVE_ITEM_FROM_ORDER, {
+                orderLineId: addItemToOrder.lines[1].id,
+            });
+            orderGuard.assertSuccess(removeOrderLine);
+
+            // Called when looking for a fallback
+            expect(check1Spy).toHaveBeenCalledTimes(2);
+            // Called when checking if still eligibile (no)
+            expect(check2Spy).toHaveBeenCalledTimes(3);
+
+            // Falls back to the first eligible shipping method
+            expect(removeOrderLine.shippingMethod?.id).toBe(singleLineShippingMethod.id);
+        });
     });
 
-    it('Runs checks when assigned method becomes ineligible', async () => {
-        check1Spy.mockClear();
-        check2Spy.mockClear();
+    describe('optimization via shouldRunCheck function', () => {
+        let order: UpdatedOrderFragment;
 
-        // Adding a second OrderLine will make the singleLineShippingMethod
-        // ineligible
-        const { addItemToOrder } = await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(
-            ADD_ITEM_TO_ORDER,
-            {
+        beforeAll(async () => {
+            await shopClient.asAnonymousUser();
+            await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(ADD_ITEM_TO_ORDER, {
+                quantity: 1,
+                productVariantId: 'T_1',
+            });
+            await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(ADD_ITEM_TO_ORDER, {
                 quantity: 1,
                 productVariantId: 'T_2',
-            },
-        );
-        orderGuard.assertSuccess(addItemToOrder);
-
-        // Checked once to see if still eligible (no)
-        expect(check1Spy).toHaveBeenCalledTimes(1);
-        // Checked once when looking for a fallback
-        expect(check2Spy).toHaveBeenCalledTimes(1);
-
-        const { activeOrder } = await shopClient.query<GetActiveOrder.Query>(GET_ACTIVE_ORDER);
-        // multiLineShippingMethod assigned as a fallback
-        expect(activeOrder?.shippingMethod?.id).toBe(multiLineShippingMethod.id);
-
-        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
-            ADJUST_ITEM_QUANTITY,
-            {
-                quantity: 2,
-                orderLineId: addItemToOrder.lines[1].id,
-            },
-        );
-
-        // No longer called as singleLineShippingMethod not assigned
-        expect(check1Spy).toHaveBeenCalledTimes(1);
-        // Called on changes since multiLineShippingMethod is assigned
-        expect(check2Spy).toHaveBeenCalledTimes(2);
-
-        // Remove the second OrderLine and make multiLineShippingMethod ineligible
-        const { removeOrderLine } = await shopClient.query<
-            RemoveItemFromOrder.Mutation,
-            RemoveItemFromOrder.Variables
-        >(REMOVE_ITEM_FROM_ORDER, {
-            orderLineId: addItemToOrder.lines[1].id,
+            });
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
+                quantity: 1,
+                productVariantId: 'T_3',
+            });
+            orderGuard.assertSuccess(addItemToOrder);
+            order = addItemToOrder;
+
+            await shopClient.query<SetShippingAddress.Mutation, SetShippingAddress.Variables>(
+                SET_SHIPPING_ADDRESS,
+                {
+                    input: {
+                        streetLine1: '42 Test Street',
+                        city: 'Doncaster',
+                        postalCode: 'DN1 4EE',
+                        countryCode: 'GB',
+                    },
+                },
+            );
+        });
+
+        it('runs check on getEligibleShippingMethods', async () => {
+            check3Spy.mockClear();
+            const { eligibleShippingMethods } = await shopClient.query<GetShippingMethods.Query>(
+                GET_ELIGIBLE_SHIPPING_METHODS,
+            );
+
+            expect(check3Spy).toHaveBeenCalledTimes(1);
+        });
+
+        it('does not re-run check on setting shipping method', async () => {
+            check3Spy.mockClear();
+            await shopClient.query<SetShippingMethod.Mutation, SetShippingMethod.Variables>(
+                SET_SHIPPING_METHOD,
+                {
+                    id: optimizedShippingMethod.id,
+                },
+            );
+            expect(check3Spy).toHaveBeenCalledTimes(0);
         });
-        orderGuard.assertSuccess(removeOrderLine);
 
-        // Called when looking for a fallback
-        expect(check1Spy).toHaveBeenCalledTimes(2);
-        // Called when checking if still eligibile (no)
-        expect(check2Spy).toHaveBeenCalledTimes(3);
+        it('does not re-run check when changing cart contents', async () => {
+            check3Spy.mockClear();
+
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 3,
+                    orderLineId: order.lines[0].id,
+                },
+            );
+
+            expect(check3Spy).toHaveBeenCalledTimes(0);
+        });
 
-        // Falls back to the first eligible shipping method
-        expect(removeOrderLine.shippingMethod?.id).toBe(singleLineShippingMethod.id);
+        it('re-runs check when shouldRunCheck fn invalidates last check', async () => {
+            check3Spy.mockClear();
+            // Update the shipping address, causing the `shouldRunCheck` function
+            // to trigger a check
+            await shopClient.query<SetShippingAddress.Mutation, SetShippingAddress.Variables>(
+                SET_SHIPPING_ADDRESS,
+                {
+                    input: {
+                        streetLine1: '43 Test Street', // This line changed
+                        city: 'Doncaster',
+                        postalCode: 'DN1 4EE',
+                        countryCode: 'GB',
+                    },
+                },
+            );
+
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 2,
+                    orderLineId: order.lines[0].id,
+                },
+            );
+
+            expect(check3Spy).toHaveBeenCalledTimes(1);
+
+            // Does not check a second time though, since the shipping address
+            // is now the same as on the last check.
+            await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+                ADJUST_ITEM_QUANTITY,
+                {
+                    quantity: 3,
+                    orderLineId: order.lines[0].id,
+                },
+            );
+
+            expect(check3Spy).toHaveBeenCalledTimes(1);
+        });
     });
 });

packages/core/src/config/shipping-method/shipping-eligibility-checker.ts

@@ -1,4 +1,6 @@
 import { ConfigArg } from '@vendure/common/lib/generated-types';
+import { Json } from '@vendure/common/lib/shared-types';
+import { createHash } from 'crypto';
 
 import { RequestContext } from '../../api/common/request-context';
 import {
@@ -7,6 +9,7 @@ import {
     ConfigurableOperationDef,
     ConfigurableOperationDefOptions,
 } from '../../common/configurable-operation';
+import { TtlCache } from '../../common/ttl-cache';
 import { Order } from '../../entity/order/order.entity';
 
 /**
@@ -19,6 +22,7 @@ import { Order } from '../../entity/order/order.entity';
 export interface ShippingEligibilityCheckerConfig<T extends ConfigArgs>
     extends ConfigurableOperationDefOptions<T> {
     check: CheckShippingEligibilityCheckerFn<T>;
+    shouldRunCheck?: ShouldRunCheckFn<T>;
 }
 /**
  * @description
@@ -46,10 +50,13 @@ export class ShippingEligibilityChecker<T extends ConfigArgs = ConfigArgs> exten
     T
 > {
     private readonly checkFn: CheckShippingEligibilityCheckerFn<T>;
+    private readonly shouldRunCheckFn?: ShouldRunCheckFn<T>;
+    private shouldRunCheckCache = new TtlCache({ cacheSize: 5000, ttl: 1000 * 60 * 60 * 5 });
 
     constructor(config: ShippingEligibilityCheckerConfig<T>) {
         super(config);
         this.checkFn = config.check;
+        this.shouldRunCheckFn = config.shouldRunCheck;
     }
 
     /**
@@ -58,15 +65,43 @@ export class ShippingEligibilityChecker<T extends ConfigArgs = ConfigArgs> exten
      *
      * @internal
      */
-    check(ctx: RequestContext, order: Order, args: ConfigArg[]): boolean | Promise<boolean> {
-        return this.checkFn(ctx, order, this.argsArrayToHash(args));
+    async check(ctx: RequestContext, order: Order, args: ConfigArg[]): Promise<boolean> {
+        const shouldRunCheck = await this.shouldRunCheck(ctx, order, args);
+        return shouldRunCheck ? this.checkFn(ctx, order, this.argsArrayToHash(args)) : true;
+    }
+
+    /**
+     * Determines whether the check function needs to be run, based on the presence and
+     * result of any defined `shouldRunCheckFn`.
+     */
+    private async shouldRunCheck(ctx: RequestContext, order: Order, args: ConfigArg[]): Promise<boolean> {
+        if (typeof this.shouldRunCheckFn === 'function') {
+            const cacheKey = ctx.session?.id;
+            if (cacheKey) {
+                const checkResult = await this.shouldRunCheckFn(ctx, order, this.argsArrayToHash(args));
+                const checkResultHash = createHash('sha1')
+                    .update(JSON.stringify(checkResult))
+                    .digest('base64');
+                const lastResultHash = this.shouldRunCheckCache.get(cacheKey);
+                this.shouldRunCheckCache.set(cacheKey, checkResultHash);
+                if (checkResultHash === lastResultHash) {
+                    return false;
+                }
+            }
+        }
+        return true;
     }
 }
 
 /**
  * @description
  * A function which implements logic to determine whether a given {@link Order} is eligible for
- * a particular shipping method.
+ * a particular shipping method. Once a ShippingMethod has been assigned to an Order, this
+ * function will be called on every change to the Order (e.g. updating quantities, adding/removing
+ * items etc).
+ *
+ * If the code running in this function is expensive, then consider also defining
+ * a {@link ShouldRunCheckFn} to avoid unnecessary calls.
  *
  * @docsCategory shipping
  */
@@ -75,3 +110,37 @@ export type CheckShippingEligibilityCheckerFn<T extends ConfigArgs> = (
     order: Order,
     args: ConfigArgValues<T>,
 ) => boolean | Promise<boolean>;
+
+/**
+ * @description
+ * An optional method which is used to decide whether to run the `check()` function.
+ * Returns a JSON-compatible object which is cached and compared between calls.
+ * If the value is the same, then the `check()` function is not called.
+ *
+ * Use of this function is an optimization technique which can be useful when
+ * the `check()` function is expensive and should be kept to an absolute minimum.
+ *
+ * @example
+ * ```TypeScript
+ * const optimizedChecker = new ShippingEligibilityChecker({
+ *   code: 'example',
+ *   description: [],
+ *   args: {},
+ *   check: async (ctx, order) => {
+ *     // some slow, expensive function here
+ *   },
+ *   shouldRunCheck: (ctx, order) => {
+ *     // Will only run the `check()` function any time
+ *     // the shippingAddress object has changed.
+ *     return order.shippingAddress;
+ *   },
+ * });
+ * ```
+ *
+ * @docsCategory shipping
+ */
+export type ShouldRunCheckFn<T extends ConfigArgs> = (
+    ctx: RequestContext,
+    order: Order,
+    args: ConfigArgValues<T>,
+) => Json | Promise<Json>;