tabshift/payload
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: adds overrideLock flag to update & delete operations (#8294)

Browse Source 
- Adds `overrideLock` flag to `update` & `delete` operations

- Instead of throwing an `APIError` (500) when trying to update / delete
a locked document - now throw a `Locked` (423) error status
This commit is contained in:
Patrik
2024-09-19 17:07:02 -04:00
committed by GitHub
parent 879f690161
commit 9c3f863ad2
15 changed files with 307 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
docs/admin/locked-documents.mdx
View File

@@ -58,3 +58,22 @@ export const Posts: CollectionConfig = {
Document locking affects both the Local API and the REST API, ensuring that if a document is locked, concurrent users will not be able to perform updates or deletes on that document (including globals). If a user attempts to update or delete a locked document, they will receive an error.
Once the document is unlocked or the lock duration has expired, other users can proceed with updates or deletes as normal.
#### Overriding Locks
For operations like update and delete, Payload includes an `overrideLock` option. This boolean flag, when set to `false`, enforces document locks, ensuring that the operation will not proceed if another user currently holds the lock.
By default, `overrideLock` is set to `true`, which means that document locks are ignored, and the operation will proceed even if the document is locked. To enforce locks and prevent updates or deletes on locked documents, set `overrideLock: false`.
```ts
const result = await payload.update({
collection: 'posts',
id: '123',
data: {
title: 'New title',
},
overrideLock: false, // Enforces the document lock, preventing updates if the document is locked
})
```
This option is particularly useful in scenarios where administrative privileges or specific workflows require you to override the lock and ensure the operation is completed.

28
docs/local-api/overview.mdx
View File

@@ -77,17 +77,18 @@ Both options function in exactly the same way outside of one having HMR support
You can specify more options within the Local API vs. REST or GraphQL due to the server-only context that they are executed in.
| Local Option | Description |
| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `collection` | Required for Collection operations. Specifies the Collection slug to operate against. |
| `data` | The data to use within the operation. Required for `create`, `update`. |
| `depth` | [Control auto-population](../queries/depth) of nested relationship and upload fields. |
| `locale` | Specify [locale](/docs/configuration/localization) for any returned documents. |
| `fallbackLocale` | Specify a [fallback locale](/docs/configuration/localization) to use for any returned documents. |
| `overrideAccess` | Skip access control. By default, this property is set to true within all Local API operations. |
| `user` | If you set `overrideAccess` to `false`, you can pass a user to use against the access control checks. |
| `showHiddenFields` | Opt-in to receiving hidden fields. By default, they are hidden from returned documents in accordance to your config. |
| `pagination` | Set to false to return all documents and avoid querying for document counts. |
| Local Option | Description |
| ------------------ | ------------ |
| `collection` | Required for Collection operations. Specifies the Collection slug to operate against. |
| `data` | The data to use within the operation. Required for `create`, `update`. |
| `depth` | [Control auto-population](../queries/depth) of nested relationship and upload fields. |
| `locale` | Specify [locale](/docs/configuration/localization) for any returned documents. |
| `fallbackLocale` | Specify a [fallback locale](/docs/configuration/localization) to use for any returned documents. |
| `overrideAccess` | Skip access control. By default, this property is set to true within all Local API operations. |
| `overrideLock` | By default, document locks are ignored (`true`). Set to `false` to enforce locks and prevent operations when a document is locked by another user. [More details](../admin/locked-documents).|
| `user` | If you set `overrideAccess` to `false`, you can pass a user to use against the access control checks. |
| `showHiddenFields` | Opt-in to receiving hidden fields. By default, they are hidden from returned documents in accordance to your config. |
| `pagination` | Set to false to return all documents and avoid querying for document counts. |
| `context` | [Context](/docs/hooks/context), which will then be passed to `context` and `req.context`, which can be read by hooks. Useful if you want to pass additional information to the hooks which shouldn't be necessarily part of the document, for example a `triggerBeforeChange` option which can be read by the BeforeChange hook to determine if it should run or not. |
| `disableErrors` | When set to `true`, errors will not be thrown. Instead, the `findByID` operation will return `null`, and the `find` operation will return an empty documents array. |
@@ -206,6 +207,7 @@ const result = await payload.update({
fallbackLocale: false,
user: dummyUser,
overrideAccess: false,
overrideLock: false, // By default, document locks are ignored. Set to false to enforce locks.
showHiddenFields: true,
// If your collection supports uploads, you can upload
@@ -244,6 +246,7 @@ const result = await payload.update({
fallbackLocale: false,
user: dummyUser,
overrideAccess: false,
overrideLock: false, // By default, document locks are ignored. Set to false to enforce locks.
showHiddenFields: true,
// If your collection supports uploads, you can upload
@@ -270,6 +273,7 @@ const result = await payload.delete({
fallbackLocale: false,
user: dummyUser,
overrideAccess: false,
overrideLock: false, // By default, document locks are ignored. Set to false to enforce locks.
showHiddenFields: true,
})
```
@@ -293,6 +297,7 @@ const result = await payload.delete({
fallbackLocale: false,
user: dummyUser,
overrideAccess: false,
overrideLock: false, // By default, document locks are ignored. Set to false to enforce locks.
showHiddenFields: true,
})
```
@@ -429,6 +434,7 @@ const result = await payload.updateGlobal({
fallbackLocale: false,
user: dummyUser,
overrideAccess: false,
overrideLock: false, // By default, document locks are ignored. Set to false to enforce locks.
showHiddenFields: true,
})
```

3
packages/payload/src/collections/operations/delete.ts
View File

@@ -23,6 +23,7 @@ export type Arguments = {
collection: Collection
depth?: number
overrideAccess?: boolean
overrideLock?: boolean
req: PayloadRequest
showHiddenFields?: boolean
where: Where
@@ -65,6 +66,7 @@ export const deleteOperation = async <TSlug extends CollectionSlug>(
collection: { config: collectionConfig },
depth,
overrideAccess,
overrideLock,
req: {
fallbackLocale,
locale,
@@ -126,6 +128,7 @@ export const deleteOperation = async <TSlug extends CollectionSlug>(
id,
collectionSlug: collectionConfig.slug,
lockErrorMessage: `Document with ID ${id} is currently locked and cannot be deleted.`,
overrideLock,
req,
})

3
packages/payload/src/collections/operations/deleteByID.ts
View File

@@ -21,6 +21,7 @@ export type Arguments = {
depth?: number
id: number | string
overrideAccess?: boolean
overrideLock?: boolean
req: PayloadRequest
showHiddenFields?: boolean
}
@@ -58,6 +59,7 @@ export const deleteByIDOperation = async <TSlug extends CollectionSlug>(
collection: { config: collectionConfig },
depth,
overrideAccess,
overrideLock,
req: {
fallbackLocale,
locale,
@@ -118,6 +120,7 @@ export const deleteByIDOperation = async <TSlug extends CollectionSlug>(
id,
collectionSlug: collectionConfig.slug,
lockErrorMessage: `Document with ID ${id} is currently locked and cannot be deleted.`,
overrideLock,
req,
})

3
packages/payload/src/collections/operations/local/delete.ts
View File

@@ -17,6 +17,7 @@ export type BaseOptions<TSlug extends CollectionSlug> = {
fallbackLocale?: TypedLocale
locale?: TypedLocale
overrideAccess?: boolean
overrideLock?: boolean
req?: PayloadRequest
showHiddenFields?: boolean
user?: Document
@@ -55,6 +56,7 @@ async function deleteLocal<TSlug extends CollectionSlug>(
collection: collectionSlug,
depth,
overrideAccess = true,
overrideLock,
showHiddenFields,
where,
} = options
@@ -72,6 +74,7 @@ async function deleteLocal<TSlug extends CollectionSlug>(
collection,
depth,
overrideAccess,
overrideLock,
req: await createLocalReq(options, payload),
showHiddenFields,
where,

3
packages/payload/src/collections/operations/local/update.ts
View File

@@ -30,6 +30,7 @@ export type BaseOptions<TSlug extends CollectionSlug> = {
filePath?: string
locale?: TypedLocale
overrideAccess?: boolean
overrideLock?: boolean
overwriteExistingFiles?: boolean
publishSpecificLocale?: string
req?: PayloadRequest
@@ -75,6 +76,7 @@ async function updateLocal<TSlug extends CollectionSlug>(
file,
filePath,
overrideAccess = true,
overrideLock,
overwriteExistingFiles = false,
publishSpecificLocale,
showHiddenFields,
@@ -100,6 +102,7 @@ async function updateLocal<TSlug extends CollectionSlug>(
depth,
draft,
overrideAccess,
overrideLock,
overwriteExistingFiles,
payload,
publishSpecificLocale,

3
packages/payload/src/collections/operations/update.ts
View File

@@ -41,6 +41,7 @@ export type Arguments<TSlug extends CollectionSlug> = {
disableVerificationEmail?: boolean
draft?: boolean
overrideAccess?: boolean
overrideLock?: boolean
overwriteExistingFiles?: boolean
req: PayloadRequest
showHiddenFields?: boolean
@@ -78,6 +79,7 @@ export const updateOperation = async <TSlug extends CollectionSlug>(
depth,
draft: draftArg = false,
overrideAccess,
overrideLock,
overwriteExistingFiles = false,
req: {
fallbackLocale,
@@ -186,6 +188,7 @@ export const updateOperation = async <TSlug extends CollectionSlug>(
id,
collectionSlug: collectionConfig.slug,
lockErrorMessage: `Document with ID ${id} is currently locked by another user and cannot be updated.`,
overrideLock,
req,
})

3
packages/payload/src/collections/operations/updateByID.ts
View File

@@ -43,6 +43,7 @@ export type Arguments<TSlug extends CollectionSlug> = {
draft?: boolean
id: number | string
overrideAccess?: boolean
overrideLock?: boolean
overwriteExistingFiles?: boolean
publishSpecificLocale?: string
req: PayloadRequest
@@ -86,6 +87,7 @@ export const updateByIDOperation = async <TSlug extends CollectionSlug>(
depth,
draft: draftArg = false,
overrideAccess,
overrideLock,
overwriteExistingFiles = false,
publishSpecificLocale,
req: {
@@ -150,6 +152,7 @@ export const updateByIDOperation = async <TSlug extends CollectionSlug>(
id,
collectionSlug: collectionConfig.slug,
lockErrorMessage: `Document with ID ${id} is currently locked by another user and cannot be updated.`,
overrideLock,
req,
})

9
packages/payload/src/errors/Locked.ts Normal file
View File

@@ -0,0 +1,9 @@
import httpStatus from 'http-status'
import { APIError } from './APIError.js'
export class Locked extends APIError {
constructor(message: string) {
super(message, httpStatus.LOCKED)
}
}

1
packages/payload/src/errors/index.ts
View File

@@ -10,6 +10,7 @@ export { Forbidden } from './Forbidden.js'
export { InvalidConfiguration } from './InvalidConfiguration.js'
export { InvalidFieldName } from './InvalidFieldName.js'
export { InvalidFieldRelationship } from './InvalidFieldRelationship.js'
export { Locked } from './Locked.js'
export { LockedAuth } from './LockedAuth.js'
export { MissingCollectionLabel } from './MissingCollectionLabel.js'
export { MissingEditorProp } from './MissingEditorProp.js'

3
packages/payload/src/globals/operations/local/update.ts
View File

@@ -16,6 +16,7 @@ export type Options<TSlug extends GlobalSlug> = {
fallbackLocale?: TypedLocale
locale?: TypedLocale
overrideAccess?: boolean
overrideLock?: boolean
publishSpecificLocale?: TypedLocale
req?: PayloadRequest
showHiddenFields?: boolean
@@ -33,6 +34,7 @@ export default async function updateLocal<TSlug extends GlobalSlug>(
depth,
draft,
overrideAccess = true,
overrideLock,
publishSpecificLocale,
showHiddenFields,
} = options
@@ -50,6 +52,7 @@ export default async function updateLocal<TSlug extends GlobalSlug>(
draft,
globalConfig,
overrideAccess,
overrideLock,
publishSpecificLocale,
req: await createLocalReq(options, payload),
showHiddenFields,

3
packages/payload/src/globals/operations/update.ts
View File

@@ -24,6 +24,7 @@ type Args<TSlug extends GlobalSlug> = {
draft?: boolean
globalConfig: SanitizedGlobalConfig
overrideAccess?: boolean
overrideLock?: boolean
publishSpecificLocale?: string
req: PayloadRequest
showHiddenFields?: boolean
@@ -44,6 +45,7 @@ export const updateOperation = async <TSlug extends GlobalSlug>(
draft: draftArg,
globalConfig,
overrideAccess,
overrideLock,
publishSpecificLocale,
req: { fallbackLocale, locale, payload },
req,
@@ -121,6 +123,7 @@ export const updateOperation = async <TSlug extends GlobalSlug>(
await checkDocumentLockStatus({
globalSlug: slug,
lockErrorMessage: `Global with slug "${slug}" is currently locked by another user and cannot be updated.`,
overrideLock,
req,
})

1
packages/payload/src/index.ts
View File

@@ -817,6 +817,7 @@ export {
InvalidConfiguration,
InvalidFieldName,
InvalidFieldRelationship,
Locked,
LockedAuth,
MissingCollectionLabel,
MissingEditorProp,

79
packages/payload/src/utilities/checkDocumentLockStatus.ts
View File

@@ -2,7 +2,7 @@ import type { TypeWithID } from '../collections/config/types.js'
import type { PaginatedDocs } from '../database/types.js'
import type { JsonObject, PayloadRequest } from '../types/index.js'
import { APIError } from '../errors/index.js'
import { Locked } from '../errors/index.js'
type CheckDocumentLockStatusArgs = {
collectionSlug?: string
@@ -10,6 +10,7 @@ type CheckDocumentLockStatusArgs = {
id?: number | string
lockDurationDefault?: number
lockErrorMessage?: string
overrideLock?: boolean
req: PayloadRequest
}
@@ -19,6 +20,7 @@ export const checkDocumentLockStatus = async ({
globalSlug,
lockDurationDefault = 300, // Default 5 minutes in seconds
lockErrorMessage,
overrideLock = true,
req,
}: CheckDocumentLockStatusArgs): Promise<void> => {
const { payload } = req
@@ -30,11 +32,6 @@ export const checkDocumentLockStatus = async ({
const isLockingEnabled = lockDocumentsProp !== false
// If lockDocuments is explicitly set to false, skip the lock logic and return early
if (isLockingEnabled === false) {
return
}
let lockedDocumentQuery = {}
if (collectionSlug) {
@@ -50,45 +47,47 @@ export const checkDocumentLockStatus = async ({
throw new Error('Either collectionSlug or globalSlug must be provided.')
}
const defaultLockErrorMessage = collectionSlug
? `Document with ID ${id} is currently locked by another user and cannot be modified.`
: `Global document with slug "${globalSlug}" is currently locked by another user and cannot be modified.`
// Only perform lock checks if overrideLock is false and locking is enabled
if (!overrideLock && isLockingEnabled !== false) {
const defaultLockErrorMessage = collectionSlug
? `Document with ID ${id} is currently locked by another user and cannot be modified.`
: `Global document with slug "${globalSlug}" is currently locked by another user and cannot be modified.`
const finalLockErrorMessage = lockErrorMessage || defaultLockErrorMessage
const finalLockErrorMessage = lockErrorMessage || defaultLockErrorMessage
const lockedDocumentResult: PaginatedDocs<JsonObject & TypeWithID> = await payload.find({
collection: 'payload-locked-documents',
depth: 1,
limit: 1,
pagination: false,
req,
sort: '-updatedAt',
where: lockedDocumentQuery,
})
const lockedDocumentResult: PaginatedDocs<JsonObject & TypeWithID> = await payload.find({
collection: 'payload-locked-documents',
depth: 1,
limit: 1,
pagination: false,
req,
sort: '-updatedAt',
where: lockedDocumentQuery,
})
// If there's a locked document, check lock conditions
const lockedDoc = lockedDocumentResult?.docs[0]
if (!lockedDoc) {
return
}
const lastEditedAt = new Date(lockedDoc?.editedAt)
const now = new Date()
const lockDuration =
typeof lockDocumentsProp === 'object' ? lockDocumentsProp.duration : lockDurationDefault
const lockDurationInMilliseconds = lockDuration * 1000
const currentUserId = req.user?.id
// document is locked by another user and the lock hasn't expired
if (
lockedDoc.user?.value?.id !== currentUserId &&
now.getTime() - lastEditedAt.getTime() <= lockDurationInMilliseconds
) {
throw new APIError(finalLockErrorMessage)
// If there's a locked document, check lock conditions
const lockedDoc = lockedDocumentResult?.docs[0]
if (lockedDoc) {
const lastEditedAt = new Date(lockedDoc?.editedAt)
const now = new Date()
const lockDuration =
typeof lockDocumentsProp === 'object' ? lockDocumentsProp.duration : lockDurationDefault
const lockDurationInMilliseconds = lockDuration * 1000
const currentUserId = req.user?.id
// document is locked by another user and the lock hasn't expired
if (
lockedDoc.user?.value?.id !== currentUserId &&
now.getTime() - lastEditedAt.getTime() <= lockDurationInMilliseconds
) {
throw new Locked(finalLockErrorMessage)
}
}
}
// Perform the delete operation regardless of overrideLock status
await payload.db.deleteMany({
collection: 'payload-locked-documents',
req,

203
test/locked-documents/int.spec.ts
View File

@@ -1,5 +1,5 @@
import path from 'path'
import { APIError, NotFound, type Payload } from 'payload'
import { Locked, NotFound, type Payload } from 'payload'
import { fileURLToPath } from 'url'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
@@ -156,6 +156,7 @@ describe('Locked documents', () => {
data: {
text: 'updated post 2',
},
overrideLock: false,
id: newPost2.id,
})
@@ -209,6 +210,7 @@ describe('Locked documents', () => {
data: {
globalText: 'global text 2',
},
overrideLock: false,
slug: menuSlug,
})
@@ -269,15 +271,21 @@ describe('Locked documents', () => {
data: {
text: 'updated post',
},
overrideLock: false, // necessary to trigger the lock check
id: newPost.id,
})
} catch (error) {
expect(error).toBeInstanceOf(APIError)
expect(error).toBeInstanceOf(Locked)
expect(error.message).toMatch(/currently locked by another user and cannot be updated/)
}
const updatedPost = await payload.findByID({
collection: postsSlug,
id: newPost.id,
})
// Should not allow update - expect data not to change
expect(newPost.text).toEqual('some post')
expect(updatedPost.text).toEqual('some post')
})
it('should not allow update of locked document - global', async () => {
@@ -300,15 +308,20 @@ describe('Locked documents', () => {
data: {
globalText: 'updated global text',
},
overrideLock: false, // necessary to trigger the lock check
slug: menuSlug,
})
} catch (error) {
expect(error).toBeInstanceOf(APIError)
expect(error).toBeInstanceOf(Locked)
expect(error.message).toMatch(/currently locked by another user and cannot be updated/)
}
const updatedGlobalMenu = await payload.findGlobal({
slug: menuSlug,
})
// Should not allow update - expect data not to change
expect(menu.globalText).toEqual('global text')
expect(updatedGlobalMenu.globalText).toEqual('global text 2')
})
// Try to delete locked document (collection)
@@ -341,11 +354,21 @@ describe('Locked documents', () => {
await payload.delete({
collection: postsSlug,
id: newPost3.id,
overrideLock: false, // necessary to trigger the lock check
})
} catch (error) {
expect(error).toBeInstanceOf(APIError)
expect(error).toBeInstanceOf(Locked)
expect(error.message).toMatch(/currently locked and cannot be deleted/)
}
const findPostDocs = await payload.find({
collection: postsSlug,
where: {
id: { equals: newPost3.id },
},
})
expect(findPostDocs.docs).toHaveLength(1)
})
it('should allow delete of stale locked document - collection', async () => {
@@ -381,6 +404,7 @@ describe('Locked documents', () => {
await payload.delete({
collection: postsSlug,
id: newPost4.id,
overrideLock: false,
})
const findPostDocs = await payload.find({
@@ -411,4 +435,171 @@ describe('Locked documents', () => {
expect(docsFromLocksCollection.docs).toHaveLength(0)
})
it('should allow update of locked document w/ overrideLock flag - collection', async () => {
const newPost5 = await payload.create({
collection: postsSlug,
data: {
text: 'new post 5',
},
})
// Give locking ownership to another user
const lockedDocInstance = await payload.create({
collection: lockedDocumentCollection,
data: {
editedAt: new Date().toISOString(),
user: {
relationTo: 'users',
value: user2.id,
},
document: {
relationTo: 'posts',
value: newPost5.id,
},
globalSlug: undefined,
},
})
const updateLockedDoc = await payload.update({
collection: postsSlug,
data: {
text: 'updated post 5',
},
id: newPost5.id,
overrideLock: true,
})
// Should allow update since using overrideLock flag
expect(updateLockedDoc.text).toEqual('updated post 5')
// Check to make sure the document does not exist in payload-locked-documents anymore
try {
await payload.findByID({
collection: lockedDocumentCollection,
id: lockedDocInstance.id,
})
} catch (error) {
expect(error).toBeInstanceOf(NotFound)
}
const docsFromLocksCollection = await payload.find({
collection: lockedDocumentCollection,
where: {
id: { equals: lockedDocInstance.id },
},
})
// Updating a document with the local API should not keep a stored doc
// in the payload-locked-documents collection
expect(docsFromLocksCollection.docs).toHaveLength(0)
})
it('should allow update of locked document w/ overrideLock flag - global', async () => {
// Give locking ownership to another user
const lockedGlobalInstance = await payload.create({
collection: lockedDocumentCollection,
data: {
editedAt: new Date().toISOString(),
globalSlug: menuSlug,
user: {
relationTo: 'users',
value: user2.id,
},
document: undefined,
},
})
const updateGlobalLockedDoc = await payload.updateGlobal({
data: {
globalText: 'updated global text 2',
},
slug: menuSlug,
overrideLock: true,
})
// Should allow update since using overrideLock flag
expect(updateGlobalLockedDoc.globalText).toEqual('updated global text 2')
// Check to make sure the document does not exist in payload-locked-documents anymore
try {
await payload.findByID({
collection: lockedDocumentCollection,
id: lockedGlobalInstance.id,
})
} catch (error) {
expect(error).toBeInstanceOf(NotFound)
}
const docsFromLocksCollection = await payload.find({
collection: lockedDocumentCollection,
where: {
id: { equals: lockedGlobalInstance.id },
},
})
// Updating a document with the local API should not keep a stored doc
// in the payload-locked-documents collection
expect(docsFromLocksCollection.docs).toHaveLength(0)
})
it('should allow delete of locked document w/ overrideLock flag - collection', async () => {
const newPost6 = await payload.create({
collection: postsSlug,
data: {
text: 'new post 6',
},
})
// Give locking ownership to another user
const lockedDocInstance = await payload.create({
collection: lockedDocumentCollection,
data: {
editedAt: new Date().toISOString(),
user: {
relationTo: 'users',
value: user2.id,
},
document: {
relationTo: 'posts',
value: newPost6.id,
},
globalSlug: undefined,
},
})
await payload.delete({
collection: postsSlug,
id: newPost6.id,
overrideLock: true,
})
const findPostDocs = await payload.find({
collection: postsSlug,
where: {
id: { equals: newPost6.id },
},
})
expect(findPostDocs.docs).toHaveLength(0)
// Check to make sure the document does not exist in payload-locked-documents anymore
try {
await payload.findByID({
collection: lockedDocumentCollection,
id: lockedDocInstance.id,
})
} catch (error) {
expect(error).toBeInstanceOf(NotFound)
}
const docsFromLocksCollection = await payload.find({
collection: lockedDocumentCollection,
where: {
id: { equals: lockedDocInstance.id },
},
})
expect(docsFromLocksCollection.docs).toHaveLength(0)
})
})