fix(mongo): do not use separate update queries for M:N collections if…

… not needed

Closes #2483

packages/core/src/unit-of-work/ChangeSetComputer.ts

@@ -145,7 +145,11 @@ export class ChangeSetComputer {
     }
 
     if (prop.owner || target.getItems(false).filter(item => !item.__helper!.__initialized).length > 0) {
-      this.collectionUpdates.add(target);
+      if (this.platform.usesPivotTable()) {
+        this.collectionUpdates.add(target);
+      } else {
+        changeSet.payload[prop.name] = target.getItems(false).map((item: AnyEntity) => item.__helper!.__identifier ?? item.__helper!.getPrimaryKey());
+      }
     } else {
       target.setDirty(false); // inverse side with only populated items, nothing to persist
     }

packages/core/src/unit-of-work/ChangeSetPersister.ts

@@ -8,6 +8,7 @@ import type { Configuration } from '../utils';
 import { Utils } from '../utils';
 import type { DriverMethodOptions, IDatabaseDriver } from '../drivers';
 import { OptimisticLockError } from '../errors';
+import { ReferenceType } from '../enums';
 
 export class ChangeSetPersister {
 
@@ -339,6 +340,10 @@ export class ChangeSetPersister {
     const values = Utils.unwrapProperty(changeSet.payload, meta, prop, true); // for object embeddables
     const value = changeSet.payload[prop.name] as unknown; // for inline embeddables
 
+    if (prop.reference === ReferenceType.MANY_TO_MANY && Array.isArray(value)) {
+      changeSet.payload[prop.name] = value.map(val => val instanceof EntityIdentifier ? val.getValue() : val);
+    }
+
     if (value instanceof EntityIdentifier) {
       Utils.setPayloadProperty<T>(changeSet.payload, meta, prop, value.getValue());
     }

packages/core/src/unit-of-work/CommitOrderCalculator.ts

@@ -59,7 +59,10 @@ export class CommitOrderCalculator {
   }
 
   discoverProperty(prop: EntityProperty, entityName: string): void {
-    if (!(prop.reference === ReferenceType.ONE_TO_ONE && prop.owner) && prop.reference !== ReferenceType.MANY_TO_ONE) {
+    const toOneOwner = (prop.reference === ReferenceType.ONE_TO_ONE && prop.owner) || prop.reference === ReferenceType.MANY_TO_ONE;
+    const toManyOwner = prop.reference === ReferenceType.MANY_TO_MANY && prop.owner && !prop.pivotTable;
+
+    if (!toOneOwner && !toManyOwner) {
       return;
     }
 

tests/EntityFactory.test.ts

@@ -217,13 +217,12 @@ describe('EntityFactory', () => {
 
     await orm.em.persistAndFlush(a1);
 
-    expect(mock.mock.calls).toHaveLength(6);
+    expect(mock.mock.calls).toHaveLength(5);
     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()');
+    expect(mock.mock.calls[3][0]).toMatch(/db\.getCollection\('books-table'\)\.insertOne\({ createdAt: ISODate\('.*'\), title: 'B1', publisher: ObjectId\('5b0d19b28b21c648c2c8a600'\), author: ObjectId\('.*'\), tags: \[ ObjectId\('.*'\), ObjectId\('5b0d19b28b21c648c2c8a601'\) ] }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[4][0]).toMatch('db.commit()');
 
     orm.em.clear();
     mock.mock.calls.length = 0;
@@ -249,13 +248,12 @@ describe('EntityFactory', () => {
 
     await orm.em.persistAndFlush(a2);
 
-    expect(mock.mock.calls.length).toBe(6);
+    expect(mock.mock.calls.length).toBe(5);
     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()');
+    expect(mock.mock.calls[3][0]).toMatch(/db\.getCollection\('books-table'\)\.insertOne\({ createdAt: ISODate\('.*'\), title: 'B1', publisher: ObjectId\('5b0d19b28b21c648c2c8a600'\), author: ObjectId\('.*'\), tags: \[ ObjectId\('.*'\), ObjectId\('5b0d19b28b21c648c2c8a601'\) ] }, { session: '\[ClientSession]' }\);/);
+    expect(mock.mock.calls[4][0]).toMatch('db.commit()');
   });
 
   test('em.create() should not mutate the input object (GH issue 1294)', async () => {

tests/EntityManager.mongo.test.ts

@@ -515,9 +515,7 @@ describe('EntityManagerMongo', () => {
     expect(driver.getConnection().getCollection(BookTag).collectionName).toBe('book-tag');
     expect(orm.em.getCollection(BookTag).collectionName).toBe('book-tag');
 
-    expect(() => {
-      driver.getPlatform().generateCustomOrder('foo', [1, 2, 3]);
-    }).toThrow();
+    expect(() => driver.getPlatform().generateCustomOrder('foo', [1, 2, 3])).toThrow();
 
     const conn = driver.getConnection();
     const ctx = await conn.begin();
@@ -685,6 +683,8 @@ describe('EntityManagerMongo', () => {
   });
 
   test('many to many relation', async () => {
+    const mock = mockLogger(orm);
+
     const author = new Author('Jon Snow', 'snow@wall.st');
     const book1 = new Book('My Life on The Wall, part 1', author);
     const book2 = new Book('My Life on The Wall, part 2', author);
@@ -710,9 +710,19 @@ describe('EntityManagerMongo', () => {
     expect(book1.tags.toArray()).toEqual([wrap(tag1).toJSON(), wrap(tag3).toJSON()]);
     expect(book1.tags.toJSON()).toEqual([wrap(tag1).toJSON(), wrap(tag3).toJSON()]);
 
+    // ensure we don't have separate update queries for collection sync
+    expect(mock.mock.calls).toHaveLength(5);
+    expect(mock.mock.calls[1][0]).toMatch(`db.getCollection('book-tag').insertMany(`);
+    expect(mock.mock.calls[2][0]).toMatch(`db.getCollection('author').insertOne(`);
+    expect(mock.mock.calls[3][0]).toMatch(`db.getCollection('books-table').insertMany(`);
+    orm.em.clear();
+
+    // just to raise coverage, that method is no longer used internally
+    await orm.em.getDriver().syncCollection(book1.tags);
+
     // test inverse side
     const tagRepository = orm.em.getRepository(BookTag);
-    let tags = await tagRepository.findAll();
+    let tags = await tagRepository.findAll({ populate: ['books'] });
     expect(tags).toBeInstanceOf(Array);
     expect(tags.length).toBe(5);
     expect(tags[0]).toBeInstanceOf(BookTag);