From db488b4124d7671b99cb222a05e05cb60e0a9bfe Mon Sep 17 00:00:00 2001
From: "Fabien \"egg\" O'Carroll" <fabien@allou.is>
Date: Fri, 22 Sep 2023 15:23:07 +0700
Subject: [PATCH] Optimised the storage of collection posts relations

refs https://github.com/TryGhost/Arch/issues/95

Rather than a big nested loop to reconcile the in-memory vs. persisted
PostCollections we can instead use the events to know which rows we have to
delete and which we have to insert. This removes a tonne of work.

This implementation isn't perfect, and misses cases where the same post is
added and removed, our use-cases don't currently support that however.
---
 .../BookshelfCollectionsRepository.js         | 62 +++++++++----------
 .../test/e2e-api/admin/collections.test.js    |  4 +-
 2 files changed, 30 insertions(+), 36 deletions(-)

diff --git a/ghost/core/core/server/services/collections/BookshelfCollectionsRepository.js b/ghost/core/core/server/services/collections/BookshelfCollectionsRepository.js
index 09054cf95d..452d4e350a 100644
--- a/ghost/core/core/server/services/collections/BookshelfCollectionsRepository.js
+++ b/ghost/core/core/server/services/collections/BookshelfCollectionsRepository.js
@@ -204,45 +204,39 @@ module.exports = class BookshelfCollectionsRepository {
                 transacting: options.transaction
             });
 
-            const collectionPostsRelations = collection.posts.map((postId, index) => {
-                return {
-                    id: (new ObjectID).toHexString(),
-                    sort_order: collection.type === 'manual' ? index : 0,
-                    collection_id: collection.id,
-                    post_id: postId
-                };
-            });
-
-            const collectionPostRelationsToDeleteIds = [];
-
             if (collection.type === 'manual') {
+                const collectionPostsRelations = collection.posts.map((postId, index) => {
+                    return {
+                        id: (new ObjectID).toHexString(),
+                        sort_order: index,
+                        collection_id: collection.id,
+                        post_id: postId
+                    };
+                });
                 await this.#relationModel.query().delete().where('collection_id', collection.id).transacting(options.transaction);
-            } else {
-                const collectionPostsOptions = {
-                    transaction: options.transaction,
-                    columns: ['id', 'post_id']
-                };
-                const existingRelations = await this.#fetchCollectionPostIds(collection.id, collectionPostsOptions);
-
-                for (const existingRelation of existingRelations) {
-                    const found = collectionPostsRelations.find((thing) => {
-                        return thing.post_id === existingRelation.post_id;
-                    });
-                    if (found) {
-                        found.id = null;
-                    } else {
-                        collectionPostRelationsToDeleteIds.push(existingRelation.id);
-                    }
+                if (collectionPostsRelations.length > 0) {
+                    await this.#relationModel.query().insert(collectionPostsRelations).transacting(options.transaction);
                 }
-            }
+            } else {
+                const collectionPostsToDelete = collection.events.filter(event => event.type === 'CollectionPostRemoved').map((event) => {
+                    return event.data.post_id;
+                });
 
-            const missingCollectionPostsRelations = collectionPostsRelations.filter(thing => thing.id !== null);
+                const collectionPostsToInsert = collection.events.filter(event => event.type === 'CollectionPostAdded').map((event) => {
+                    return {
+                        id: (new ObjectID).toHexString(),
+                        sort_order: 0,
+                        collection_id: collection.id,
+                        post_id: event.data.post_id
+                    };
+                });
 
-            if (missingCollectionPostsRelations.length > 0) {
-                await this.#relationModel.query().insert(missingCollectionPostsRelations).transacting(options.transaction);
-            }
-            if (collectionPostRelationsToDeleteIds.length > 0) {
-                await this.#relationModel.query().delete().whereIn('id', collectionPostRelationsToDeleteIds).transacting(options.transaction);
+                if (collectionPostsToDelete.length > 0) {
+                    await this.#relationModel.query().delete().where('collection_id', collection.id).whereIn('post_id', collectionPostsToDelete).transacting(options.transaction);
+                }
+                if (collectionPostsToInsert.length > 0) {
+                    await this.#relationModel.query().insert(collectionPostsToInsert).transacting(options.transaction);
+                }
             }
 
             options.transaction.executionPromise.then(() => {
diff --git a/ghost/core/test/e2e-api/admin/collections.test.js b/ghost/core/test/e2e-api/admin/collections.test.js
index 96d362a171..8486a22509 100644
--- a/ghost/core/test/e2e-api/admin/collections.test.js
+++ b/ghost/core/test/e2e-api/admin/collections.test.js
@@ -572,7 +572,7 @@ describe('Collections API', function () {
                 }, this.skip.bind(this));
 
                 const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection'));
-                assert.equal(collectionRelatedQueries.length, 13);
+                assert.equal(collectionRelatedQueries.length, 12);
             }
 
             await agent
@@ -604,7 +604,7 @@ describe('Collections API', function () {
                 }, this.skip.bind(this));
 
                 const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection'));
-                assert.equal(collectionRelatedQueries.length, 18);
+                assert.equal(collectionRelatedQueries.length, 16);
             }
 
             await agent