Skip to content

Commit

Permalink
fix: latest: true version disappear on parallel writes (#9032)
Browse files Browse the repository at this point in the history
What?
Fixes issue when on parallel writes in result you can have 0 latest:
true versions.

Why?
There must be always a version with latest: true

How?
Ensures that we always have a version with latest: true by adding a
filter on createdAt < createdVersion.createdAt.
Instead, this ponentially can lead to a situation where we have 2
versions with latest: true, if they were created at the exact same time,
but this shouldn't happen in a real world scenario and it's much less
problematic than not having a version with latest: true.

Fixes #5895

Changes from #8986

---------

Co-authored-by: Sasha <64744993+r1tsuu@users.noreply.github.com>
  • Loading branch information
DanRibbens and r1tsuu authored Dec 2, 2024
1 parent c5fe021 commit 631edd4
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 1 deletion.
5 changes: 5 additions & 0 deletions packages/db-mongodb/src/createVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ export const createVersion: CreateVersion = async function createVersion(
$eq: true,
},
},
{
updatedAt: {
$lt: new Date(doc.updatedAt),
},
},
],
},
{ $unset: { latest: 1 } },
Expand Down
1 change: 1 addition & 0 deletions packages/drizzle/src/createVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export async function createVersion<T extends TypeWithID>(
SET latest = false
WHERE ${table.id} != ${result.id}
AND ${table.parent} = ${parent}
AND ${table.updatedAt} < ${result.updatedAt}
`,
})
}
Expand Down
109 changes: 108 additions & 1 deletion test/versions/int.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Payload } from 'payload'
import type { Payload, PayloadRequest } from 'payload'

import path from 'path'
import { ValidationError } from 'payload'
Expand Down Expand Up @@ -484,6 +484,57 @@ describe('Versions', () => {
})
})

it('should restore published version with correct data', async () => {
// create a post
const originalPost = await payload.create({
collection: draftCollectionSlug,
data: {
description: 'description',
title: 'v1',
_status: 'published',
},
})

// update the post
await payload.update({
collection: draftCollectionSlug,
draft: true,
id: originalPost.id,
data: {
title: 'v2',
_status: 'published',
},
})

// get the version id of the original draft
const versions = await payload.findVersions({
collection: draftCollectionSlug,
where: {
parent: {
equals: originalPost.id,
},
},
})

// restore the version
const versionToRestore = versions.docs[versions.docs.length - 1]
const restoredVersion = await payload.restoreVersion({
id: versionToRestore.id,
collection: draftCollectionSlug,
})

// get the latest draft
const latestDraft = await payload.findByID({
id: originalPost.id,
collection: draftCollectionSlug,
draft: true,
})

// assert it has the original post content
expect(latestDraft.title).toStrictEqual('v1')
expect(restoredVersion.title).toStrictEqual('v1')
})

describe('Update', () => {
it('should allow a draft to be patched', async () => {
const originalTitle = 'Here is a published post'
Expand Down Expand Up @@ -831,6 +882,62 @@ describe('Versions', () => {
expect(docs.totalDocs).toStrictEqual(2)
})
})

describe('Race conditions', () => {
it('should keep latest true with parallel writes', async () => {
const doc = await payload.create({
collection: 'draft-posts',
data: {
description: 'A',
title: 'A',
},
})

for (let i = 0; i < 200; i++) {
payload.logger.info(`try ${i}`)
const writeAmount = 3

const promises = Array.from({ length: writeAmount }, async (_, i) => {
return new Promise((resolve) => {
// Add latency so updates aren't immediate after each other but still in parallel
setTimeout(() => {
payload
.update({
id: doc.id,
collection: 'draft-posts',
data: {},
draft: true,
})
.then(resolve)
.catch(resolve)
}, i * 5)
})
})

await Promise.all(promises)

const { docs } = await payload.findVersions({
collection: 'draft-posts',
where: {
and: [
{
parent: {
equals: doc.id,
},
},
{
latest: {
equals: true,
},
},
],
},
})

expect(docs[0]).toBeDefined()
}
})
})
})

describe('Querying', () => {
Expand Down

0 comments on commit 631edd4

Please sign in to comment.