fix(core): fix em.create() with deeply nested data (#683)

Previously `em.create()` did not handle collection updates correctly, basically ignoring all the
values if the property was not M:N owning side.

With this fix, both `em.create()` and `wrap(e).assign()` properly handled collection updates,
including propagation to the owning sides.

Closes #678

packages/core/src/EntityManager.ts

@@ -386,6 +386,13 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
     return this.getEntityFactory().create(entityName, data, true, true);
   }
 
+  /**
+   * Shortcut for `wrap(entity).assign(data, { em })`
+   */
+  assign<T>(entity: T, data: EntityData<T>): T {
+    return EntityAssigner.assign(entity, data, { em: this });
+  }
+
   /**
    * Gets a reference to the entity identified by the given type and identifier without actually loading it, if the entity is not yet loaded
    */

packages/core/src/entity/ArrayCollection.ts

@@ -70,8 +70,7 @@ export class ArrayCollection<T extends AnyEntity<T>, O extends AnyEntity<O>> {
    */
   hydrate(items: T[]): void {
     this.items.length = 0;
-    this.items.push(...items);
-    Object.assign(this, this.items);
+    this.add(...items);
   }
 
   remove(...items: (T | Reference<T>)[]): void {
@@ -158,7 +157,7 @@ export class ArrayCollection<T extends AnyEntity<T>, O extends AnyEntity<O>> {
   }
 
   protected shouldPropagateToCollection(collection: Collection<O, T>, method: 'add' | 'remove'): boolean {
-    if (!collection) {
+    if (!collection || !collection.isInitialized()) {
       return false;
     }
 

packages/core/src/entity/Collection.ts

@@ -20,6 +20,20 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
     Object.defineProperty(this, '_populated', { enumerable: false });
   }
 
+  /**
+   * Creates new Collection instance, assigns it to the owning entity and sets the items to it (propagating them to their inverse sides)
+   */
+  static create<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEntity>(owner: O, prop: keyof O, items: undefined | T[], initialized: boolean): Collection<T, O> {
+    const coll = new Collection<T, O>(owner, items, initialized);
+    owner[prop] = coll as unknown as O[keyof O];
+
+    if (items) {
+      coll.set(items);
+    }
+
+    return coll;
+  }
+
   /**
    * Initializes the collection and returns the items
    */
@@ -67,8 +81,10 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
     }
 
     const wasInitialized = this.initialized;
+    const wasDirty = this.dirty;
     this.initialized = true;
     super.hydrate(items);
+    this.dirty = wasDirty;
 
     if (!wasInitialized) {
       this.snapshot = undefined;

packages/core/src/entity/EntityRepository.ts

@@ -115,6 +115,13 @@ export class EntityRepository<T> {
     return this.em.create<T>(this.entityName, data);
   }
 
+  /**
+   * Shortcut for `wrap(entity).assign(data, { em })`
+   */
+  assign(entity: T, data: EntityData<T>): T {
+    return this.em.assign(entity, data);
+  }
+
   async count(where: FilterQuery<T> = {}): Promise<number> {
     return this.em.count<T>(this.entityName, where);
   }

packages/core/src/entity/Reference.ts

@@ -1,12 +1,10 @@
-import { AnyEntity, Dictionary, EntityProperty, IWrappedEntityInternal, Primary } from '../typings';
+import { AnyEntity, Dictionary, EntityProperty, Primary } from '../typings';
 import { wrap } from './wrap';
 
 export type IdentifiedReference<T, PK extends keyof T = 'id' & keyof T> = { [K in PK]: T[K] } & Reference<T>;
 
 export class Reference<T> {
 
-  private __helper?: IWrappedEntityInternal<T, keyof T>;
-
   constructor(private entity: T) {
     this.set(entity);
     const wrapped = wrap(this.entity, true);
@@ -81,7 +79,7 @@ export class Reference<T> {
     }
 
     this.entity = entity;
-    this.__helper = wrap(this.entity, true);
+    Object.defineProperty(this, '__helper', { value: wrap(this.entity, true), writable: true });
   }
 
   unwrap(): T {

packages/core/src/hydration/ObjectHydrator.ts

@@ -7,22 +7,18 @@ export class ObjectHydrator extends Hydrator {
 
   protected hydrateProperty<T>(entity: T, prop: EntityProperty, data: EntityData<T>, newEntity: boolean): void {
     if (prop.reference === ReferenceType.MANY_TO_ONE || prop.reference === ReferenceType.ONE_TO_ONE) {
-      this.hydrateManyToOne(data[prop.name], entity, prop);
+      this.hydrateToOne(data[prop.name], entity, prop);
     } else if (prop.reference === ReferenceType.ONE_TO_MANY) {
-      this.hydrateOneToMany(entity, prop, data[prop.name], newEntity);
+      this.hydrateToMany(entity, prop, data[prop.name], newEntity);
     } else if (prop.reference === ReferenceType.MANY_TO_MANY) {
-      this.hydrateManyToMany(entity, prop, data[prop.name], newEntity);
+      this.hydrateToMany(entity, prop, data[prop.name], newEntity);
     } else if (prop.reference === ReferenceType.EMBEDDED) {
       this.hydrateEmbeddable(entity, prop, data);
     } else { // ReferenceType.SCALAR
       this.hydrateScalar(entity, prop, data[prop.name]);
     }
   }
 
-  private hydrateOneToMany<T>(entity: T, prop: EntityProperty<T>, value: any, newEntity: boolean): void {
-    entity[prop.name as keyof T] = new Collection<AnyEntity, T>(entity, undefined, !!value || newEntity) as unknown as T[keyof T];
-  }
-
   private hydrateScalar<T>(entity: T, prop: EntityProperty, value: any): void {
     if (typeof value === 'undefined' || (prop.getter && !prop.setter)) {
       return;
@@ -32,7 +28,7 @@ export class ObjectHydrator extends Hydrator {
       value = prop.customType.convertToJSValue(value, this.em.getDriver().getPlatform());
     }
 
-    entity[prop.name as keyof T] = value;
+    entity[prop.name] = value;
   }
 
   private hydrateEmbeddable<T>(entity: T, prop: EntityProperty, data: EntityData<T>): void {
@@ -46,41 +42,27 @@ export class ObjectHydrator extends Hydrator {
     Object.keys(value).forEach(k => entity[prop.name][k] = value[k]);
   }
 
-  private hydrateManyToMany<T>(entity: T, prop: EntityProperty, value: any, newEntity: boolean): void {
-    if (prop.owner) {
-      return this.hydrateManyToManyOwner(entity, prop, value, newEntity);
-    }
-
-    this.hydrateManyToManyInverse(entity, prop, newEntity);
-  }
-
-  private hydrateManyToManyOwner<T>(entity: T, prop: EntityProperty, value: any, newEntity: boolean): void {
+  private hydrateToMany<T>(entity: T, prop: EntityProperty, value: any, newEntity: boolean): void {
     if (Array.isArray(value)) {
       const items = value.map((value: Primary<T> | EntityData<T>) => this.createCollectionItem(prop, value));
-      const coll = new Collection<AnyEntity>(entity, items);
-      entity[prop.name as keyof T] = coll as unknown as T[keyof T];
+      const coll = Collection.create<AnyEntity>(entity, prop.name, items, newEntity);
       coll.setDirty();
-    } else if (!entity[prop.name as keyof T]) {
-      const items = this.em.getDriver().getPlatform().usesPivotTable() ? undefined : [];
-      entity[prop.name as keyof T] = new Collection<AnyEntity>(entity, items, newEntity) as unknown as T[keyof T];
-    }
-  }
-
-  private hydrateManyToManyInverse<T>(entity: T, prop: EntityProperty, newEntity: boolean): void {
-    if (!entity[prop.name as keyof T]) {
-      entity[prop.name as keyof T] = new Collection<AnyEntity>(entity, undefined, newEntity) as unknown as T[keyof T];
+    } else if (!entity[prop.name]) {
+      const items = this.em.getDriver().getPlatform().usesPivotTable() || !prop.owner ? undefined : [];
+      const coll = Collection.create<AnyEntity>(entity, prop.name, items, !!value || newEntity);
+      coll.setDirty(false);
     }
   }
 
-  private hydrateManyToOne<T>(value: any, entity: T, prop: EntityProperty): void {
+  private hydrateToOne<T>(value: any, entity: T, prop: EntityProperty): void {
     if (typeof value === 'undefined') {
       return;
     }
 
     if (Utils.isPrimaryKey<T[keyof T]>(value)) {
-      entity[prop.name as keyof T] = Reference.wrapReference(this.factory.createReference<T[keyof T]>(prop.type, value), prop) as T[keyof T];
+      entity[prop.name] = Reference.wrapReference(this.factory.createReference<T[keyof T]>(prop.type, value), prop) as T[keyof T];
     } else if (Utils.isObject<EntityData<T[keyof T]>>(value)) {
-      entity[prop.name as keyof T] = Reference.wrapReference(this.factory.create(prop.type, value), prop) as T[keyof T];
+      entity[prop.name] = Reference.wrapReference(this.factory.create(prop.type, value), prop) as T[keyof T];
     }
 
     if (entity[prop.name]) {

tests/EntityFactory.test.ts

@@ -1,5 +1,5 @@
 import { ObjectId } from 'mongodb';
-import { MikroORM, Collection, EntityFactory, MetadataDiscovery, ReferenceType, wrap } from '@mikro-orm/core';
+import { MikroORM, Collection, EntityFactory, MetadataDiscovery, ReferenceType, wrap, Logger } from '@mikro-orm/core';
 import { Book, Author, Publisher, Test, BookTag } from './entities';
 import { initORMMongo, wipeDatabase } from './bootstrap';
 import { AuthorRepository } from './repositories/AuthorRepository';
@@ -166,6 +166,73 @@ describe('EntityFactory', () => {
     expect(b.tags.isDirty()).toBe(true);
   });
 
+  test('create entity from nested object', async () => {
+    const repo = orm.em.getRepository(Author);
+    const a1 = repo.create({ name: 'Jon', email: 'jon@snow.com', books: [
+      { title: 'B1', publisher: '5b0d19b28b21c648c2c8a600', tags: [{ name: 't1' }, '5b0d19b28b21c648c2c8a601'] },
+    ] });
+
+    expect(a1.name).toBe('Jon');
+    expect(a1.email).toBe('jon@snow.com');
+    expect(a1.books.isInitialized()).toBe(true);
+    expect(a1.books.isDirty()).toBe(false); // inverse side
+    expect(a1.books[0].title).toBe('B1');
+    expect(a1.books[0].author).toBe(a1); // propagation to owning side
+    expect(a1.books[0].publisher.id).toBe('5b0d19b28b21c648c2c8a600');
+    expect(wrap(a1.books[0].publisher).isInitialized()).toBe(false);
+    expect(a1.books[0].tags.isInitialized()).toBe(true);
+    expect(a1.books[0].tags.isDirty()).toBe(true); // owning side
+    expect(a1.books[0].tags[0].name).toBe('t1');
+    expect(a1.books[0].tags[0].id).toBe(null);
+    expect(a1.books[0].tags[1].id).toBe('5b0d19b28b21c648c2c8a601');
+
+    const mock = jest.fn();
+    const logger = new Logger(mock, true);
+    Object.assign(orm.config, { logger });
+
+    await orm.em.persistAndFlush(a1);
+
+    expect(mock.mock.calls.length).toBe(6);
+    expect(mock.mock.calls[0][0]).toMatch('db.begin()');
+    expect(mock.mock.calls[1][0]).toMatch(/db\.getCollection\('book-tag'\)\.insertOne\({ name: 't1' }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[2][0]).toMatch(/db\.getCollection\('author'\)\.insertOne\({ createdAt: ISODate\('.*'\), updatedAt: ISODate\('.*'\), foo: 'bar', name: 'Jon', email: 'jon@snow\.com', termsAccepted: false }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[3][0]).toMatch(/db\.getCollection\('books-table'\)\.insertOne\({ createdAt: ISODate\('.*'\), title: 'B1', publisher: ObjectId\('5b0d19b28b21c648c2c8a600'\), author: ObjectId\('.*'\) }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[4][0]).toMatch(/db\.getCollection\('books-table'\)\.updateMany\({ _id: ObjectId\('.*'\) }, { '\$set': { tags: \[ ObjectId\('.*'\), ObjectId\('5b0d19b28b21c648c2c8a601'\) ] } }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[5][0]).toMatch('db.commit()');
+
+    orm.em.clear();
+    mock.mock.calls.length = 0;
+
+    const a2 = repo.create({});
+    repo.assign(a2, { name: 'Jon', email: 'jon2@snow.com', books: [
+      { title: 'B1', publisher: '5b0d19b28b21c648c2c8a600', tags: [{ name: 't1' }, '5b0d19b28b21c648c2c8a601'] },
+    ] });
+
+    expect(a2.name).toBe('Jon');
+    expect(a2.email).toBe('jon2@snow.com');
+    expect(a2.books.isInitialized()).toBe(true);
+    expect(a2.books.isDirty()).toBe(false); // inverse side
+    expect(a2.books[0].title).toBe('B1');
+    expect(a2.books[0].author).toBe(a2); // propagation to owning side
+    expect(a2.books[0].publisher.id).toBe('5b0d19b28b21c648c2c8a600');
+    expect(wrap(a2.books[0].publisher).isInitialized()).toBe(false);
+    expect(a2.books[0].tags.isInitialized()).toBe(true);
+    expect(a2.books[0].tags.isDirty()).toBe(true); // owning side
+    expect(a2.books[0].tags[0].name).toBe('t1');
+    expect(a2.books[0].tags[0].id).toBe(null);
+    expect(a2.books[0].tags[1].id).toBe('5b0d19b28b21c648c2c8a601');
+
+    await orm.em.persistAndFlush(a2);
+
+    expect(mock.mock.calls.length).toBe(6);
+    expect(mock.mock.calls[0][0]).toMatch('db.begin()');
+    expect(mock.mock.calls[1][0]).toMatch(/db\.getCollection\('book-tag'\)\.insertOne\({ name: 't1' }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[2][0]).toMatch(/db\.getCollection\('author'\)\.insertOne\({ createdAt: ISODate\('.*'\), updatedAt: ISODate\('.*'\), foo: 'bar', name: 'Jon', email: 'jon2@snow\.com', termsAccepted: false }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[3][0]).toMatch(/db\.getCollection\('books-table'\)\.insertOne\({ createdAt: ISODate\('.*'\), title: 'B1', publisher: ObjectId\('5b0d19b28b21c648c2c8a600'\), author: ObjectId\('.*'\) }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[4][0]).toMatch(/db\.getCollection\('books-table'\)\.updateMany\({ _id: ObjectId\('.*'\) }, { '\$set': { tags: \[ ObjectId\('.*'\), ObjectId\('5b0d19b28b21c648c2c8a601'\) ] } }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[5][0]).toMatch('db.commit()');
+  });
+
   afterAll(async () => orm.close(true));
 
 });