feat(core): validate unique tableName

Closes #4149
mikro-orm · Apr 23, 2023 · 0693029 · 0693029
1 parent eba4563
commit 0693029
Show file tree

Hide file tree

Showing 15 changed files with 54 additions and 28 deletions.
diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts
@@ -220,8 +220,8 @@ export class MetadataError<T extends AnyEntity = AnyEntity> extends ValidationEr
     return new MetadataError('Only abstract entities were discovered, maybe you forgot to use @Entity() decorator?');
   }
 
-  static duplicateEntityDiscovered(paths: string[]): MetadataError {
-    return new MetadataError(`Duplicate entity names are not allowed: ${paths.join(', ')}`);
+  static duplicateEntityDiscovered(paths: string[], subject = 'entity names'): MetadataError {
+    return new MetadataError(`Duplicate ${subject} are not allowed: ${paths.join(', ')}`);
   }
 
   static multipleDecorators(entityName: string, propertyName: string): MetadataError {

diff --git a/packages/core/src/metadata/MetadataValidator.ts b/packages/core/src/metadata/MetadataValidator.ts
@@ -64,6 +64,13 @@ export class MetadataValidator {
       throw MetadataError.duplicateEntityDiscovered(duplicates);
     }
 
+    const tableNames = discovered.filter(meta => !meta.abstract && meta === meta.root && (meta.tableName || meta.collection));
+    const duplicateTableNames = Utils.findDuplicates(tableNames.map(meta => meta.tableName || meta.collection));
+
+    if (duplicateTableNames.length > 0) {
+      throw MetadataError.duplicateEntityDiscovered(duplicateTableNames, 'table names');
+    }
+
     // validate we found at least one entity (not just abstract/base entities)
     if (discovered.filter(meta => meta.name).length === 0 && warnWhenNoEntities) {
       throw MetadataError.onlyAbstractEntitiesDiscovered();

diff --git a/tests/MetadataValidator.test.ts b/tests/MetadataValidator.test.ts
@@ -1,5 +1,5 @@
 import type { Dictionary } from '@mikro-orm/core';
-import { ReferenceType, MetadataStorage, MetadataValidator } from '@mikro-orm/core';
+import { ReferenceType, MetadataStorage, MetadataValidator, EntitySchema } from '@mikro-orm/core';
 
 describe('MetadataValidator', () => {
 
@@ -137,6 +137,25 @@ describe('MetadataValidator', () => {
     expect(() => validator.validateEntityDefinition(new MetadataStorage(meta as any), 'AuthorProfile')).not.toThrowError();
   });
 
+  test('validates duplicities in tableName', async () => {
+    const properties: Dictionary = {
+      id: { reference: 'scalar', primary: true, name: 'id', type: 'number' },
+      name: { reference: 'scalar', name: 'name', type: 'string' },
+      age: { reference: 'scalar', name: 'age', type: 'string' },
+    };
+    const schema1 = EntitySchema.fromMetadata({
+      name: 'Foo1',
+      tableName: 'foo',
+      properties,
+    } as any);
+    const schema2 = EntitySchema.fromMetadata({
+      name: 'Foo2',
+      tableName: 'foo',
+      properties,
+    } as any);
+    expect(() => validator.validateDiscovered([schema1.meta, schema2.meta], true)).toThrowError(`Duplicate table names are not allowed: foo`);
+  });
+
   test('MetadataStorage.get throws when no metadata found', async () => {
     const storage = new MetadataStorage({});
     expect(() => storage.get('Test')).toThrowError('Metadata for entity Test not found');

diff --git a/tests/features/schema-generator/GH1904.test.ts b/tests/features/schema-generator/GH1904.test.ts
@@ -82,32 +82,32 @@ describe('ignore specific schema changes (GH 1904)', () => {
   test('schema generator respects ignoreSchemaChanges for `type`', async () => {
     const diff0 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff0).toBe('');
-    await orm.discoverEntity(Book2);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book2);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe('');
 
     // Once we remove ignoreSchemaChanges, we should see a diff again.
-    await orm.discoverEntity(Book5);
     orm.getMetadata().reset('Book2');
+    await orm.discoverEntity(Book5);
     const diff2 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff2).toBe('alter table `book` modify `changing_field` timestamp not null;\n\n');
   });
 
   test('schema generator respects ignoreSchemaChanges for `extra`', async () => {
     const diff0 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff0).toBe('');
-    await orm.discoverEntity(Book3);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book3);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe('');
   });
 
   test('schema generator respects ignoreSchemaChanges for `extra` and `type`', async () => {
     const diff0 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff0).toBe('');
-    await orm.discoverEntity(Book4);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book4);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe('');
   });

diff --git a/tests/features/schema-generator/GH2386.test.ts b/tests/features/schema-generator/GH2386.test.ts
@@ -61,14 +61,14 @@ describe('changing column in mysql (GH 2386)', () => {
   test('schema generator respect indexes on FKs on column update', async () => {
     const diff0 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff0).toBe('');
-    await orm.discoverEntity(Book2);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book2);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe('alter table `book` modify `updated_at` timestamp not null default current_timestamp;\n\n');
     await orm.schema.execute(diff1);
 
-    await orm.discoverEntity(Book3);
     orm.getMetadata().reset('Book2');
+    await orm.discoverEntity(Book3);
     const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff3).toBe('alter table `book` modify `updated_at` timestamp not null default current_timestamp on update current_timestamp;\n\n');
     await orm.schema.execute(diff3);

diff --git a/tests/features/schema-generator/adding-fk-column.sqlite.test.ts b/tests/features/schema-generator/adding-fk-column.sqlite.test.ts
@@ -45,8 +45,8 @@ describe('adding FK column (GH 942)', () => {
   afterAll(() => orm.close(true));
 
   test('schema: adding 1:1 relation', async () => {
-    await orm.discoverEntity(User2);
     orm.getMetadata().reset('User');
+    await orm.discoverEntity(User2);
     const diff1 = await orm.schema.getUpdateSchemaSQL();
     expect(diff1).toMatchSnapshot();
     await orm.schema.execute(diff1);

diff --git a/tests/features/schema-generator/changing-column-type.mysql.test.ts b/tests/features/schema-generator/changing-column-type.mysql.test.ts
@@ -73,26 +73,26 @@ describe('changing column in mysql (GH 2407)', () => {
   afterAll(() => orm.close(true));
 
   test('schema generator respect indexes on FKs on column update', async () => {
-    await orm.discoverEntity(Book2);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book2);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe('alter table `book` modify `my_column` tinyint(1) not null default 1 comment \'this is a comment\';\n\n');
     await orm.schema.execute(diff1);
 
-    await orm.discoverEntity(Book3);
     orm.getMetadata().reset('Book2');
+    await orm.discoverEntity(Book3);
     const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff3).toBe('alter table `book` modify `my_column` tinyint(1) null default 123;\n\n');
     await orm.schema.execute(diff3);
 
-    await orm.discoverEntity(Book4);
     orm.getMetadata().reset('Book3');
+    await orm.discoverEntity(Book4);
     const diff4 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff4).toBe('alter table `book` modify `my_column` tinyint(1) null default 123 comment \'lalala\';\n\n');
     await orm.schema.execute(diff4);
 
-    await orm.discoverEntity(Book5);
     orm.getMetadata().reset('Book4');
+    await orm.discoverEntity(Book5);
     const diff5 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff5).toBe('alter table `book` modify `my_column` tinyint(1) null default 123 comment \'lololo\';\n\n');
     await orm.schema.execute(diff5);

diff --git a/tests/features/schema-generator/changing-column-type.postgres.test.ts b/tests/features/schema-generator/changing-column-type.postgres.test.ts
@@ -72,29 +72,29 @@ describe('changing column in postgres (GH 2407)', () => {
   afterAll(() => orm.close(true));
 
   test('schema generator respect indexes on FKs on column update', async () => {
-    await orm.discoverEntity(Book2);
     orm.getMetadata().reset('Book1');
+    await orm.discoverEntity(Book2);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toBe(`alter table "book" alter column "my_column" type boolean using ("my_column"::boolean);
 alter table "book" alter column "my_column" set default false;
 alter table "book" alter column "my_column" set not null;\n\n`);
     await orm.schema.execute(diff1);
 
-    await orm.discoverEntity(Book3);
     orm.getMetadata().reset('Book2');
+    await orm.discoverEntity(Book3);
     const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff3).toBe(`alter table "book" alter column "my_column" type boolean using ("my_column"::boolean);
 alter table "book" alter column "my_column" drop not null;\n\n`);
     await orm.schema.execute(diff3);
 
-    await orm.discoverEntity(Book4);
     orm.getMetadata().reset('Book3');
+    await orm.discoverEntity(Book4);
     const diff4 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff4).toBe(`comment on column "book"."my_column" is 'lalala';\n\n`);
     await orm.schema.execute(diff4);
 
-    await orm.discoverEntity(Book5);
     orm.getMetadata().reset('Book4');
+    await orm.discoverEntity(Book5);
     const diff5 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff5).toBe(`comment on column "book"."my_column" is 'lololo';\n\n`);
     await orm.schema.execute(diff5);

diff --git a/tests/features/schema-generator/changing-pk-type.mysql.test.ts b/tests/features/schema-generator/changing-pk-type.mysql.test.ts
@@ -76,8 +76,8 @@ describe('changing PK column type [mysql] (GH 1480)', () => {
     const generator = orm.schema;
     const testMigration = async (e1: Constructor, e2: Constructor | undefined, snap: string) => {
       if (e2) {
-        await orm.discoverEntity(e2);
         orm.getMetadata().reset(e1.name);
+        await orm.discoverEntity(e2);
       }
 
       const diff = await generator.getUpdateSchemaMigrationSQL({ wrap: false });

diff --git a/tests/features/schema-generator/changing-pk-type.postgres.test.ts b/tests/features/schema-generator/changing-pk-type.postgres.test.ts
@@ -73,8 +73,8 @@ describe('changing PK column type [postgres] (GH 1480)', () => {
   test('changing PK type', async () => {
     const testMigration = async (e1: Constructor, e2: Constructor | undefined, snap: string) => {
       if (e2) {
-        await orm.discoverEntity(e2);
         orm.getMetadata().reset(e1.name);
+        await orm.discoverEntity(e2);
       }
 
       const diff = await orm.schema.getUpdateSchemaMigrationSQL({ wrap: false });

diff --git a/tests/features/schema-generator/comment-diffing.mysql.test.ts b/tests/features/schema-generator/comment-diffing.mysql.test.ts
@@ -64,8 +64,8 @@ describe('comment diffing in mysql', () => {
   afterAll(() => orm.close(true));
 
   test('schema orm.schema updates comments', async () => {
-    await orm.discoverEntity(Book1);
     orm.getMetadata().reset('Book0');
+    await orm.discoverEntity(Book1);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toMatchSnapshot();
     await orm.schema.execute(diff1);

diff --git a/tests/features/schema-generator/comment-diffing.postgres.test.ts b/tests/features/schema-generator/comment-diffing.postgres.test.ts
@@ -60,20 +60,20 @@ beforeAll(async () => {
 afterAll(() => orm.close(true));
 
 test('comment diffing in postgres', async () => {
-  await orm.discoverEntity(Book1);
   orm.getMetadata().reset('Book0');
+  await orm.discoverEntity(Book1);
   const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
   expect(diff1).toMatchSnapshot();
   await orm.schema.execute(diff1);
 
-  orm.getMetadata().reset('Book1');
   await orm.discoverEntity(Book2);
+  orm.getMetadata().reset('Book1');
   const diff2 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
   expect(diff2).toMatchSnapshot();
   await orm.schema.execute(diff2);
 
-  orm.getMetadata().reset('Book2');
   await orm.discoverEntity(Book3);
+  orm.getMetadata().reset('Book2');
   const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
   expect(diff3).toMatchSnapshot();
   await orm.schema.execute(diff3);

diff --git a/tests/features/schema-generator/length-diffing.mysql.test.ts b/tests/features/schema-generator/length-diffing.mysql.test.ts
@@ -108,8 +108,8 @@ describe('length diffing in mysql', () => {
   afterAll(() => orm.close(true));
 
   test('schema generator updates column types when length changes (varchar, decimal, ...)', async () => {
-    await orm.discoverEntity(Book1);
     orm.getMetadata().reset('Book0');
+    await orm.discoverEntity(Book1);
     const diff1 = await generator.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toMatchSnapshot();
     await generator.execute(diff1);

diff --git a/tests/features/schema-generator/serial-property.postgres.test.ts b/tests/features/schema-generator/serial-property.postgres.test.ts
@@ -94,8 +94,8 @@ test('schema generator works with non-pk autoincrement columns (serial)', async
   await generator.refreshDatabase();
   await expect(generator.getUpdateSchemaSQL()).resolves.toBe('');
 
-  await orm.discoverEntity(Something1);
   orm.getMetadata().reset('Something0');
+  await orm.discoverEntity(Something1);
   const diff1 = await generator.getUpdateSchemaSQL();
   expect(diff1).toMatchSnapshot();
   await generator.execute(diff1);

diff --git a/tests/issues/GH3339.test.ts b/tests/issues/GH3339.test.ts
@@ -63,14 +63,14 @@ describe('GH issue 3339', () => {
   afterAll(() => orm.close(true));
 
   test('reference schema name when updating column names inside sql', async () => {
-    await orm.discoverEntity(Customer2);
     orm.getMetadata().reset('Customer1');
+    await orm.discoverEntity(Customer2);
     const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff1).toContain('"gh3339"."Customer"');
     await orm.schema.execute(diff1);
 
-    await orm.discoverEntity(Customer3);
     orm.getMetadata().reset('Customer2');
+    await orm.discoverEntity(Customer3);
     const diff2 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
     expect(diff2).toContain('"gh3339"."Customer"');
     await orm.schema.execute(diff2);