fix(core): Fix uuid strategy, rework setting of ID data types in DB

Fixing the use of the UuidIdStrategy prompted a complete re-working of the way that ID data types are handled. The result is a much more resilient system that is not fragile and easily broken by changes in order of imports, as it was before. Now the ID data types are set lazily only after the config has loaded, using the TypeORM decorators as functions rather than static decorators.

Closes #176

packages/core/e2e/entity-uuid-strategy.e2e-spec.ts

@@ -0,0 +1,59 @@
+/* tslint:disable:no-non-null-assertion */
+import path from 'path';
+
+import { UuidIdStrategy } from '../src/config/entity-id-strategy/uuid-id-strategy';
+// This import is here to simulate the behaviour of
+// the package end-user importing symbols from the
+// @vendure/core barrel file. Doing so will then cause the
+// recusrsive evaluation of all imported files. This tests
+// the resilience of the id strategy implementation to the
+// order of file evaluation.
+import '../src/index';
+
+import { TEST_SETUP_TIMEOUT_MS } from './config/test-config';
+import { GetProductList } from './graphql/generated-e2e-admin-types';
+import { GET_PRODUCT_LIST } from './graphql/shared-definitions';
+import { TestAdminClient } from './test-client';
+import { TestServer } from './test-server';
+
+describe('UuidIdStrategy', () => {
+    const adminClient = new TestAdminClient();
+    const server = new TestServer();
+
+    beforeAll(async () => {
+        await server.init(
+            {
+                productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-full.csv'),
+                customerCount: 1,
+            },
+            {
+                entityIdStrategy: new UuidIdStrategy(),
+            },
+        );
+        await adminClient.init();
+    }, TEST_SETUP_TIMEOUT_MS);
+
+    afterAll(async () => {
+        await server.destroy();
+    });
+
+    it('uses uuids', async () => {
+        const { products } = await adminClient.query<GetProductList.Query, GetProductList.Variables>(
+            GET_PRODUCT_LIST,
+            {
+                options: {
+                    take: 1,
+                },
+            },
+        );
+
+        expect(isV4Uuid(products.items[0].id)).toBe(true);
+    });
+});
+
+/**
+ * Returns true if the id string matches the format for a v4 UUID.
+ */
+function isV4Uuid(id: string): boolean {
+    return /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i.test(id);
+}

packages/core/mock-data/clear-all-tables.ts

@@ -1,17 +1,25 @@
 import { createConnection } from 'typeorm';
 
+import { Type } from '../../common/lib/shared-types';
 import { isTestEnvironment } from '../e2e/utils/test-environment';
+import { getAllEntities, preBootstrapConfig } from '../src/bootstrap';
+import { defaultConfig } from '../src/config/default-config';
+import { VendureConfig } from '../src/config/vendure-config';
+import { coreEntitiesMap } from '../src/entity/entities';
 import { registerCustomEntityFields } from '../src/entity/register-custom-entity-fields';
+import { setEntityIdStrategy } from '../src/entity/set-entity-id-strategy';
 
 // tslint:disable:no-console
 // tslint:disable:no-floating-promises
 /**
  * Clears all tables in the detabase sepcified by the connectionOptions
  */
