Browse Source

perf(core): Fix perf regression from lookahead on certain fields

Closes #1578
Michael Bromley 3 years ago
parent
commit
9e65753c63

+ 45 - 26
packages/core/src/api/decorators/relations.decorator.ts

@@ -11,11 +11,12 @@ import { VendureEntity } from '../../entity/index';
 
 export type RelationPaths<T extends VendureEntity> = Array<EntityRelationPaths<T>>;
 
-export type FieldsDecoratorConfig =
-    | Type<VendureEntity>
+export type FieldsDecoratorConfig<T extends VendureEntity> =
+    | Type<T>
     | {
-          entity: Type<VendureEntity>;
-          depth: number;
+          entity: Type<T>;
+          depth?: number;
+          omit?: RelationPaths<T>;
       };
 
 const DEFAULT_DEPTH = 3;
@@ -111,32 +112,50 @@ const cache = new TtlCache({ cacheSize: 500, ttl: 5 * 60 * 1000 });
  * \@Relations({ entity: Order, depth: 2 }) relations: RelationPaths<Order>,
  * ```
  *
+ * ## Omit
+ *
+ * The `omit` option is used to explicitly omit certain relations from the calculated relations array. This is useful in certain
+ * cases where we know for sure that we need to run the field resolver _anyway_. A good example is the `Collection.productVariants` relation.
+ * When a GraphQL query comes in for a Collection and also requests its `productVariants` field, there is no point using a lookahead to eagerly
+ * join that relation, because we will throw that data away anyway when the `productVariants` field resolver executes, since it returns a
+ * PaginatedList query rather than a simple array.
+ *
+ * @example
+ * ```TypeScript
+ * \@Relations({ entity: Collection, omit: ['productVariant'] }) relations: RelationPaths<Collection>,
+ * ```
+ *
  * @docsCategory request
  * @docsPage Api Decorator
  * @since 1.6.0
  */
-export const Relations = createParamDecorator<FieldsDecoratorConfig>((data, ctx: ExecutionContext) => {
-    const info = ctx.getArgByIndex(3);
-    if (data == null) {
-        throw new InternalServerError(`The @Relations() decorator requires an entity type argument`);
-    }
-    if (!isGraphQLResolveInfo(info)) {
-        return [];
-    }
-    const cacheKey = info.fieldName + '__' + ctx.getArgByIndex(2).req.body.query;
-    const cachedResult = cache.get(cacheKey);
-    if (cachedResult) {
-        return cachedResult;
-    }
-    const fields = graphqlFields(info);
-    const targetFields = isPaginatedListQuery(info) ? fields.items ?? {} : fields;
-    const entity = typeof data === 'function' ? data : data.entity;
-    const maxDepth = typeof data === 'function' ? DEFAULT_DEPTH : data.depth;
-    const relationFields = getRelationPaths(targetFields, entity, maxDepth);
-    const result = unique(relationFields);
-    cache.set(cacheKey, result);
-    return result;
-});
+export const Relations: <T extends VendureEntity>(data: FieldsDecoratorConfig<T>) => ParameterDecorator =
+    createParamDecorator<FieldsDecoratorConfig<any>>((data, ctx: ExecutionContext) => {
+        const info = ctx.getArgByIndex(3);
+        if (data == null) {
+            throw new InternalServerError(`The @Relations() decorator requires an entity type argument`);
+        }
+        if (!isGraphQLResolveInfo(info)) {
+            return [];
+        }
+        const cacheKey = info.fieldName + '__' + ctx.getArgByIndex(2).req.body.query;
+        const cachedResult = cache.get(cacheKey);
+        if (cachedResult) {
+            return cachedResult;
+        }
+        const fields = graphqlFields(info);
+        const targetFields = isPaginatedListQuery(info) ? fields.items ?? {} : fields;
+        const entity = typeof data === 'function' ? data : data.entity;
+        const maxDepth = typeof data === 'function' ? DEFAULT_DEPTH : data.depth ?? DEFAULT_DEPTH;
+        const omit = typeof data === 'function' ? [] : data.omit ?? [];
+        const relationFields = getRelationPaths(targetFields, entity, maxDepth);
+        let result = unique(relationFields);
+        for (const omitPath of omit) {
+            result = result.filter(resultPath => !resultPath.startsWith(omitPath as string));
+        }
+        cache.set(cacheKey, result);
+        return result;
+    });
 
 function getRelationPaths(
     fields: Record<string, Record<string, any>>,

+ 5 - 7
packages/core/src/api/resolvers/admin/collection.resolver.ts

@@ -48,7 +48,8 @@ export class CollectionResolver {
     async collections(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCollectionsArgs,
-        @Relations(Collection) relations: RelationPaths<Collection>,
+        @Relations({ entity: Collection, omit: ['productVariants', 'assets'] })
+        relations: RelationPaths<Collection>,
     ): Promise<PaginatedList<Translated<Collection>>> {
         return this.collectionService.findAll(ctx, args.options || undefined, relations).then(res => {
             res.items.forEach(this.encodeFilters);
@@ -61,7 +62,8 @@ export class CollectionResolver {
     async collection(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCollectionArgs,
-        @Relations(Collection) relations: RelationPaths<Collection>,
+        @Relations({ entity: Collection, omit: ['productVariants', 'assets'] })
+        relations: RelationPaths<Collection>,
     ): Promise<Translated<Collection> | undefined> {
         let collection: Translated<Collection> | undefined;
         if (args.id) {
@@ -80,11 +82,7 @@ export class CollectionResolver {
 
     @Query()
     @Allow(Permission.ReadCatalog, Permission.ReadCollection)
-    previewCollectionVariants(
-        @Ctx() ctx: RequestContext,
-        @Args() args: QueryPreviewCollectionVariantsArgs,
-        @Relations(Collection) relations: RelationPaths<Collection>,
-    ) {
+    previewCollectionVariants(@Ctx() ctx: RequestContext, @Args() args: QueryPreviewCollectionVariantsArgs) {
         this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, args.input.filters);
         return this.collectionService.previewCollectionVariants(ctx, args.input, args.options || undefined);
     }

+ 2 - 2
packages/core/src/api/resolvers/admin/customer.resolver.ts

@@ -39,7 +39,7 @@ export class CustomerResolver {
     async customers(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCustomersArgs,
-        @Relations(Customer) relations: RelationPaths<Customer>,
+        @Relations({ entity: Customer, omit: ['orders'] }) relations: RelationPaths<Customer>,
     ): Promise<PaginatedList<Customer>> {
         return this.customerService.findAll(ctx, args.options || undefined, relations);
     }
@@ -49,7 +49,7 @@ export class CustomerResolver {
     async customer(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCustomerArgs,
-        @Relations(Customer) relations: RelationPaths<Customer>,
+        @Relations({ entity: Customer, omit: ['orders'] }) relations: RelationPaths<Customer>,
     ): Promise<Customer | undefined> {
         return this.customerService.findOne(ctx, args.id, relations);
     }

+ 3 - 3
packages/core/src/api/resolvers/admin/product.resolver.ts

@@ -49,7 +49,7 @@ export class ProductResolver {
     async products(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryProductsArgs,
-        @Relations(Product) relations: RelationPaths<Product>,
+        @Relations({ entity: Product, omit: ['variants', 'assets'] }) relations: RelationPaths<Product>,
     ): Promise<PaginatedList<Translated<Product>>> {
         return this.productService.findAll(ctx, args.options || undefined, relations);
     }
@@ -59,7 +59,7 @@ export class ProductResolver {
     async product(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryProductArgs,
-        @Relations(Product) relations: RelationPaths<Product>,
+        @Relations({ entity: Product, omit: ['variants', 'assets'] }) relations: RelationPaths<Product>,
     ): Promise<Translated<Product> | undefined> {
         if (args.id) {
             const product = await this.productService.findOne(ctx, args.id, relations);
@@ -79,7 +79,7 @@ export class ProductResolver {
     async productVariants(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryProductVariantsArgs,
-        @Relations(ProductVariant) relations: RelationPaths<ProductVariant>,
+        @Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths<ProductVariant>,
     ): Promise<PaginatedList<Translated<ProductVariant>>> {
         if (args.productId) {
             return this.productVariantService.getVariantsByProductId(

+ 1 - 1
packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

@@ -45,7 +45,7 @@ export class CollectionEntityResolver {
         @Parent() collection: Collection,
         @Args() args: { options: ProductVariantListOptions },
         @Api() apiType: ApiType,
-        @Relations(ProductVariant) relations: RelationPaths<ProductVariant>,
+        @Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths<ProductVariant>,
     ): Promise<PaginatedList<Translated<ProductVariant>>> {
         let options: ListQueryOptions<Product> = args.options;
         if (apiType === 'shop') {

+ 2 - 2
packages/core/src/api/resolvers/entity/product-entity.resolver.ts

@@ -54,7 +54,7 @@ export class ProductEntityResolver {
     async variants(
         @Ctx() ctx: RequestContext,
         @Parent() product: Product,
-        @Relations(ProductVariant) relations: RelationPaths<ProductVariant>,
+        @Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths<ProductVariant>,
     ): Promise<Array<Translated<ProductVariant>>> {
         const { items: variants } = await this.productVariantService.getVariantsByProductId(
             ctx,
@@ -70,7 +70,7 @@ export class ProductEntityResolver {
         @Ctx() ctx: RequestContext,
         @Parent() product: Product,
         @Args() args: { options: ProductVariantListOptions },
-        @Relations(ProductVariant) relations: RelationPaths<ProductVariant>,
+        @Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths<ProductVariant>,
     ): Promise<PaginatedList<ProductVariant>> {
         return this.productVariantService.getVariantsByProductId(ctx, product.id, args.options, relations);
     }

+ 1 - 1
packages/core/src/api/resolvers/entity/product-option-group-entity.resolver.ts

@@ -29,7 +29,7 @@ export class ProductOptionGroupEntityResolver {
         @Parent() optionGroup: Translated<ProductOptionGroup>,
     ): Promise<Array<Translated<ProductOption>>> {
         if (optionGroup.options) {
-            return Promise.resolve(optionGroup.options);
+            return optionGroup.options;
         }
         const group = await this.productOptionGroupService.findOne(ctx, optionGroup.id);
         return group ? group.options : [];

+ 6 - 4
packages/core/src/api/resolvers/shop/shop-products.resolver.ts

@@ -39,7 +39,7 @@ export class ShopProductsResolver {
     async products(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryProductsArgs,
-        @Relations(Product) relations: RelationPaths<Product>,
+        @Relations({ entity: Product, omit: ['variants', 'assets'] }) relations: RelationPaths<Product>,
     ): Promise<PaginatedList<Translated<Product>>> {
         const options: ListQueryOptions<Product> = {
             ...args.options,
@@ -55,7 +55,7 @@ export class ShopProductsResolver {
     async product(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryProductArgs,
-        @Relations(Product) relations: RelationPaths<Product>,
+        @Relations({ entity: Product, omit: ['variants', 'assets'] }) relations: RelationPaths<Product>,
     ): Promise<Translated<Product> | undefined> {
         let result: Translated<Product> | undefined;
         if (args.id) {
@@ -79,7 +79,8 @@ export class ShopProductsResolver {
     async collections(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCollectionsArgs,
-        @Relations(Collection) relations: RelationPaths<Collection>,
+        @Relations({ entity: Collection, omit: ['productVariants', 'assets'] })
+        relations: RelationPaths<Collection>,
     ): Promise<PaginatedList<Translated<Collection>>> {
         const options: ListQueryOptions<Collection> = {
             ...args.options,
@@ -95,7 +96,8 @@ export class ShopProductsResolver {
     async collection(
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCollectionArgs,
-        @Relations(Collection) relations: RelationPaths<Collection>,
+        @Relations({ entity: Collection, omit: ['productVariants', 'assets'] })
+        relations: RelationPaths<Collection>,
     ): Promise<Translated<Collection> | undefined> {
         let collection: Translated<Collection> | undefined;
         if (args.id) {