From 34582da561945cacc53d0c0664b54e63ebc8da25 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 16 Jan 2023 21:40:34 -0500 Subject: [PATCH] chore: leverages versions refactor to simplify draft query logic --- package.json | 1 + src/collections/init.ts | 5 + src/collections/operations/create.ts | 1 + src/collections/operations/find.ts | 5 +- src/collections/operations/update.ts | 1 + src/globals/operations/update.ts | 1 + src/versions/drafts/mergeDrafts.ts | 229 --------------------------- src/versions/drafts/queryDrafts.ts | 86 ++++++++++ src/versions/saveVersion.ts | 13 +- yarn.lock | 5 + 10 files changed, 112 insertions(+), 235 deletions(-) delete mode 100644 src/versions/drafts/mergeDrafts.ts create mode 100644 src/versions/drafts/queryDrafts.ts diff --git a/package.json b/package.json index c046be66d6..50e204242b 100644 --- a/package.json +++ b/package.json @@ -134,6 +134,7 @@ "minimist": "^1.2.0", "mkdirp": "^1.0.4", "mongoose": "6.5.0", + "mongoose-aggregate-paginate-v2": "^1.0.6", "mongoose-paginate-v2": "^1.6.1", "nodemailer": "^6.4.2", "object-to-formdata": "^4.1.0", diff --git a/src/collections/init.ts b/src/collections/init.ts index 00626d1252..fdd83d5ef3 100644 --- a/src/collections/init.ts +++ b/src/collections/init.ts @@ -3,6 +3,7 @@ import paginate from 'mongoose-paginate-v2'; import express from 'express'; import passport from 'passport'; import passportLocalMongoose from 'passport-local-mongoose'; +import mongooseAggregatePaginate from 'mongoose-aggregate-paginate-v2'; import { buildVersionCollectionFields } from '../versions/buildCollectionFields'; import buildQueryPlugin from '../mongoose/buildQuery'; import apiKeyStrategy from '../auth/strategies/apiKey'; @@ -82,6 +83,10 @@ export default function registerCollections(ctx: Payload): void { versionSchema.plugin(paginate, { useEstimatedCount: true }) .plugin(buildQueryPlugin); + if (collection.versions?.drafts) { + versionSchema.plugin(mongooseAggregatePaginate); + } + ctx.versions[collection.slug] = mongoose.model(versionModelName, versionSchema) as CollectionModel; } diff --git a/src/collections/operations/create.ts b/src/collections/operations/create.ts index 8fea30e3c6..5818f6a95a 100644 --- a/src/collections/operations/create.ts +++ b/src/collections/operations/create.ts @@ -228,6 +228,7 @@ async function create(incomingArgs: Arguments): Promise { id: result.id, docWithLocales: result, autosave, + createdAt: result.createdAt, }); } diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index 46e5677bff..9f195d5539 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -9,7 +9,7 @@ import flattenWhereConstraints from '../../utilities/flattenWhereConstraints'; import { buildSortParam } from '../../mongoose/buildSortParam'; import { AccessResult } from '../../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; -import { mergeDrafts } from '../../versions/drafts/mergeDrafts'; +import { queryDrafts } from '../../versions/drafts/queryDrafts'; export type Arguments = { collection: Collection @@ -160,13 +160,12 @@ async function find(incomingArgs: Arguments): Promis }; if (collectionConfig.versions?.drafts && draftsEnabled) { - result = await mergeDrafts({ + result = await queryDrafts({ accessResult, collection, locale, paginationOptions, payload, - query, where, }); } else { diff --git a/src/collections/operations/update.ts b/src/collections/operations/update.ts index 891e49219e..496a5f7d61 100644 --- a/src/collections/operations/update.ts +++ b/src/collections/operations/update.ts @@ -254,6 +254,7 @@ async function update(incomingArgs: Arguments): Promise { id, autosave, draft: shouldSaveDraft, + createdAt: result.createdAt as string, }); } diff --git a/src/globals/operations/update.ts b/src/globals/operations/update.ts index 11cdb3fc8c..9ba1f4e7c0 100644 --- a/src/globals/operations/update.ts +++ b/src/globals/operations/update.ts @@ -212,6 +212,7 @@ async function update(args: Args): Promise { docWithLocales: result, autosave, draft: shouldSaveDraft, + createdAt: global.createdAt, }); } diff --git a/src/versions/drafts/mergeDrafts.ts b/src/versions/drafts/mergeDrafts.ts deleted file mode 100644 index 2ec040de5d..0000000000 --- a/src/versions/drafts/mergeDrafts.ts +++ /dev/null @@ -1,229 +0,0 @@ -import { AccessResult } from '../../config/types'; -import { Where } from '../../types'; -import { Payload } from '../..'; -import { PaginatedDocs } from '../../mongoose/types'; -import { Collection, CollectionModel, TypeWithID } from '../../collections/config/types'; -import { hasWhereAccessResult } from '../../auth'; -import { appendVersionToQueryKey } from './appendVersionToQueryKey'; -import replaceWithDraftIfAvailable from './replaceWithDraftIfAvailable'; - -type AggregateVersion = { - _id: string - version: T - updatedAt: string - createdAt: string -} - -type VersionCollectionMatchMap = { - [_id: string | number]: { - updatedAt: string - createdAt: string - version: T - } -} - -type Args = { - accessResult: AccessResult - collection: Collection - locale: string - paginationOptions: any - payload: Payload - query: Record - where: Where -} - -export const mergeDrafts = async ({ - accessResult, - collection, - locale, - payload, - paginationOptions, - query, - where: incomingWhere, -}: Args): Promise> => { - // Query the main collection for any IDs that match the query - // Create object "map" for performant lookup - const mainCollectionMatchMap = await collection.Model.find(query, { updatedAt: 1 }, { limit: paginationOptions.limit, sort: paginationOptions.sort }) - .lean().then((res) => res.reduce((map, { _id, updatedAt }) => { - const newMap = map; - newMap[_id] = updatedAt; - return newMap; - }, {})); - - // Query the versions collection with a version-specific query - const VersionModel = payload.versions[collection.config.slug] as CollectionModel; - - const where = appendVersionToQueryKey(incomingWhere || {}); - - const versionQueryToBuild: { where: Where } = { - where: { - ...where, - and: [ - ...where?.and || [], - { - 'version._status': { - equals: 'draft', - }, - }, - ], - }, - }; - - if (hasWhereAccessResult(accessResult)) { - const versionAccessResult = appendVersionToQueryKey(accessResult); - versionQueryToBuild.where.and.push(versionAccessResult); - } - - const versionQuery = await VersionModel.buildQuery(versionQueryToBuild, locale); - const includedParentIDs: (string | number)[] = []; - - // Create version "map" for performant lookup - // and in the same loop, check if there are matched versions without a matched parent - // This means that the newer version's parent should appear in the main query. - // To do so, add the version's parent ID into an explicit `includedIDs` array - const versionCollectionMatchMap = await VersionModel.aggregate>([ - { - $sort: Object.entries(paginationOptions.sort).reduce((sort, [key, order]) => { - return { - ...sort, - [key]: order === 'asc' ? 1 : -1, - }; - }, {}), - }, - { - $group: { - _id: '$parent', - versionID: { $first: '$_id' }, - version: { $first: '$version' }, - updatedAt: { $first: '$updatedAt' }, - createdAt: { $first: '$createdAt' }, - }, - }, - { - $addFields: { - id: { - $toObjectId: '$_id', - }, - }, - }, - { - $lookup: { - from: collection.config.slug, - localField: 'id', - foreignField: '_id', - as: 'parent', - }, - }, - { - $match: { - parent: { - $size: 1, - }, - }, - }, - { $match: versionQuery }, - { $limit: paginationOptions.limit }, - ]).then((res) => res.reduce>((map, { _id, updatedAt, createdAt, version }) => { - const newMap = map; - newMap[_id] = { version, updatedAt, createdAt }; - - const matchedParent = mainCollectionMatchMap[_id]; - if (!matchedParent) includedParentIDs.push(_id); - return newMap; - }, {})); - - // Now we need to explicitly exclude any parent matches that have newer versions - // which did NOT appear in the versions query - const excludedParentIDs = await Promise.all(Object.entries(mainCollectionMatchMap).map(async ([parentDocID, parentDocUpdatedAt]) => { - // If there is a matched version, and it's newer, this parent should remain - if (versionCollectionMatchMap[parentDocID] && versionCollectionMatchMap[parentDocID].updatedAt > parentDocUpdatedAt) { - return null; - } - - // Otherwise, we need to check if there are newer versions present - // that did not get returned from the versions query - const versionsQuery = await VersionModel.find({ - updatedAt: { - $gt: parentDocUpdatedAt, - }, - parent: { - $eq: parentDocID, - }, - }, {}, { limit: 1 }).lean(); - - // If there are, - // this says that the newest version does not match the incoming query, - // and the parent ID should be excluded - if (versionsQuery.length > 0) { - return parentDocID; - } - - return null; - })).then((res) => res.filter((result) => Boolean(result))); - - // Run a final query against the main collection, - // passing in any ids to exclude and include - // so that they appear properly paginated - const finalQueryToBuild: { where: Where } = { - where: { - and: [], - }, - }; - - finalQueryToBuild.where.and.push({ or: [] }); - - if (hasWhereAccessResult(accessResult)) { - finalQueryToBuild.where.and.push(accessResult); - } - - if (incomingWhere) { - finalQueryToBuild.where.and[0].or.push(incomingWhere); - } - - if (includedParentIDs.length > 0) { - finalQueryToBuild.where.and[0].or.push({ - id: { - in: includedParentIDs, - }, - }); - } - - if (excludedParentIDs.length > 0) { - finalQueryToBuild.where.and.push({ - id: { - not_in: excludedParentIDs, - }, - }); - } - - const finalQuery = await collection.Model.buildQuery(finalQueryToBuild, locale); - - let result = await collection.Model.paginate(finalQuery, paginationOptions); - - result = { - ...result, - docs: await Promise.all(result.docs.map(async (doc) => { - const matchedVersion = versionCollectionMatchMap[doc.id]; - - if (matchedVersion && matchedVersion.updatedAt > doc.updatedAt) { - return { - ...doc, - ...matchedVersion.version, - createdAt: matchedVersion.createdAt, - updatedAt: matchedVersion.updatedAt, - }; - } - - return replaceWithDraftIfAvailable({ - accessResult, - payload, - entity: collection.config, - entityType: 'collection', - doc, - locale, - }); - })), - }; - - return result; -}; diff --git a/src/versions/drafts/queryDrafts.ts b/src/versions/drafts/queryDrafts.ts new file mode 100644 index 0000000000..34a9ce1398 --- /dev/null +++ b/src/versions/drafts/queryDrafts.ts @@ -0,0 +1,86 @@ +import { AccessResult } from '../../config/types'; +import { Where } from '../../types'; +import { Payload } from '../..'; +import { PaginatedDocs } from '../../mongoose/types'; +import { Collection, CollectionModel, TypeWithID } from '../../collections/config/types'; +import { hasWhereAccessResult } from '../../auth'; +import { appendVersionToQueryKey } from './appendVersionToQueryKey'; + +type AggregateVersion = { + _id: string + version: T + updatedAt: string + createdAt: string +} + +type Args = { + accessResult: AccessResult + collection: Collection + locale: string + paginationOptions: any + payload: Payload + where: Where +} + +export const queryDrafts = async ({ + accessResult, + collection, + locale, + payload, + paginationOptions, + where: incomingWhere, +}: Args): Promise> => { + const VersionModel = payload.versions[collection.config.slug] as CollectionModel; + + const where = appendVersionToQueryKey(incomingWhere || {}); + + const versionQueryToBuild: { where: Where } = { + where: { + ...where, + and: [ + ...where?.and || [], + ], + }, + }; + + if (hasWhereAccessResult(accessResult)) { + const versionAccessResult = appendVersionToQueryKey(accessResult); + versionQueryToBuild.where.and.push(versionAccessResult); + } + + const versionQuery = await VersionModel.buildQuery(versionQueryToBuild, locale); + + const aggregate = VersionModel.aggregate>([ + { + $sort: Object.entries(paginationOptions.sort).reduce((sort, [key, order]) => { + return { + ...sort, + [key]: order === 'asc' ? 1 : -1, + }; + }, {}), + }, + { + $group: { + _id: '$parent', + versionID: { $first: '$_id' }, + version: { $first: '$version' }, + updatedAt: { $first: '$updatedAt' }, + createdAt: { $first: '$createdAt' }, + }, + }, + { $match: versionQuery }, + { $limit: paginationOptions.limit }, + ]); + + const result = await VersionModel.aggregatePaginate(aggregate, paginationOptions); + + return { + ...result, + docs: result.docs.map((doc) => ({ + _id: doc._id, + ...doc.version, + updatedAt: doc.updatedAt, + createdAt: doc.createdAt, + })), + }; +}; diff --git a/src/versions/saveVersion.ts b/src/versions/saveVersion.ts index 7a706a5608..912daff087 100644 --- a/src/versions/saveVersion.ts +++ b/src/versions/saveVersion.ts @@ -14,6 +14,7 @@ type Args = { id?: string | number autosave?: boolean draft?: boolean + createdAt?: string } export const saveVersion = async ({ @@ -24,6 +25,7 @@ export const saveVersion = async ({ docWithLocales, autosave, draft, + createdAt, }: Args): Promise => { let entityConfig; let entityType: 'global' | 'collection'; @@ -54,13 +56,17 @@ export const saveVersion = async ({ try { if (autosave && existingAutosaveVersion?.autosave === true) { + const data: Record = { + version: versionData, + }; + + if (createdAt) data.updatedAt = createdAt; + await VersionModel.findByIdAndUpdate( { _id: existingAutosaveVersion._id, }, - { - version: versionData, - }, + data, { new: true, lean: true }, ); // Otherwise, create a new one @@ -70,6 +76,7 @@ export const saveVersion = async ({ autosave: Boolean(autosave), }; + if (createdAt) data.createdAt = createdAt; if (collection) data.parent = id; await VersionModel.create(data); diff --git a/yarn.lock b/yarn.lock index ca47c70840..497ea791f1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8514,6 +8514,11 @@ mongodb@^3.7.3: optionalDependencies: saslprep "^1.0.0" +mongoose-aggregate-paginate-v2@^1.0.6: + version "1.0.6" + resolved "https://registry.npmjs.org/mongoose-aggregate-paginate-v2/-/mongoose-aggregate-paginate-v2-1.0.6.tgz#fd2f2564d1bbf52f49a196f0b7b03675913dacca" + integrity sha512-UuALu+mjhQa1K9lMQvjLL3vm3iALvNw8PQNIh2gp1b+tO5hUa0NC0Wf6/8QrT9PSJVTihXaD8hQVy3J4e0jO0Q== + mongoose-paginate-v2@*, mongoose-paginate-v2@^1.6.1: version "1.7.1" resolved "https://registry.yarnpkg.com/mongoose-paginate-v2/-/mongoose-paginate-v2-1.7.1.tgz#0b390f5eb8e5dca55ffcb1fd7b4d8078636cb8f1"