Browse Source

fix(core): Fix regression when querying custom field relations

Fixes #1664. Relates to #1636 because it partially reverts that fix. This is
unfortunate but having the GraphQL API work without throwing errors is more
important than not loading custom fields in the service layer.

To mitigate the effect of the partial reversion of the #1636 fix, I've added
a debug-level log message to alert to the fact that an expected relation
could not be loaded.
Michael Bromley 3 years ago
parent
commit
b279d25e85

+ 55 - 1
packages/core/e2e/custom-field-relations.e2e-spec.ts

@@ -20,14 +20,16 @@ import {
     RequestContext,
     ShippingMethod,
     TransactionalConnection,
+    VendureEntity,
     VendurePlugin,
 } from '@vendure/core';
 import { createTestEnvironment } from '@vendure/testing';
 import gql from 'graphql-tag';
 import path from 'path';
+import { Entity, JoinColumn, OneToOne } from 'typeorm';
 
 import { initialData } from '../../../e2e-common/e2e-initial-data';
-import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
+import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
 
 import { testSuccessfulPaymentMethod } from './fixtures/test-payment-methods';
 import { AddItemToOrder } from './graphql/generated-e2e-shop-types';
@@ -72,6 +74,16 @@ const entitiesWithCustomFields = enumerate<keyof CustomFields>()(
     'Zone',
 );
 
