feat(core): automatically fix wrongly assigned associations (#62)

When you use `Object.assign(entity, { ... })` instead of `entity.assign({ ... })` you could put your entity
into invalid state where there will be PK instead of entity reference. With this change all managed entities
will be validated before flushing and this case will be automatically handled, converting the PK to entity
when persisting it.

This is especially handy when you migrate existing project where you used `Object.assign(entity, { ... })`
to update entities (like in TypeORM) as this kind of error could be hard to find because they were failing
silently.

lib/drivers/IDatabaseDriver.ts

@@ -1,6 +1,6 @@
 import { EntityData, EntityMetadata, EntityProperty, IEntity, IEntityType, IPrimaryKey } from '../decorators';
 import { Connection, QueryResult } from '../connections';
-import { QueryOrder } from '../query';
+import { QueryOrderMap } from '../query';
 import { Platform } from '../platforms';
 import { LockMode } from '../unit-of-work';
 

lib/entity/ArrayCollection.ts

@@ -1,4 +1,4 @@
-import { EntityProperty, IEntity, IEntityType, IPrimaryKey } from '../decorators';
+import { EntityProperty, IEntityType, IPrimaryKey } from '../decorators';
 import { MetadataStorage } from '../metadata';
 
 export class ArrayCollection<T extends IEntityType<T>> {
@@ -76,9 +76,9 @@ export class ArrayCollection<T extends IEntityType<T>> {
   contains(item: T): boolean {
     return !!this.items.find(i => {
       const objectIdentity = i === item;
-      const primaryKeyIdentity = i.__primaryKey && item.__primaryKey && i.__serializedPrimaryKey === item.__serializedPrimaryKey;
+      const primaryKeyIdentity = !!i.__primaryKey && !!item.__primaryKey && i.__serializedPrimaryKey === item.__serializedPrimaryKey;
 
-      return !!(objectIdentity || primaryKeyIdentity);
+      return objectIdentity || primaryKeyIdentity;
     });
   }
 

lib/entity/Collection.ts

@@ -1,4 +1,4 @@
-import { FilterQuery, QueryOrder, QueryOrderMap } from '..';
+import { FilterQuery, QueryOrder, QueryOrderMap, Utils } from '..';
 import { IEntityType } from '../decorators';
 import { ArrayCollection } from './ArrayCollection';
 import { ReferenceType } from './enums';
@@ -32,7 +32,7 @@ export class Collection<T extends IEntityType<T>> extends ArrayCollection<T> {
       this.initialized = true;
     }
 
-    super.set(items);
+    super.set(items.map(item => Utils.isEntity(item) ? item : this.owner.__em.getReference(this.property.type, item)));
     this.dirty = !initialize;
 
     for (const item of items) {

lib/unit-of-work/UnitOfWork.ts

@@ -183,7 +183,7 @@ export class UnitOfWork {
     }
 
     for (const prop of Object.values(meta.properties)) {
-      const reference = entity[prop.name as keyof T];
+      const reference = entity[prop.name];
       this.processReference(entity, prop, reference, visited);
     }
 
@@ -211,7 +211,7 @@ export class UnitOfWork {
   private processToOneReference<T extends IEntityType<T>>(parent: T, prop: EntityProperty<T>, reference: any, visited: IEntity[]): void {
     if (!this.hasIdentifier(reference) && visited.includes(reference)) {
       this.extraUpdates.push([parent, prop.name as keyof IEntity, reference]);
-      delete parent[prop.name as keyof T];
+      delete parent[prop.name];
     }
 
     if (!this.originalEntityData[reference.__uuid]) {
@@ -308,16 +308,18 @@ export class UnitOfWork {
     }
   }
 
-  private cascadeReference<T extends IEntityType<T>>(entity: T, prop: EntityProperty, type: Cascade, visited: IEntity[]): void {
+  private cascadeReference<T extends IEntityType<T>>(entity: T, prop: EntityProperty<T>, type: Cascade, visited: IEntity[]): void {
+    this.fixMissingReference(entity, prop);
+
     if (!this.shouldCascade(prop, type)) {
       return;
     }
 
-    if ((prop.reference === ReferenceType.MANY_TO_ONE || prop.reference === ReferenceType.ONE_TO_ONE) && entity[prop.name as keyof T]) {
-      return this.cascade(entity[prop.name as keyof T], type, visited);
+    if ((prop.reference === ReferenceType.MANY_TO_ONE || prop.reference === ReferenceType.ONE_TO_ONE) && entity[prop.name]) {
+      return this.cascade(entity[prop.name], type, visited);
     }
 
-    const collection = entity[prop.name as keyof T] as Collection<IEntity>;
+    const collection = entity[prop.name] as Collection<IEntity>;
     const requireFullyInitialized = type === Cascade.PERSIST; // only cascade persist needs fully initialized items
 
     if ([ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference) && collection.isInitialized(requireFullyInitialized)) {
@@ -365,5 +367,21 @@ export class UnitOfWork {
     }
   }
 
+  private fixMissingReference<T extends IEntityType<T>>(entity: T, prop: EntityProperty<T>): void {
+    const reference = entity[prop.name] as IEntity | Collection<IEntity> | IPrimaryKey | (IEntity | IPrimaryKey)[];
+
+    if ([ReferenceType.MANY_TO_ONE, ReferenceType.ONE_TO_ONE].includes(prop.reference) && reference && !Utils.isEntity(reference)) {
+      entity[prop.name] = this.em.getReference(prop.type, reference as IPrimaryKey);
+    }
+
+    const isCollection = [ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference);
+
+    if (isCollection && Array.isArray(reference)) {
+      entity[prop.name] = new Collection<IEntity>(entity) as T[keyof T];
+      (entity[prop.name] as Collection<IEntity>).set(reference as IEntity[]);
+      (entity[prop.name] as Collection<IEntity>).setDirty();
+    }
+  }
+
 }
 

tests/EntityManager.mongo.test.ts

@@ -1260,6 +1260,55 @@ describe('EntityManagerMongo', () => {
     expect(a.born).toBeUndefined();
   });
 
+  test('automatically fix PK instead of entity when flushing (m:1)', async () => {
+    const author = new Author('Jon Snow', 'snow@wall.st');
+    Object.assign(author, { favouriteBook: '0000007b5c9c61c332380f78' });
+    expect(author.favouriteBook).not.toBeInstanceOf(Book);
+    expect(author.favouriteBook).toBe('0000007b5c9c61c332380f78');
+    await orm.em.persist(author);
+    expect(author.favouriteBook).toBeInstanceOf(Book);
+    expect(author.favouriteBook.id).toBe('0000007b5c9c61c332380f78');
+  });
+
+  test('automatically fix array of PKs instead of collection when flushing (m:n)', async () => {
+    const mock = jest.fn();
+    const logger = new Logger(mock, true);
+    Object.assign(orm.em.getConnection(), { logger });
+
+    const author = new Author('Jon Snow', 'snow@wall.st');
+    const book = new Book('B123', author);
+    await orm.em.persistAndFlush(book);
+
+    const tag = orm.em.getReference(BookTag, '0000007b5c9c61c332380f79')
+    Object.assign(book, { tags: ['0000007b5c9c61c332380f78', tag] });
+    expect(book.tags).not.toBeInstanceOf(Collection);
+    expect(book.tags).toEqual(['0000007b5c9c61c332380f78', tag]);
+    await orm.em.persistLater(book);
+    expect(book.tags).toBeInstanceOf(Collection);
+    expect(book.tags[0]).toBeInstanceOf(BookTag);
+    expect(book.tags[1]).toBeInstanceOf(BookTag);
+    expect(book.tags[0].id).toBe('0000007b5c9c61c332380f78');
+    expect(book.tags[1].id).toBe('0000007b5c9c61c332380f79');
+    expect(book.tags.isInitialized()).toBe(true);
+    expect(book.tags.isDirty()).toBe(true);
+
+    await orm.em.flush();
+    expect(mock.mock.calls[0][0]).toMatch(/db\.getCollection\("author"\)\.insertOne\({"createdAt":".*","updatedAt":".*","termsAccepted":false,"name":"Jon Snow","email":"snow@wall\.st","foo":"bar","_id":".*"}\);/);
+    expect(mock.mock.calls[1][0]).toMatch(/db\.getCollection\("books-table"\)\.insertOne\({"title":"B123","author":".*","_id":".*"}\);/);
+    expect(mock.mock.calls[2][0]).toMatch(/db\.getCollection\("books-table"\)\.updateMany\({"_id":".*"}, {"\$set":{"tags":\["0000007b5c9c61c332380f78","0000007b5c9c61c332380f79"]}}\);/);
+  });
+
+  test('automatically fix PK in collection instead of entity when flushing (m:n)', async () => {
+    const author = new Author('Jon Snow', 'snow@wall.st');
+    const book = new Book('B123', author);
+    book.tags.set(['0000007b5c9c61c332380f78' as any]);
+    expect(book.tags).not.toBeInstanceOf(BookTag);
+    expect(book.tags[0]).toBeInstanceOf(BookTag);
+    expect(book.tags[0].id).toBe('0000007b5c9c61c332380f78');
+    expect(book.tags.isInitialized()).toBe(true);
+    expect(book.tags.isDirty()).toBe(true);
+  });
+
   afterAll(async () => orm.close(true));
 
 });