diff --git a/packages/drizzle/src/createVersion.ts b/packages/drizzle/src/createVersion.ts index 7571f11178a..96a97c93115 100644 --- a/packages/drizzle/src/createVersion.ts +++ b/packages/drizzle/src/createVersion.ts @@ -76,9 +76,8 @@ export async function createVersion( sql: sql` UPDATE ${table} SET latest = false - WHERE ${table.id} != ${result.id} + WHERE ${table.id} < ${result.id} AND ${table.parent} = ${parent} - AND ${table.updatedAt} < ${result.updatedAt || updatedAt} `, }) } diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 43613174e55..9b3d594fa55 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -7,6 +7,8 @@ import { createLocalReq, getFileByPath, saveVersion, ValidationError } from 'pay import { wait } from 'payload/shared' import * as qs from 'qs-esm' import { fileURLToPath } from 'url' +import { sql } from 'drizzle-orm' +import toSnakeCase from 'to-snake-case' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import type { NextRESTClient } from '../__helpers/shared/NextRESTClient.js' @@ -31,8 +33,8 @@ import { draftWithUploadCloudStorageCollectionSlug, draftWithUploadCollectionSlug, localizedCollectionSlug, - nestedArraySelectCollectionSlug, localizedGlobalSlug, + nestedArraySelectCollectionSlug, versionCollectionSlug, } from './slugs.js' @@ -1930,6 +1932,7 @@ describe('Versions', () => { }, }) + expect(docs).toHaveLength(1) expect(docs[0]).toBeDefined() await cleanupDocuments({ @@ -1938,6 +1941,148 @@ describe('Versions', () => { }) }) + it('should not produce duplicate latest=true rows on same-second writes', async () => { + // Reproduces the bug from #17216: autosave writes within the same + // second used to keep both rows with latest=true because the + // updatedAt < guard was strict and couldn't match same-second rows. + const doc = await payload.create({ + collection: draftCollectionSlug, + data: { + description: 'test', + title: 'test', + }, + }) + + // Force two writes that share the same updatedAt timestamp + const now = new Date().toISOString() + await payload.update({ + id: doc.id, + collection: draftCollectionSlug, + data: { title: 'first' }, + draft: true, + updatedAt: now, + }) + await payload.update({ + id: doc.id, + collection: draftCollectionSlug, + data: { title: 'second' }, + draft: true, + updatedAt: now, + }) + + const { docs } = await payload.findVersions({ + collection: draftCollectionSlug, + where: { + and: [ + { + parent: { + equals: doc.id, + }, + }, + { + latest: { + equals: true, + }, + }, + ], + }, + }) + + expect(docs).toHaveLength(1) + + await cleanupDocuments({ + collectionSlugs: [draftCollectionSlug], + payload, + }) + }) + + it('should not lose latest=true when cleanup runs out of order after concurrent inserts', async () => { + // Reproduces the interleaving from sviat-barbutsa's review: + // Two concurrent writes can interleave their INSERT and cleanup UPDATE + // like this: + // A inserts id 1, B inserts id 2, B cleans up (keeps id 2), + // A cleans up (keeps id 1) → id 2 loses latest=true + // With id < result.id guard, A's cleanup only touches rows older than id 1, + // so id 2 is preserved. + const doc = await payload.create({ + collection: draftCollectionSlug, + data: { + description: 'test', + title: 'test', + }, + }) + + // Simulate the interleaving by inserting two version rows directly + // and then running the cleanup queries in the problematic order + const tableName = payload.db.tableNameMap.get( + `_${toSnakeCase(draftCollectionSlug)}${payload.db.versionsSuffix}`, + ) + const table = payload.db.tables[tableName] + + const now = new Date().toISOString() + + // Insert version A (simulates first concurrent write) + const resultA = await payload.db.drizzle + .insert(table) + .values({ + autosave: false, + createdAt: now, + latest: true, + parent: doc.id, + updatedAt: now, + version: { title: 'A' }, + }) + .returning() + .then((r) => r[0]) + + // Insert version B (simulates second concurrent write) + const resultB = await payload.db.drizzle + .insert(table) + .values({ + autosave: false, + createdAt: now, + latest: true, + parent: doc.id, + updatedAt: now, + version: { title: 'B' }, + }) + .returning() + .then((r) => r[0]) + + // B's cleanup runs first — keeps B, clears A + await payload.db.drizzle + .update(table) + .set({ latest: false }) + .where( + sql`${table.id} < ${resultB.id} AND ${table.parent} = ${doc.id}`, + ) + + // A's cleanup runs second — with id < result.id, it only touches + // rows older than A, so B is preserved + await payload.db.drizzle + .update(table) + .set({ latest: false }) + .where( + sql`${table.id} < ${resultA.id} AND ${table.parent} = ${doc.id}`, + ) + + // Verify: only B should have latest=true + const latestRows = await payload.db.drizzle + .select() + .from(table) + .where( + sql`${table.latest} = true AND ${table.parent} = ${doc.id}`, + ) + + expect(latestRows).toHaveLength(1) + expect(latestRows[0].id).toBe(resultB.id) + + await cleanupDocuments({ + collectionSlugs: [draftCollectionSlug], + payload, + }) + }) + it('should fall back to creating a new version when updateVersion fails due to a concurrent write', async () => { const doc = await payload.create({ collection: autosaveCollectionSlug,