+@Entity()
+class Vendor extends VendureEntity {
+    constructor() {
+        super();
+    }
+    @OneToOne(type => Product, { eager: true })
+    @JoinColumn()
+    featuredProduct: Product;
+}
+
 const customFieldConfig: CustomFields = {};
 for (const entity of entitiesWithCustomFields) {
     customFieldConfig[entity] = [
@@ -90,6 +102,15 @@ customFieldConfig.Product?.push(
     { name: 'cfProduct', type: 'relation', entity: Product, list: false },
     { name: 'cfShippingMethod', type: 'relation', entity: ShippingMethod, list: false },
     { name: 'cfInternalAsset', type: 'relation', entity: Asset, list: false, internal: true },
+    {
+        name: 'cfVendor',
+        type: 'relation',
+        entity: Vendor,
+        graphQLType: 'Vendor',
+        list: false,
+        internal: false,
+        public: true,
+    },
 );
 
 const testResolverSpy = jest.fn();
@@ -110,14 +131,26 @@ class TestResolver1636 {
 
 @VendurePlugin({
     imports: [PluginCommonModule],
+    entities: [Vendor],
     shopApiExtensions: {
         schema: gql`
             extend type Query {
                 getAssetTest(id: ID!): Boolean!
             }
+            type Vendor {
+                featuredProduct: Product
+            }
         `,
         resolvers: [TestResolver1636],
     },
+    adminApiExtensions: {
+        schema: gql`
+            type Vendor {
+                featuredProduct: Product
+            }
+        `,
+        resolvers: [],
+    },
 })
 class TestPlugin1636 {}
 
@@ -125,6 +158,7 @@ const customConfig = mergeConfig(testConfig(), {
     paymentOptions: {
         paymentMethodHandlers: [testSuccessfulPaymentMethod],
     },
+    // logger: new DefaultLogger({ level: LogLevel.Debug }),
     dbConnectionOptions: {
         timezone: 'Z',
     },
@@ -764,6 +798,26 @@ describe('Custom field relations', () => {
                 `);
                 assertCustomFieldIds(updateProductVariants[0].customFields, 'T_2', ['T_3', 'T_4']);
             });
+
+            // https://github.com/vendure-ecommerce/vendure/issues/1664
+            it('successfully gets product with eager-loading custom field relation', async () => {
+                const { product } = await shopClient.query(gql`
+                    query {
+                        product(id: "T_1") {
+                            id
+                            customFields {
+                                cfVendor {
+                                    featuredProduct {
+                                        id
+                                    }
+                                }
+                            }
+                        }
+                    }
+                `);
+
+                expect(product).toBeDefined();
+            });
         });
 
         describe('ProductOptionGroup, ProductOption entity', () => {

+ 62 - 0
packages/core/src/connection/transactional-connection.ts

@@ -10,12 +10,14 @@ import {
     getRepository,
     ObjectType,
     Repository,
+    SelectQueryBuilder,
 } from 'typeorm';
 
 import { RequestContext } from '../api/common/request-context';
 import { TRANSACTION_MANAGER_KEY } from '../common/constants';
 import { EntityNotFoundError } from '../common/error/errors';
 import { ChannelAware, SoftDeletable } from '../common/types/common-types';
+import { Logger } from '../config/index';
 import { VendureEntity } from '../entity/base/base.entity';
 
 import { TransactionWrapper } from './transaction-wrapper';
@@ -261,6 +263,7 @@ export class TransactionalConnection {
         options: FindOneOptions = {},
     ) {
         const qb = this.getRepository(ctx, entity).createQueryBuilder('entity');
+        options.relations = this.removeCustomFieldsWithEagerRelations(qb, options.relations);
         FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, options);
         if (options.loadEagerRelations !== false) {
             // tslint:disable-next-line:no-non-null-assertion
@@ -273,6 +276,65 @@ export class TransactionalConnection {
             .getOne();
     }
 
+    /**
+     * This is a work-around for this issue: https://github.com/vendure-ecommerce/vendure/issues/1664
+     *
+     * Explanation:
+     * When calling `FindOptionsUtils.joinEagerRelations()`, there appears to be a bug in TypeORM whereby
+     * it will throw the following error *if* the `options.relations` array contains any customField relations
+     * where the related entity itself has eagerly-loaded relations.
+     *
+     * For example, let's say we define a custom field on the Product entity like this:
+     * ```
+     * Product: [{
+     *   name: 'featuredFacet',
+     *   type: 'relation',
+     *   entity: Facet,
+     * }],
+     * ```
+     * and then we pass into `TransactionalConnection.findOneInChannel()` an options array of:
+     *
+     * ```
+     * { relations: ['customFields.featuredFacet'] }
+     * ```
+     * it will throw an error because the `Facet` entity itself has eager relations (namely the `translations` property).
+     * This will cause TypeORM to throw the error:
+     * ```
+     * TypeORMError: "entity__customFields" alias was not found. Maybe you forgot to join it?
+     * ```
+     *
+     * So this method introspects the QueryBuilder metadata and checks for any custom field relations which
+     * themselves have eager relations. If found, it removes those items from the `options.relations` array.
+     *
+     * TODO: Ideally create a minimal reproduction case and report in the TypeORM repo for an upstream fix.
+     */
+    private removeCustomFieldsWithEagerRelations(
+        qb: SelectQueryBuilder<any>,
+        relations: string[] = [],
+    ): string[] {
+        let resultingRelations = relations;
+        const mainAlias = qb.expressionMap.mainAlias;
+        const customFieldsMetadata = mainAlias?.metadata.embeddeds.find(
+            metadata => metadata.propertyName === 'customFields',
+        );
+        if (customFieldsMetadata) {
+            const customFieldRelationsWithEagerRelations = customFieldsMetadata.relations.filter(
+                relation => !!relation.inverseEntityMetadata.ownRelations.find(or => or.isEager === true),
+            );
+            for (const relation of customFieldRelationsWithEagerRelations) {
+                const propertyName = relation.propertyName;
+                const relationsToRemove = relations.filter(r => r.startsWith(`customFields.${propertyName}`));
+                if (relationsToRemove.length) {
+                    Logger.debug(
+                        `TransactionalConnection.findOneInChannel cannot automatically join relation [${mainAlias?.metadata.name}.customFields.${propertyName}]`,
+                    );
+                    resultingRelations = relations.filter(r => !r.startsWith(`customFields.${propertyName}`));
+                }
+            }
+        }
+        return resultingRelations;
+    }
+
     /**
      * @description
      * Like the TypeOrm `Repository.findByIds()` method, but limits the results to

+ 87 - 0
packages/dev-server/test-plugins/issue-1664.ts

@@ -0,0 +1,87 @@
+import {
+    LanguageCode,
+    LocaleString,
+    PluginCommonModule,
+    Product,
+    Translation,
+    VendureEntity,
+    VendurePlugin,
+} from '@vendure/core';
+import gql from 'graphql-tag';
+import { Column, Entity, ManyToOne, OneToMany } from 'typeorm';
+
+@Entity()
+class Vendor extends VendureEntity {
+    constructor(input: Partial<Vendor>) {
+        super(input);
+    }
+
+    description: LocaleString;
+
+    @Column()
+    name: string;
+
+    @OneToMany(() => Product, product => (product.customFields as any).vendor)
+    products: Product[];
+
+    @OneToMany(() => VendorTranslation, translation => translation.base, { eager: true })
+    translations: Array<Translation<Vendor>>;
+}
+
+@Entity()
+export class VendorTranslation extends VendureEntity implements Translation<Vendor> {
+    constructor(input?: Partial<Translation<Vendor>>) {
+        super(input);
+    }
+
+    @Column('varchar')
+    languageCode: LanguageCode;
+
+    @Column('text')
+    description: string;
+
+    @ManyToOne(() => Vendor, vendor => vendor.translations, { onDelete: 'CASCADE' })
+    base: Vendor;
+}
+
+const schema = gql`
+    type Vendor implements Node {
+        id: ID!
+        createdAt: DateTime!
+        updatedAt: DateTime!
+        name: String!
+        description: String!
+    }
+`;
+
+/**
+ * Test plugin for https://github.com/vendure-ecommerce/vendure/issues/1664
+ */
+@VendurePlugin({
+    imports: [PluginCommonModule],
+    entities: [Vendor, VendorTranslation],
+    shopApiExtensions: { schema, resolvers: [] },
+    adminApiExtensions: { schema, resolvers: [] },
+    configuration: config => {
+        config.customFields.Product.push({
+            name: 'vendor',
+            label: [{ languageCode: LanguageCode.en_AU, value: 'Vendor' }],
+            type: 'relation',
+            entity: Vendor,
+            eager: true,
+            nullable: false,
+            defaultValue: null,
+            ui: {
+                component: 'cp-product-vendor-selector',
+            },
+        });
+
+        config.customFields.Product.push({
+            name: 'shopifyId',
+            type: 'float',
+            public: false,
+        });
+        return config;
+    },
+})
+export class Test1664Plugin {}