-export async function clearAllTables(config: any, logging = true) {
-    (config.dbConnectionOptions as any).entities = [__dirname + '/../src/**/*.entity.ts'];
+export async function clearAllTables(config: VendureConfig, logging = true) {
+    await preBootstrapConfig(config);
+    const entities = await getAllEntities(config);
+    (config.dbConnectionOptions as any).entities = entities;
+    const entityIdStrategy = config.entityIdStrategy || defaultConfig.entityIdStrategy;
     const name = isTestEnvironment() ? undefined : 'clearAllTables';
-    registerCustomEntityFields(config);
     const connection = await createConnection({ ...config.dbConnectionOptions, name });
     if (logging) {
         console.log('Clearing all tables...');

packages/core/src/api/resolvers/admin/role.resolver.ts

@@ -1,10 +1,10 @@
 import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
 import {
     MutationCreateRoleArgs,
+    MutationUpdateRoleArgs,
     Permission,
     QueryRoleArgs,
     QueryRolesArgs,
-    MutationUpdateRoleArgs,
 } from '@vendure/common/lib/generated-types';
 import { PaginatedList } from '@vendure/common/lib/shared-types';
 

packages/core/src/bootstrap.ts

@@ -10,27 +10,15 @@ import { getConfig, setConfig } from './config/config-helpers';
 import { DefaultLogger } from './config/logger/default-logger';
 import { Logger } from './config/logger/vendure-logger';
 import { RuntimeVendureConfig, VendureConfig } from './config/vendure-config';
+import { coreEntitiesMap } from './entity/entities';
 import { registerCustomEntityFields } from './entity/register-custom-entity-fields';
+import { setEntityIdStrategy } from './entity/set-entity-id-strategy';
 import { validateCustomFieldsConfig } from './entity/validate-custom-fields-config';
 import { getConfigurationFunction, getEntitiesFromPlugins } from './plugin/plugin-metadata';
 import { logProxyMiddlewares } from './plugin/plugin-utils';
 
 export type VendureBootstrapFunction = (config: VendureConfig) => Promise<INestApplication>;
 
-/**
- * https://github.com/vendure-ecommerce/vendure/issues/152
- * fix race condition when modifying DB
- * @param {VendureConfig} userConfig
- */
-const disableSynchronize = (userConfig: ReadOnlyRequired<VendureConfig>): ReadOnlyRequired<VendureConfig> => {
-    const config = { ...userConfig };
-    config.dbConnectionOptions = {
-        ...userConfig.dbConnectionOptions,
-        synchronize: false,
-    } as ConnectionOptions;
-    return config;
-};
-
 /**
  * @description
  * Bootstraps the Vendure server. This is the entry point to the application.
@@ -156,10 +144,6 @@ export async function preBootstrapConfig(
         setConfig(userConfig);
     }
 
-    // Entities *must* be loaded after the user config is set in order for the
-    // base VendureEntity to be correctly configured with the primary key type
-    // specified in the EntityIdStrategy.
-    const pluginEntities = getEntitiesFromPlugins(userConfig.plugins);
     const entities = await getAllEntities(userConfig);
     const { coreSubscribersMap } = await import('./entity/subscribers');
     setConfig({
@@ -170,6 +154,7 @@ export async function preBootstrapConfig(
     });
 
     let config = getConfig();
+    setEntityIdStrategy(config.entityIdStrategy, entities);
     const customFieldValidationResult = validateCustomFieldsConfig(config.customFields, entities);
     if (!customFieldValidationResult.valid) {
         process.exitCode = 1;
@@ -197,8 +182,7 @@ async function runPluginConfigurations(config: RuntimeVendureConfig): Promise<Ru
 /**
  * Returns an array of core entities and any additional entities defined in plugins.
  */
-async function getAllEntities(userConfig: Partial<VendureConfig>): Promise<Array<Type<any>>> {
-    const { coreEntitiesMap } = await import('./entity/entities');
+export async function getAllEntities(userConfig: Partial<VendureConfig>): Promise<Array<Type<any>>> {
     const coreEntities = Object.values(coreEntitiesMap) as Array<Type<any>>;
     const pluginEntities = getEntitiesFromPlugins(userConfig.plugins);
 
@@ -284,3 +268,16 @@ function logWelcomeMessage(config: VendureConfig) {
     logProxyMiddlewares(config);
     Logger.info(`=================================================`);
 }
+
+/**
+ * Fix race condition when modifying DB
+ * See: https://github.com/vendure-ecommerce/vendure/issues/152
+ */
+function disableSynchronize(userConfig: ReadOnlyRequired<VendureConfig>): ReadOnlyRequired<VendureConfig> {
+    const config = { ...userConfig };
+    config.dbConnectionOptions = {
+        ...userConfig.dbConnectionOptions,
+        synchronize: false,
+    } as ConnectionOptions;
+    return config;
+}

packages/core/src/config/config-helpers.ts

@@ -1,7 +1,5 @@
 import { DeepPartial } from '@vendure/common/lib/shared-types';
 
-import { ReadOnlyRequired } from '../common/types/common-types';
-
 import { defaultConfig } from './default-config';
 import { mergeConfig } from './merge-config';
 import { RuntimeVendureConfig, VendureConfig } from './vendure-config';
@@ -24,18 +22,3 @@ export function setConfig(userConfig: DeepPartial<VendureConfig>): void {
 export function getConfig(): Readonly<RuntimeVendureConfig> {
     return activeConfig;
 }
-
-/**
- * Returns the type argument to be passed to the PrimaryGeneratedColumn() decorator
- * of the base VendureEntity.
- */
-export function primaryKeyType(): any {
-    return activeConfig.entityIdStrategy.primaryKeyType;
-}
-
-/**
- * Returns the DB data type of ID columns based on the configured primaryKeyType
- */
-export function idType(): 'int' | 'varchar' {
-    return activeConfig.entityIdStrategy.primaryKeyType === 'increment' ? 'int' : 'varchar';
-}

packages/core/src/entity/base/base.entity.ts

@@ -1,9 +1,7 @@
 import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { CreateDateColumn, PrimaryGeneratedColumn, UpdateDateColumn } from 'typeorm';
 
-import { primaryKeyType } from '../../config/config-helpers';
-
-const keyType = primaryKeyType();
+import { PrimaryGeneratedId } from '../entity-id.decorator';
 
 /**
  * @description
@@ -21,7 +19,8 @@ export abstract class VendureEntity {
         }
     }
 
-    @PrimaryGeneratedColumn(keyType) id: ID;
+    @PrimaryGeneratedId()
+    id: ID;
 
     @CreateDateColumn() createdAt: Date;
 

packages/core/src/entity/entity-id.decorator.ts

@@ -0,0 +1,71 @@
+import { Type } from '@vendure/common/lib/shared-types';
+
+interface IdColumnOptions {
+    /** Whether the field is nullable. Defaults to false */
+    nullable?: boolean;
+    /** Whether this is a primary key. Defaults to false */
+    primary?: boolean;
+}
+
+interface IdColumnConfig {
+    name: string;
+    entity: any;
+    options?: IdColumnOptions;
+}
+
+const idColumnRegistry = new Map<any, IdColumnConfig[]>();
+let primaryGeneratedColumn: { entity: any; name: string } | undefined;
+
+/**
+ * Decorates a property which should be marked as a generated primary key.
+ * Designed to be applied to the VendureEntity id property.
+ */
+export function PrimaryGeneratedId() {
+    return (entity: any, propertyName: string) => {
+        primaryGeneratedColumn = {
+            entity,
+            name: propertyName,
+        };
+    };
+}
+
+/**
+ * Decorates a property which points to another entity by ID. This custom decorator is needed
+ * because we do not know the data type of the ID column until runtime, when we have access
+ * to the configured EntityIdStrategy.
+ */
+export function EntityId(options?: IdColumnOptions) {
+    return (entity: any, propertyName: string) => {
+        const idColumns = idColumnRegistry.get(entity);
+        const entry = { name: propertyName, entity, options };
+        if (idColumns) {
+            idColumns.push(entry);
+        } else {
+            idColumnRegistry.set(entity, [entry]);
+        }
+    };
+}
+
+/**
+ * Returns any columns on the entity which have been decorated with the {@link EntityId}
+ * decorator.
+ */
+export function getIdColumnsFor(entityType: Type<any>): IdColumnConfig[] {
+    const match = Array.from(idColumnRegistry.entries()).find(
+        ([entity, columns]) => entity.constructor === entityType,
+    );
+    return match ? match[1] : [];
+}
+
+/**
+ * Returns the entity and property name that was decorated with the {@link PrimaryGeneratedId}
+ * decorator.
+ */
+export function getPrimaryGeneratedIdColumn(): { entity: any; name: string } {
+    if (!primaryGeneratedColumn) {
+        throw new Error(
+            `primaryGeneratedColumn is undefined. The base VendureEntity must have the @PrimaryGeneratedId() decorator set on its id property.`,
+        );
+    }
+    return primaryGeneratedColumn;
+}

packages/core/src/entity/order-item/order-item.entity.ts

@@ -3,8 +3,8 @@ import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, ManyToOne, OneToOne, RelationId } from 'typeorm';
 
 import { Calculated } from '../../common/calculated-decorator';
-import { idType } from '../../config/config-helpers';
 import { VendureEntity } from '../base/base.entity';
+import { EntityId } from '../entity-id.decorator';
 import { Fulfillment } from '../fulfillment/fulfillment.entity';
 import { OrderLine } from '../order-line/order-line.entity';
 import { Refund } from '../refund/refund.entity';
@@ -36,13 +36,13 @@ export class OrderItem extends VendureEntity {
     @ManyToOne(type => Fulfillment)
     fulfillment: Fulfillment;
 
-    @Column({ type: idType(), nullable: true })
+    @EntityId({ nullable: true })
     fulfillmentId: ID | null;
 
     @ManyToOne(type => Refund)
     refund: Refund;
 
-    @Column({ type: idType(), nullable: true })
+    @EntityId({ nullable: true })
     refundId: ID | null;
 
     @OneToOne(type => Cancellation, cancellation => cancellation.orderItem)

packages/core/src/entity/order/order.entity.ts

@@ -1,18 +1,17 @@
 import { Adjustment, AdjustmentType, CurrencyCode, OrderAddress } from '@vendure/common/lib/generated-types';
 import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
-import { Column, Entity, JoinTable, ManyToMany, ManyToOne, OneToMany, RelationId } from 'typeorm';
+import { Column, Entity, ManyToOne, OneToMany } from 'typeorm';
 
 import { Calculated } from '../../common/calculated-decorator';
-import { idType } from '../../config/config-helpers';
 import { HasCustomFields } from '../../config/custom-field/custom-field-types';
 import { OrderState } from '../../service/helpers/order-state-machine/order-state';
 import { VendureEntity } from '../base/base.entity';
 import { CustomOrderFields } from '../custom-entity-fields';
 import { Customer } from '../customer/customer.entity';
+import { EntityId } from '../entity-id.decorator';
 import { OrderItem } from '../order-item/order-item.entity';
 import { OrderLine } from '../order-line/order-line.entity';
 import { Payment } from '../payment/payment.entity';
-import { Refund } from '../refund/refund.entity';
 import { ShippingMethod } from '../shipping-method/shipping-method.entity';
 
 /**
@@ -64,7 +63,7 @@ export class Order extends VendureEntity implements HasCustomFields {
 
     @Column() subTotal: number;
 
-    @Column({ type: idType(), nullable: true })
+    @EntityId({ nullable: true })
     shippingMethodId: ID | null;
 
     @ManyToOne(type => ShippingMethod)

packages/core/src/entity/product-option/product-option.entity.ts

@@ -2,10 +2,10 @@ import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, ManyToOne, OneToMany } from 'typeorm';
 
 import { LocaleString, Translatable, Translation } from '../../common/types/locale-types';
-import { idType } from '../../config/config-helpers';
 import { HasCustomFields } from '../../config/custom-field/custom-field-types';
 import { VendureEntity } from '../base/base.entity';
 import { CustomProductOptionFields } from '../custom-entity-fields';
+import { EntityId } from '../entity-id.decorator';
 import { ProductOptionGroup } from '../product-option-group/product-option-group.entity';
 
 import { ProductOptionTranslation } from './product-option-translation.entity';
@@ -32,7 +32,7 @@ export class ProductOption extends VendureEntity implements Translatable, HasCus
     @ManyToOne(type => ProductOptionGroup, group => group.options)
     group: ProductOptionGroup;
 
-    @Column({ type: idType() })
+    @EntityId()
     groupId: ID;
 
     @Column(type => CustomProductOptionFields)

packages/core/src/entity/product-variant/product-variant-price.entity.ts

@@ -1,8 +1,8 @@
 import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, ManyToOne } from 'typeorm';
 
-import { idType } from '../../config/config-helpers';
 import { VendureEntity } from '../base/base.entity';
+import { EntityId } from '../entity-id.decorator';
 
 import { ProductVariant } from './product-variant.entity';
 
@@ -14,7 +14,7 @@ export class ProductVariantPrice extends VendureEntity {
 
     @Column() price: number;
 
-    @Column({ type: idType() }) channelId: ID;
+    @EntityId() channelId: ID;
 
     @ManyToOne(type => ProductVariant, variant => variant.productVariantPrices)
     variant: ProductVariant;

packages/core/src/entity/refund/refund.entity.ts

@@ -1,9 +1,9 @@
 import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, JoinColumn, JoinTable, ManyToOne, OneToMany } from 'typeorm';
 
-import { idType } from '../../config/config-helpers';
 import { RefundState } from '../../service/helpers/refund-state-machine/refund-state';
 import { VendureEntity } from '../base/base.entity';
+import { EntityId } from '../entity-id.decorator';
 import { OrderItem } from '../order-item/order-item.entity';
 import { Payment, PaymentMetadata } from '../payment/payment.entity';
 
@@ -37,7 +37,7 @@ export class Refund extends VendureEntity {
     @JoinColumn()
     payment: Payment;
 
-    @Column({ type: idType() })
+    @EntityId()
     paymentId: ID;
 
     @Column('simple-json') metadata: PaymentMetadata;

packages/core/src/entity/set-entity-id-strategy.ts

@@ -0,0 +1,30 @@
+import { Type } from '@vendure/common/lib/shared-types';
+import { Column, PrimaryColumn, PrimaryGeneratedColumn } from 'typeorm';
+
+import { EntityIdStrategy } from '../config/entity-id-strategy/entity-id-strategy';
+
+import { getIdColumnsFor, getPrimaryGeneratedIdColumn } from './entity-id.decorator';
+
+export function setEntityIdStrategy(entityIdStrategy: EntityIdStrategy, entities: Array<Type<any>>) {
+    setBaseEntityIdType(entityIdStrategy);
+    setEntityIdColumnTypes(entityIdStrategy, entities);
+}
+
+function setEntityIdColumnTypes(entityIdStrategy: EntityIdStrategy, entities: Array<Type<any>>) {
+    const columnDataType = entityIdStrategy.primaryKeyType === 'increment' ? 'int' : 'varchar';
+    for (const EntityCtor of entities) {
+        const columnConfig = getIdColumnsFor(EntityCtor);
+        for (const { name, options, entity } of columnConfig) {
+            Column({
+                type: columnDataType,
+                nullable: (options && options.nullable) || false,
+                primary: (options && options.primary) || false,
+            })(entity, name);
+        }
+    }
+}
+
+function setBaseEntityIdType(entityIdStrategy: EntityIdStrategy) {
+    const { entity, name } = getPrimaryGeneratedIdColumn();
+    PrimaryGeneratedColumn(entityIdStrategy.primaryKeyType as any)(entity, name);
+}

packages/core/src/entity/validate-custom-fields-config.spec.ts

@@ -6,15 +6,11 @@ import { coreEntitiesMap } from './entities';
 import { validateCustomFieldsConfig } from './validate-custom-fields-config';
 
 describe('validateCustomFieldsConfig()', () => {
-
     const allEntities = Object.values(coreEntitiesMap) as Array<Type<any>>;
 
     it('valid config', () => {
         const config: CustomFields = {
-            Product: [
-                { name: 'foo', type: 'string' },
-                { name: 'bar', type: 'localeString' },
-            ],
+            Product: [{ name: 'foo', type: 'string' }, { name: 'bar', type: 'localeString' }],
         };
         const result = validateCustomFieldsConfig(config, allEntities);
 
@@ -24,17 +20,12 @@ describe('validateCustomFieldsConfig()', () => {
 
     it('invalid localeString', () => {
         const config: CustomFields = {
-            User: [
-                { name: 'foo', type: 'string' },
-                { name: 'bar', type: 'localeString' },
-            ],
+            User: [{ name: 'foo', type: 'string' }, { name: 'bar', type: 'localeString' }],
         };
         const result = validateCustomFieldsConfig(config, allEntities);
 
         expect(result.valid).toBe(false);
-        expect(result.errors).toEqual([
-            'User entity does not support custom fields of type "localeString"',
-        ]);
+        expect(result.errors).toEqual(['User entity does not support custom fields of type "localeString"']);
     });
 
     it('valid names', () => {
@@ -112,37 +103,27 @@ describe('validateCustomFieldsConfig()', () => {
 
     it('name conflict with existing fields', () => {
         const config: CustomFields = {
-            Product: [
-                { name: 'id', type: 'string' },
-            ],
+            Product: [{ name: 'createdAt', type: 'string' }],
         };
         const result = validateCustomFieldsConfig(config, allEntities);
 
         expect(result.valid).toBe(false);
-        expect(result.errors).toEqual([
-            'Product entity already has a field named "id"',
-        ]);
+        expect(result.errors).toEqual(['Product entity already has a field named "createdAt"']);
     });
 
     it('name conflict with existing fields in translation', () => {
         const config: CustomFields = {
-            Product: [
-                { name: 'name', type: 'string' },
-            ],
+            Product: [{ name: 'name', type: 'string' }],
         };
         const result = validateCustomFieldsConfig(config, allEntities);
 
         expect(result.valid).toBe(false);
-        expect(result.errors).toEqual([
-            'Product entity already has a field named "name"',
-        ]);
+        expect(result.errors).toEqual(['Product entity already has a field named "name"']);
     });
 
     it('non-nullable must have defaultValue', () => {
         const config: CustomFields = {
-            Product: [
-                { name: 'foo', type: 'string', nullable: false },
-            ],
+            Product: [{ name: 'foo', type: 'string', nullable: false }],
         };
         const result = validateCustomFieldsConfig(config, allEntities);
 
@@ -151,5 +132,4 @@ describe('validateCustomFieldsConfig()', () => {
             'Product entity custom field "foo" is non-nullable and must have a defaultValue',
         ]);
     });
-
 });

packages/core/src/entity/validate-custom-fields-config.ts

@@ -2,7 +2,8 @@ import { Type } from '@vendure/common/lib/shared-types';
 import { getMetadataArgsStorage } from 'typeorm';
 
 import { CustomFieldConfig, CustomFields } from '../config/custom-field/custom-field-types';
-import { VendureEntity } from '../entity/base/base.entity';
+
+import { VendureEntity } from './base/base.entity';
 
 function validateCustomFieldsForEntity(
     entity: Type<VendureEntity>,
@@ -82,9 +83,7 @@ function assetNonNullablesHaveDefaults(entityName: string, customFields: CustomF
     for (const field of customFields) {
         if (field.nullable === false && field.defaultValue === undefined) {
             errors.push(
-                `${entityName} entity custom field "${
-                    field.name
-                }" is non-nullable and must have a defaultValue`,
+                `${entityName} entity custom field "${field.name}" is non-nullable and must have a defaultValue`,
             );
         }
     }

packages/core/src/plugin/default-search-plugin/search-index-item.entity.ts

@@ -2,7 +2,7 @@ import { CurrencyCode, LanguageCode } from '@vendure/common/lib/generated-types'
 import { ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, Index, PrimaryColumn } from 'typeorm';
 
-import { idType } from '../../config/config-helpers';
+import { EntityId } from '../../entity/entity-id.decorator';
 
 @Entity()
 export class SearchIndexItem {
@@ -14,13 +14,13 @@ export class SearchIndexItem {
         }
     }
 
-    @PrimaryColumn({ type: idType() })
+    @EntityId({ primary: true })
     productVariantId: ID;
 
     @PrimaryColumn('varchar')
     languageCode: LanguageCode;
 
-    @Column({ type: idType() })
+    @EntityId()
     productId: ID;
 
     @Column()