fix(validation): throw when multiple property decorators are used

mikro-orm · Mar 3, 2020 · 5a8d3f2 · 5a8d3f2
1 parent 00d7543
commit 5a8d3f2
Show file tree

Hide file tree

Showing 15 changed files with 48 additions and 14 deletions.
diff --git a/lib/decorators/ManyToMany.ts b/lib/decorators/ManyToMany.ts
@@ -1,7 +1,7 @@
 import { ReferenceOptions } from './Property';
 import { MetadataStorage } from '../metadata';
 import { Utils } from '../utils';
-import { ReferenceType } from '../entity';
+import { EntityValidator, ReferenceType } from '../entity';
 import { EntityName, EntityProperty, AnyEntity } from '../typings';
 import { QueryOrder } from '../query';
 
@@ -13,6 +13,7 @@ export function ManyToMany<T extends AnyEntity<T>>(
   return function (target: AnyEntity, propertyName: string) {
     options = Utils.isObject<ManyToManyOptions<T>>(entity) ? entity : { ...options, entity, mappedBy };
     const meta = MetadataStorage.getMetadata(target.constructor.name);
+    EntityValidator.validateSingleDecorator(meta, propertyName);
     Utils.lookupPathFromDecorator(meta);
     const property = { name: propertyName, reference: ReferenceType.MANY_TO_MANY } as EntityProperty<T>;
     meta.properties[propertyName] = Object.assign(property, options);

diff --git a/lib/decorators/ManyToOne.ts b/lib/decorators/ManyToOne.ts
@@ -1,7 +1,7 @@
 import { ReferenceOptions } from './Property';
 import { MetadataStorage } from '../metadata';
 import { Utils } from '../utils';
-import { ReferenceType } from '../entity';
+import { EntityValidator, ReferenceType } from '../entity';
 import { AnyEntity, EntityName, EntityProperty } from '../typings';
 
 export function ManyToOne<T extends AnyEntity<T>>(
@@ -16,6 +16,7 @@ export function ManyToOne<T extends AnyEntity<T>>(
     }
 
     const meta = MetadataStorage.getMetadata(target.constructor.name);
+    EntityValidator.validateSingleDecorator(meta, propertyName);
     Utils.lookupPathFromDecorator(meta);
     const property = { name: propertyName, reference: ReferenceType.MANY_TO_ONE } as EntityProperty;
     meta.properties[propertyName] = Object.assign(property, options);

diff --git a/lib/decorators/OneToMany.ts b/lib/decorators/OneToMany.ts
@@ -1,7 +1,7 @@
 import { ReferenceOptions } from './Property';
 import { MetadataStorage } from '../metadata';
 import { Utils } from '../utils';
-import { ReferenceType } from '../entity';
+import { EntityValidator, ReferenceType } from '../entity';
 import { QueryOrder } from '../query';
 import { EntityName, EntityProperty, AnyEntity } from '../typings';
 
@@ -22,6 +22,7 @@ export function createOneToDecorator<T extends AnyEntity<T>>(
   return function (target: AnyEntity, propertyName: string) {
     options = Utils.isObject<OneToManyOptions<T>>(entity) ? entity : { ...options, entity, mappedBy };
     const meta = MetadataStorage.getMetadata(target.constructor.name);
+    EntityValidator.validateSingleDecorator(meta, propertyName);
     Utils.lookupPathFromDecorator(meta);
 
     if (reference === ReferenceType.ONE_TO_MANY) {

diff --git a/lib/decorators/Property.ts b/lib/decorators/Property.ts
@@ -1,11 +1,12 @@
 import { MetadataStorage } from '../metadata';
 import { Utils } from '../utils';
-import { Cascade, ReferenceType } from '../entity';
+import { Cascade, EntityValidator, ReferenceType } from '../entity';
 import { EntityName, EntityProperty, AnyEntity } from '../typings';
 
 export function Property(options: PropertyOptions = {}): Function {
   return function (target: AnyEntity, propertyName: string) {
     const meta = MetadataStorage.getMetadata(target.constructor.name);
+    EntityValidator.validateSingleDecorator(meta, propertyName);
     Utils.lookupPathFromDecorator(meta);
     options.name = options.name || propertyName;
     const prop = Object.assign({ reference: ReferenceType.SCALAR }, options) as EntityProperty;

diff --git a/lib/entity/EntityValidator.ts b/lib/entity/EntityValidator.ts
@@ -8,6 +8,12 @@ export class EntityValidator {
 
   constructor(private strict: boolean) { }
 
+  static validateSingleDecorator(meta: EntityMetadata, propertyName: string): void {
+    if (meta.properties[propertyName] && meta.properties[propertyName].reference) {
+      throw ValidationError.multipleDecorators(meta.className, propertyName);
+    }
+  }
+
   validate<T extends AnyEntity<T>>(entity: T, payload: any, meta: EntityMetadata): void {
     Object.values(meta.properties).forEach(prop => {
       if ([ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference)) {

diff --git a/lib/metadata/MetadataStorage.ts b/lib/metadata/MetadataStorage.ts
@@ -16,7 +16,7 @@ export class MetadataStorage {
   static getMetadata<T extends AnyEntity<T> = any>(entity: string): EntityMetadata<T>; // tslint:disable-next-line:lines-between-class-members
   static getMetadata<T extends AnyEntity<T> = any>(entity?: string): Record<string, EntityMetadata> | EntityMetadata<T> {
     if (entity && !MetadataStorage.metadata[entity]) {
-      MetadataStorage.metadata[entity] = { properties: {}, hooks: {}, indexes: [] as any[], uniques: [] as any[] } as EntityMetadata;
+      MetadataStorage.metadata[entity] = { className: entity, properties: {}, hooks: {}, indexes: [] as any[], uniques: [] as any[] } as EntityMetadata;
     }
 
     if (entity) {

diff --git a/lib/utils/ValidationError.ts b/lib/utils/ValidationError.ts
@@ -136,6 +136,10 @@ export class ValidationError<T extends AnyEntity = AnyEntity> extends Error {
     return new ValidationError(`Entity '${entityName}' does not have property '${invalid}'`);
   }
 
+  static multipleDecorators(entityName: string, propertyName: string): ValidationError {
+    return new ValidationError(`Multiple property decorators used on '${entityName}.${propertyName}' property`);
+  }
+
   static invalidType(type: Constructor<Type>, value: any, mode: string): ValidationError {
     const valueType = Utils.getObjectType(value);
 

diff --git a/tests/MikroORM.test.ts b/tests/MikroORM.test.ts
@@ -1,6 +1,7 @@
 (global as any).process.env.FORCE_COLOR = 0;
 
 import { MikroORM, EntityManager, Configuration, ReflectMetadataProvider } from '../lib';
+import { MetadataStorage } from '../lib/metadata';
 import { Author, Test } from './entities';
 import { BASE_DIR } from './bootstrap';
 import { FooBaz2 } from './entities-sql';
@@ -10,6 +11,11 @@ describe('MikroORM', () => {
 
   jest.setTimeout(10e3);
 
+  beforeEach(() => {
+    const meta = MetadataStorage.getMetadata();
+    // Object.keys(meta).forEach(k => delete meta[k]);
+  });
+
   test('should throw when not enough config provided', async () => {
     expect(() => new MikroORM({ entitiesDirs: ['entities'], dbName: '' })).toThrowError('No database specified, please fill in `dbName` option');
     expect(() => new MikroORM({ entities: [], entitiesDirs: [], dbName: 'test' })).toThrowError('No entities found, please use `entities` or `entitiesDirs` option');
@@ -64,6 +70,11 @@ describe('MikroORM', () => {
     await expect(MikroORM.init({ dbName: 'test', baseDir: BASE_DIR, cache: { enabled: false }, entities: [BaseEntity2], entitiesDirsTs: ['entities-sql'] })).rejects.toThrowError(err);
   });
 
+  test('should throw when only multiple property decorators are used', async () => {
+    const err = `Multiple property decorators used on 'MultiDecorator.name' property`;
+    await expect(MikroORM.init({ dbName: 'test', baseDir: BASE_DIR, cache: { enabled: false }, entitiesDirs: ['entities-4'] })).rejects.toThrowError(err);
+  });
+
   test('should use CLI config', async () => {
     const options = {
       entities: [Test],

diff --git a/tests/decorators.test.ts b/tests/decorators.test.ts
@@ -35,10 +35,6 @@ describe('decorators', () => {
     OneToOne({ entity: () => Test, inversedBy: 'test5' } as any)(new Test6(), 'test1');
     expect(storage.Test6.properties.test1).toMatchObject({ reference: ReferenceType.ONE_TO_ONE, name: 'test1', inversedBy: 'test5' });
     expect(storage.Test6.properties.test1.entity()).toBe(Test);
-
-    OneToOne({ entity: () => Test, mappedBy: 'test5', owner: true } as any)(new Test6(), 'test1');
-    expect(storage.Test6.properties.test1).toMatchObject({ reference: ReferenceType.ONE_TO_ONE, name: 'test1', mappedBy: 'test5', owner: true });
-    expect(storage.Test6.properties.test1.entity()).toBe(Test);
   });
 
   test('OneToMany', () => {

diff --git a/tests/entities-1/dup1.model.ts b/tests/entities-1/dup1.model.ts
@@ -12,6 +12,6 @@ export class Dup1 {
   id!: string;
 
   @Property({ type: 'string' })
-  name?: string;
+  name1?: string;
 
 }
diff --git a/tests/entities-1/dup2.model.ts b/tests/entities-1/dup2.model.ts
@@ -13,6 +13,6 @@ export class Dup2 {
   id!: string;
 
   @OneToOne({ type: 'Dup1', owner: true })
-  dup1?: Dup1;
+  dup11?: Dup1;
 
 }
diff --git a/tests/entities-2/dup1.model.ts b/tests/entities-2/dup1.model.ts
@@ -12,6 +12,6 @@ export class Dup1 {
   id!: string;
 
   @Property({ type: 'string' })
-  name?: string;
+  name2?: string;
 
 }
diff --git a/tests/entities-2/dup2.model.ts b/tests/entities-2/dup2.model.ts
@@ -13,6 +13,6 @@ export class Dup2 {
   id!: string;
 
   @OneToOne({ type: 'Dup1', owner: true })
-  dup1?: Dup1;
+  dup12?: Dup1;
 
 }
diff --git a/tests/entities-3/bad-name.model.ts b/tests/entities-3/bad-name.model.ts
@@ -2,7 +2,7 @@ import { Entity, PrimaryKey, Property } from '../../lib';
 import { SerializedPrimaryKey } from '../../lib/decorators';
 
 @Entity()
-export class Test {
+export class BadNameTest {
 
   @PrimaryKey({ type: 'ObjectId' })
   _id: any;

diff --git a/tests/entities-4/multi-decorator.model.ts b/tests/entities-4/multi-decorator.model.ts
@@ -0,0 +1,13 @@
+import { Entity, ManyToOne, PrimaryKey, Property } from '../../lib';
+
+@Entity()
+export class MultiDecorator {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property({ type: 'string' })
+  @ManyToOne({ type: 'Foo' })
+  name: any;
+
+}