From c755143cc09cbb7a8d1e2134b195d41695e3e257 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 19 Dec 2022 16:01:30 -0500 Subject: [PATCH] chore: uses mongo aggregate for merging draft results --- src/collections/operations/find.ts | 64 +------ src/globals/buildModel.ts | 2 +- .../drafts/buildDraftMergeAggregate.ts | 95 ++++++++++ src/versions/drafts/mergeDrafts.ts | 173 ------------------ test/versions/int.spec.ts | 95 ++++++++++ 5 files changed, 196 insertions(+), 233 deletions(-) create mode 100644 src/versions/drafts/buildDraftMergeAggregate.ts delete mode 100644 src/versions/drafts/mergeDrafts.ts diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index fdda6c86c..c42edaf57 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -9,6 +9,7 @@ import flattenWhereConstraints from '../../utilities/flattenWhereConstraints'; import { buildSortParam } from '../../mongoose/buildSortParam'; import { AccessResult } from '../../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; +import { buildDraftMergeAggregate } from '../../versions/drafts/buildDraftMergeAggregate'; export type Arguments = { collection: Collection @@ -158,65 +159,10 @@ async function find(incomingArgs: Arguments): Promis }; if (collectionConfig.versions?.drafts && draftsEnabled) { - const aggregate = Model.aggregate([ - { - $addFields: { id: { $toString: '$_id' } }, - }, - { - $lookup: { - from: `_${collectionConfig.slug}_versions`, - as: 'docs', - let: { - id: { $toString: '$_id' }, - updatedAt: '$updatedAt', - }, - pipeline: [ - { - $match: { - $expr: { - $and: [ - { $eq: ['$parent', '$$id'] }, - { $gt: ['$updatedAt', '$$updatedAt'] }, - ], - }, - }, - }, - { $sort: { updatedAt: -1 } }, - { $limit: 1 }, - ], - }, - }, - - // WHEN docs.length > 0 - { - $project: { - doc: { $arrayElemAt: ['$docs', 0] }, - }, - }, - { - $addFields: { - createdAt: '$doc.createdAt', - updatedAt: '$doc.updatedAt', - }, - }, - { - $replaceRoot: { - newRoot: { - $mergeObjects: ['$doc.version', '$$ROOT'], - }, - }, - }, - // End "when" - { - $project: { - docs: 0, - doc: 0, - }, - }, - { - $match: query, - }, - ]); + const aggregate = Model.aggregate(buildDraftMergeAggregate({ + config: collectionConfig, + query, + })); result = await Model.aggregatePaginate(aggregate, paginationOptions); } else { diff --git a/src/globals/buildModel.ts b/src/globals/buildModel.ts index 544fa0b7d..4fcbe46ca 100644 --- a/src/globals/buildModel.ts +++ b/src/globals/buildModel.ts @@ -10,7 +10,7 @@ const buildModel = (config: SanitizedConfig): GlobalModel | null => { globalsSchema.plugin(buildQueryPlugin); - const Globals = mongoose.model('globals', globalsSchema) as GlobalModel; + const Globals = mongoose.model('globals', globalsSchema) as unknown as GlobalModel; Object.values(config.globals).forEach((globalConfig) => { const globalSchema = buildSchema(config, globalConfig.fields, { global: true }); diff --git a/src/versions/drafts/buildDraftMergeAggregate.ts b/src/versions/drafts/buildDraftMergeAggregate.ts new file mode 100644 index 000000000..7e59594b8 --- /dev/null +++ b/src/versions/drafts/buildDraftMergeAggregate.ts @@ -0,0 +1,95 @@ +import { PipelineStage } from 'mongoose'; +import { SanitizedCollectionConfig } from '../../collections/config/types'; + +type Args = { + config: SanitizedCollectionConfig + query: Record +} + +export const buildDraftMergeAggregate = ({ config, query }: Args): PipelineStage[] => [ + // Add string-based ID to query with + { + $addFields: { id: { $toString: '$_id' } }, + }, + + // Merge in one version + // that has the same parent ID + // and is newer, sorting by updatedAt + { + $lookup: { + from: `_${config.slug}_versions`, + as: 'docs', + let: { + id: { $toString: '$_id' }, + updatedAt: '$updatedAt', + }, + pipeline: [ + { + $match: { + $expr: { + $and: [ + { $eq: ['$parent', '$$id'] }, + { $gt: ['$updatedAt', '$$updatedAt'] }, + ], + }, + }, + }, + { $sort: { updatedAt: -1 } }, + { $limit: 1 }, + ], + }, + }, + + // Add a new field + // with the first doc returned + { + $addFields: { + doc: { + $arrayElemAt: ['$docs', 0], + }, + }, + }, + + // If newer version exists, + // merge the version into the root and + // replace the updatedAt + // Otherwise, do nothing to the root + { + $replaceRoot: { + newRoot: { + $cond: { + if: { + $ne: ['$doc', undefined], + }, + then: { + $mergeObjects: [ + '$$ROOT', + { + $mergeObjects: [ + '$doc.version', + { + updatedAt: '$doc.updatedAt', + }, + ], + }, + ], + }, + else: '$$ROOT', + }, + }, + }, + }, + + // Clear out the temporarily added `docs` + { + $project: { + doc: 0, + docs: 0, + }, + }, + + // Run the original query on the results + { + $match: query, + }, +]; diff --git a/src/versions/drafts/mergeDrafts.ts b/src/versions/drafts/mergeDrafts.ts deleted file mode 100644 index 45830f8e0..000000000 --- a/src/versions/drafts/mergeDrafts.ts +++ /dev/null @@ -1,173 +0,0 @@ -import { AccessResult } from '../../config/types'; -import { docHasTimestamps, 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 sanitizeInternalFields from '../../utilities/sanitizeInternalFields'; -import replaceWithDraftIfAvailable from './replaceWithDraftIfAvailable'; - -type AggregateVersion = { - _id: string - version: T - updatedAt: string - createdAt: string -} - -type Args = { - accessResult: AccessResult - collection: Collection - locale: string - originalQueryResult: PaginatedDocs - paginationOptions: any - payload: Payload - where: Where -} - -export const mergeDrafts = async ({ - accessResult, - collection, - locale, - originalQueryResult, - 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 || [], - { - 'version._status': { - equals: 'draft', - }, - }, - ], - }, - }; - - if (hasWhereAccessResult(accessResult)) { - const versionAccessResult = appendVersionToQueryKey(accessResult); - versionQueryToBuild.where.and.push(versionAccessResult); - } - - const versionQuery = await VersionModel.buildQuery(versionQueryToBuild, locale); - - const matchedDraftVersions = await VersionModel.aggregate>([ - { $sort: { updatedAt: -1 } }, - { $match: versionQuery }, - { - $group: { - _id: '$parent', - version: { $first: '$version' }, - updatedAt: { $first: '$updatedAt' }, - createdAt: { $first: '$createdAt' }, - }, - }, - ]); - - const matchedDrafts: AggregateVersion[] = []; - const unmatchedDrafts: AggregateVersion[] = []; - - matchedDraftVersions.forEach((draft) => { - const matchedDocFromOriginalQuery = originalQueryResult.docs.find(({ id }) => id === draft._id); - const sanitizedDraft = JSON.parse(JSON.stringify(draft)); - - // If we find a matched doc from the original query, - // No need to store this doc - if (matchedDocFromOriginalQuery) { - matchedDrafts.push(sanitizedDraft); - } else { - unmatchedDrafts.push(sanitizedDraft); - } - }); - - let result = originalQueryResult; - - // If there are results from drafts, - // we need to re-query while explicitly passing in - // the IDs of the un-matched drafts so that they appear correctly - // in paginated results, properly paginated - if (unmatchedDrafts.length > 0) { - const whereWithUnmatchedIDs: { where: Where } = { - where: { - and: [], - }, - }; - - if (hasWhereAccessResult(accessResult)) { - whereWithUnmatchedIDs.where.and.push(accessResult); - } - - if (where) { - whereWithUnmatchedIDs.where.and.push({ - or: [ - where, - { - id: { - in: unmatchedDrafts.map(({ _id }) => _id), - }, - }, - ], - }); - } - - const queryWithUnmatchedIDs = await collection.Model.buildQuery(whereWithUnmatchedIDs, locale); - - result = await collection.Model.paginate(queryWithUnmatchedIDs, paginationOptions); - - result = { - ...result, - docs: result.docs.map((doc) => { - const sanitizedDoc = JSON.parse(JSON.stringify(doc)); - sanitizedDoc.id = sanitizedDoc._id; - return sanitizeInternalFields(sanitizedDoc); - }), - }; - } - - result = { - ...result, - docs: await Promise.all(result.docs.map(async (doc) => { - // If we have an existing unmatched draft, we can replace with that if it's newer - const correlatedUnmatchedDraft = unmatchedDrafts.find(({ _id, updatedAt }) => _id === doc.id && docHasTimestamps(doc) && updatedAt > doc.updatedAt); - - if (correlatedUnmatchedDraft) { - return { - ...doc, - ...correlatedUnmatchedDraft.version, - createdAt: correlatedUnmatchedDraft.createdAt, - updatedAt: correlatedUnmatchedDraft.updatedAt, - }; - } - - const correlatedMatchedDraft = matchedDrafts.find(({ _id, updatedAt }) => _id === doc.id && docHasTimestamps(doc) && updatedAt > doc.updatedAt); - - if (correlatedMatchedDraft) { - return { - ...doc, - ...correlatedMatchedDraft.version, - createdAt: correlatedMatchedDraft.createdAt, - updatedAt: correlatedMatchedDraft.updatedAt, - }; - } - - return replaceWithDraftIfAvailable({ - accessResult, - payload, - entity: collection.config, - entityType: 'collection', - doc, - locale, - }); - })), - }; - - return result; -}; diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 17e71a28c..c6f31c1cc 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -258,6 +258,101 @@ describe('Versions', () => { }); }); + describe('Querying', () => { + const originalTitle = 'original title'; + const updatedTitle1 = 'new title 1'; + const updatedTitle2 = 'new title 2'; + let firstDraft; + + beforeAll(async () => { + // This will be created in the `draft-posts` collection + firstDraft = await payload.create({ + collection: 'draft-posts', + data: { + title: originalTitle, + description: 'my description', + radio: 'test', + }, + }); + + // This will be created in the `_draft-posts_versions` collection + await payload.update({ + collection: 'draft-posts', + id: firstDraft.id, + draft: true, + data: { + title: updatedTitle1, + }, + }); + + // This will be created in the `_draft-posts_versions` collection + // and will be the newest draft, able to be queried on + await payload.update({ + collection: 'draft-posts', + id: firstDraft.id, + draft: true, + data: { + title: updatedTitle2, + }, + }); + }); + + it('should allow querying a draft doc from main collection', async () => { + const findResults = await payload.find({ + collection: 'draft-posts', + where: { + title: { + equals: originalTitle, + }, + }, + }); + + expect(findResults.docs[0].title).toStrictEqual(originalTitle); + }); + + it('should not be able to query an old draft version with draft=true', async () => { + const draftFindResults = await payload.find({ + collection: 'draft-posts', + draft: true, + where: { + title: { + equals: updatedTitle1, + }, + }, + }); + + expect(draftFindResults.docs).toHaveLength(0); + }); + + it('should be able to query the newest draft version with draft=true', async () => { + const draftFindResults = await payload.find({ + collection: 'draft-posts', + draft: true, + where: { + title: { + equals: updatedTitle2, + }, + }, + }); + + expect(draftFindResults.docs[0].title).toStrictEqual(updatedTitle2); + }); + + it('should not be able to query old drafts that don\'t match with draft=true', async () => { + const draftFindResults = await payload.find({ + collection: 'draft-posts', + draft: true, + where: { + title: { + equals: originalTitle, + }, + }, + }); + + expect(draftFindResults.docs).toHaveLength(0); + }); + }); + describe('Collections - GraphQL', () => { describe('Create', () => { it('should allow a new doc to be created with draft status', async () => {