Skip to content

fix: invalidate cache in dataloader after a document was updated or deleted #11577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion packages/payload/src/collections/dataloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string[]> => {
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
Expand Down Expand Up @@ -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
}
Expand All @@ -157,7 +172,12 @@ const batchAndLoadDocs =
return docs
}

export const getDataLoader = (req: PayloadRequest) => new DataLoader(batchAndLoadDocs(req))
export const getDataLoader = (req: PayloadRequest): DataLoader<string, TypeWithID> => {
const dataLoader = new DataLoader(batchAndLoadDocs(req))
dataLoader[InternalIDSlugCacheKeyMap] = new Map()

return dataLoader
}

type CreateCacheKeyArgs = {
collectionSlug: string
Expand Down Expand Up @@ -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}`)
}
3 changes: 3 additions & 0 deletions packages/payload/src/collections/operations/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -203,6 +204,8 @@ export const deleteOperation = async <
},
})

invalidateDocumentInDataloader({ id, collectionSlug: collectionConfig.slug, req })

// /////////////////////////////////////
// afterRead - Fields
// /////////////////////////////////////
Expand Down
5 changes: 4 additions & 1 deletion packages/payload/src/collections/operations/deleteByID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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 = {
Expand Down Expand Up @@ -177,6 +178,8 @@ export const deleteByIDOperation = async <TSlug extends CollectionSlug, TSelect
where: { id: { equals: id } },
})

invalidateDocumentInDataloader({ id, collectionSlug: collectionConfig.slug, req })

// /////////////////////////////////////
// Delete Preferences
// /////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { uploadFiles } from '../../../uploads/uploadFiles.js'
import { checkDocumentLockStatus } from '../../../utilities/checkDocumentLockStatus.js'
import { getLatestCollectionVersion } from '../../../versions/getLatestCollectionVersion.js'
import { saveVersion } from '../../../versions/saveVersion.js'
import { invalidateDocumentInDataloader } from '../../dataloader.js'

export type SharedUpdateDocumentArgs<TSlug extends CollectionSlug> = {
accessResults: AccessResult
Expand Down Expand Up @@ -314,6 +315,8 @@ export const updateDocument = async <
})
}

invalidateDocumentInDataloader({ id, collectionSlug: collectionConfig.slug, req })

// /////////////////////////////////////
// afterRead - Fields
// /////////////////////////////////////
Expand Down
77 changes: 75 additions & 2 deletions test/relationships/int.spec.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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}`, {
Expand Down
Loading