From 0a5c97b568364c51c3074fdedc7fca1fefa9907c Mon Sep 17 00:00:00 2001 From: Sasha <64744993+r1tsuu@users.noreply.github.com> Date: Thu, 6 Mar 2025 20:45:46 +0200 Subject: [PATCH] fix: invalidate cache in dataloader after a document was updated or deleted --- .../payload/src/collections/dataloader.ts | 49 +++++++++++- .../src/collections/operations/delete.ts | 3 + .../src/collections/operations/deleteByID.ts | 5 +- .../operations/utilities/update.ts | 3 + test/relationships/int.spec.ts | 77 ++++++++++++++++++- 5 files changed, 133 insertions(+), 4 deletions(-) diff --git a/packages/payload/src/collections/dataloader.ts b/packages/payload/src/collections/dataloader.ts index 38934876947..e6a0e3c692b 100644 --- a/packages/payload/src/collections/dataloader.ts +++ b/packages/payload/src/collections/dataloader.ts @@ -3,11 +3,18 @@ import type { BatchLoadFn } from 'dataloader' import DataLoader from 'dataloader' +import type { CollectionSlug } from '../index.js' import type { PayloadRequest, PopulateType, SelectType } from '../types/index.js' import type { TypeWithID } from './config/types.js' import { isValidID } from '../utilities/isValidID.js' +const InternalIDSlugCacheKeyMap = Symbol('InternalIDSlugCacheKeyMap') + +const getInternalIDSlugCacheKeyMap = (req: PayloadRequest): Map => { + return req.payloadDataLoader[InternalIDSlugCacheKeyMap] +} + // Payload uses `dataloader` to solve the classic GraphQL N+1 problem. // We keep a list of all documents requested to be populated for any given request @@ -145,6 +152,14 @@ const batchAndLoadDocs = }) const docsIndex = keys.findIndex((key) => key === docKey) + const map = getInternalIDSlugCacheKeyMap(req) + let items = map.get(`${doc.id}-${collection}`) + if (!items) { + items = [] + map.set(`${doc.id}-${collection}`, items) + } + items.push(docKey) + if (docsIndex > -1) { docs[docsIndex] = doc } @@ -157,7 +172,12 @@ const batchAndLoadDocs = return docs } -export const getDataLoader = (req: PayloadRequest) => new DataLoader(batchAndLoadDocs(req)) +export const getDataLoader = (req: PayloadRequest): DataLoader => { + const dataLoader = new DataLoader(batchAndLoadDocs(req)) + dataLoader[InternalIDSlugCacheKeyMap] = new Map() + + return dataLoader +} type CreateCacheKeyArgs = { collectionSlug: string @@ -201,3 +221,30 @@ export const createDataloaderCacheKey = ({ select, populate, ]) + +export const invalidateDocumentInDataloader = ({ + id, + collectionSlug, + req, +}: { + collectionSlug: CollectionSlug + id: number | string + req: PayloadRequest +}) => { + if (!req.payloadDataLoader) { + return + } + + const cacheKeyMap = getInternalIDSlugCacheKeyMap(req) + const keys = cacheKeyMap.get(`${id}-${collectionSlug}`) + + if (!keys) { + return + } + + for (const key of keys) { + req.payloadDataLoader.clear(key) + } + + cacheKeyMap.delete(`${id}-${collectionSlug}`) +} diff --git a/packages/payload/src/collections/operations/delete.ts b/packages/payload/src/collections/operations/delete.ts index 28a777319f6..988f9193c1d 100644 --- a/packages/payload/src/collections/operations/delete.ts +++ b/packages/payload/src/collections/operations/delete.ts @@ -25,6 +25,7 @@ import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' import { deleteCollectionVersions } from '../../versions/deleteCollectionVersions.js' import { deleteScheduledPublishJobs } from '../../versions/deleteScheduledPublishJobs.js' +import { invalidateDocumentInDataloader } from '../dataloader.js' import { buildAfterOperation } from './utils.js' export type Arguments = { @@ -203,6 +204,8 @@ export const deleteOperation = async < }, }) + invalidateDocumentInDataloader({ id, collectionSlug: collectionConfig.slug, req }) + // ///////////////////////////////////// // afterRead - Fields // ///////////////////////////////////// diff --git a/packages/payload/src/collections/operations/deleteByID.ts b/packages/payload/src/collections/operations/deleteByID.ts index fc72780d4b7..317c95a1be3 100644 --- a/packages/payload/src/collections/operations/deleteByID.ts +++ b/packages/payload/src/collections/operations/deleteByID.ts @@ -6,7 +6,7 @@ import type { SelectType, TransformCollectionWithSelect, } from '../../types/index.js' -import type { BeforeOperationHook, Collection, DataFromCollectionSlug } from '../config/types.js' +import type { Collection, DataFromCollectionSlug } from '../config/types.js' import executeAccess from '../../auth/executeAccess.js' import { hasWhereAccessResult } from '../../auth/types.js' @@ -21,6 +21,7 @@ import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' import { deleteCollectionVersions } from '../../versions/deleteCollectionVersions.js' import { deleteScheduledPublishJobs } from '../../versions/deleteScheduledPublishJobs.js' +import { invalidateDocumentInDataloader } from '../dataloader.js' import { buildAfterOperation } from './utils.js' export type Arguments = { @@ -177,6 +178,8 @@ export const deleteByIDOperation = async = { accessResults: AccessResult @@ -314,6 +315,8 @@ export const updateDocument = async < }) } + invalidateDocumentInDataloader({ id, collectionSlug: collectionConfig.slug, req }) + // ///////////////////////////////////// // afterRead - Fields // ///////////////////////////////////// diff --git a/test/relationships/int.spec.ts b/test/relationships/int.spec.ts index 6a562370320..86f5c0d58a6 100644 --- a/test/relationships/int.spec.ts +++ b/test/relationships/int.spec.ts @@ -1,11 +1,17 @@ -import type { Payload, PayloadRequest } from 'payload' - import { randomBytes, randomUUID } from 'crypto' import path from 'path' +import { + commitTransaction, + createLocalReq, + initTransaction, + type Payload, + type PayloadRequest, +} from 'payload' import { fileURLToPath } from 'url' import type { NextRESTClient } from '../helpers/NextRESTClient.js' import type { + Chained, ChainedRelation, CustomIdNumberRelation, CustomIdRelation, @@ -898,6 +904,73 @@ describe('Relationships', () => { expect(doc?.chainedRelation).toEqual(chained.id) }) + it('should populate with a right data within the same transaction after update', async () => { + const req = await createLocalReq({}, payload) + await initTransaction(req) + + const chained = await payload.create({ + collection: chainedRelSlug, + req, + data: { name: 'my-name' }, + }) + let doc = await payload.create({ + collection: slug, + data: { chainedRelation: chained.id }, + req, + }) + expect((doc.chainedRelation as Chained).id).toBe(chained.id) + + await payload.update({ + collection: chainedRelSlug, + req, + data: { name: 'new-name' }, + id: chained.id, + }) + + doc = await payload.findByID({ + collection: slug, + id: doc.id, + req, + }) + + expect((doc.chainedRelation as Chained).name).toBe('new-name') + + await commitTransaction(req) + }) + + it('should not populate within the same transaction after delete', async () => { + const req = await createLocalReq({}, payload) + await initTransaction(req) + + const chained = await payload.create({ + collection: chainedRelSlug, + req, + data: { name: 'my-name' }, + }) + let doc = await payload.create({ + collection: slug, + data: { chainedRelation: chained.id }, + req, + }) + expect((doc.chainedRelation as Chained).id).toBe(chained.id) + + await payload.delete({ + collection: chainedRelSlug, + req, + id: chained.id, + }) + + doc = await payload.findByID({ + collection: slug, + id: doc.id, + req, + }) + + expect((doc.chainedRelation as Chained)?.name).toBeUndefined() + + await commitTransaction(req) + }) + it('should respect maxDepth at field level', async () => { const doc = await restClient .GET(`/${slug}/${post.id}`, {