From bb911cc7eca1eeef15ade8eb043c0056c281e311 Mon Sep 17 00:00:00 2001 From: James Mikrut Date: Tue, 25 Jun 2024 14:57:50 -0400 Subject: [PATCH] fix(payload): #6800, #5108 - graphql query concurrency issues (#6857) Fixes #6800 and #5108 by improving the `isolateObjectProperty` utility function and flattening `req.transactionID` and `req.transactionIDPromise` to a single `req.transactionID` property. --- packages/db-mongodb/src/count.ts | 2 +- packages/db-mongodb/src/create.ts | 2 +- packages/db-mongodb/src/createGlobal.ts | 2 +- .../db-mongodb/src/createGlobalVersion.ts | 2 +- packages/db-mongodb/src/createVersion.ts | 2 +- packages/db-mongodb/src/deleteMany.ts | 2 +- packages/db-mongodb/src/deleteOne.ts | 2 +- packages/db-mongodb/src/deleteVersions.ts | 2 +- packages/db-mongodb/src/find.ts | 2 +- packages/db-mongodb/src/findGlobal.ts | 2 +- packages/db-mongodb/src/findGlobalVersions.ts | 2 +- packages/db-mongodb/src/findOne.ts | 2 +- packages/db-mongodb/src/findVersions.ts | 2 +- packages/db-mongodb/src/queryDrafts.ts | 2 +- .../src/transactions/commitTransaction.ts | 2 + .../src/transactions/rollbackTransaction.ts | 22 +++++--- packages/db-mongodb/src/updateGlobal.ts | 2 +- .../db-mongodb/src/updateGlobalVersion.ts | 2 +- packages/db-mongodb/src/updateOne.ts | 2 +- packages/db-mongodb/src/updateVersion.ts | 2 +- packages/db-mongodb/src/withSession.ts | 15 +++-- packages/db-postgres/src/count.ts | 2 +- packages/db-postgres/src/create.ts | 2 +- packages/db-postgres/src/createGlobal.ts | 2 +- .../db-postgres/src/createGlobalVersion.ts | 2 +- packages/db-postgres/src/createVersion.ts | 2 +- packages/db-postgres/src/deleteMany.ts | 2 +- packages/db-postgres/src/deleteOne.ts | 2 +- packages/db-postgres/src/deleteVersions.ts | 2 +- packages/db-postgres/src/find/findMany.ts | 2 +- .../src/transactions/commitTransaction.ts | 2 + .../src/transactions/rollbackTransaction.ts | 10 ++-- packages/db-postgres/src/update.ts | 2 +- packages/db-postgres/src/updateGlobal.ts | 2 +- .../db-postgres/src/updateGlobalVersion.ts | 2 +- packages/db-postgres/src/updateVersion.ts | 2 +- .../src/collections/graphql/resolvers/find.ts | 10 ++-- packages/payload/src/database/types.ts | 4 +- packages/payload/src/express/types.ts | 7 +-- .../payload/src/utilities/initTransaction.ts | 20 ++++--- .../src/utilities/isolateObjectProperty.ts | 26 ++++++--- .../payload/src/utilities/killTransaction.ts | 2 +- test/dataloader/config.ts | 55 +++++++++++++++++++ test/dataloader/int.spec.ts | 29 ++++++++++ 44 files changed, 191 insertions(+), 75 deletions(-) diff --git a/packages/db-mongodb/src/count.ts b/packages/db-mongodb/src/count.ts index 8ac820f9d4..f95ee03a2c 100644 --- a/packages/db-mongodb/src/count.ts +++ b/packages/db-mongodb/src/count.ts @@ -13,7 +13,7 @@ export const count: Count = async function count( { collection, locale, req = {} as PayloadRequest, where }, ) { const Model = this.collections[collection] - const options: QueryOptions = withSession(this, req.transactionID) + const options: QueryOptions = await withSession(this, req) let hasNearConstraint = false diff --git a/packages/db-mongodb/src/create.ts b/packages/db-mongodb/src/create.ts index 775caa07fa..c0f42863b5 100644 --- a/packages/db-mongodb/src/create.ts +++ b/packages/db-mongodb/src/create.ts @@ -11,7 +11,7 @@ export const create: Create = async function create( { collection, data, req = {} as PayloadRequest }, ) { const Model = this.collections[collection] - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) let doc try { ;[doc] = await Model.create([data], options) diff --git a/packages/db-mongodb/src/createGlobal.ts b/packages/db-mongodb/src/createGlobal.ts index 0f60460fae..691598b2ac 100644 --- a/packages/db-mongodb/src/createGlobal.ts +++ b/packages/db-mongodb/src/createGlobal.ts @@ -15,7 +15,7 @@ export const createGlobal: CreateGlobal = async function createGlobal( globalType: slug, ...data, } - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) let [result] = (await Model.create([global], options)) as any diff --git a/packages/db-mongodb/src/createGlobalVersion.ts b/packages/db-mongodb/src/createGlobalVersion.ts index 718ae918e9..32a03d38fa 100644 --- a/packages/db-mongodb/src/createGlobalVersion.ts +++ b/packages/db-mongodb/src/createGlobalVersion.ts @@ -11,7 +11,7 @@ export const createGlobalVersion: CreateGlobalVersion = async function createGlo { autosave, createdAt, globalSlug, parent, req = {} as PayloadRequest, updatedAt, versionData }, ) { const VersionModel = this.versions[globalSlug] - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) const [doc] = await VersionModel.create( [ diff --git a/packages/db-mongodb/src/createVersion.ts b/packages/db-mongodb/src/createVersion.ts index e26caf8290..163068f228 100644 --- a/packages/db-mongodb/src/createVersion.ts +++ b/packages/db-mongodb/src/createVersion.ts @@ -19,7 +19,7 @@ export const createVersion: CreateVersion = async function createVersion( }, ) { const VersionModel = this.versions[collectionSlug] - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) const [doc] = await VersionModel.create( [ diff --git a/packages/db-mongodb/src/deleteMany.ts b/packages/db-mongodb/src/deleteMany.ts index 5d4104f651..66801f626c 100644 --- a/packages/db-mongodb/src/deleteMany.ts +++ b/packages/db-mongodb/src/deleteMany.ts @@ -11,7 +11,7 @@ export const deleteMany: DeleteMany = async function deleteMany( ) { const Model = this.collections[collection] const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, } diff --git a/packages/db-mongodb/src/deleteOne.ts b/packages/db-mongodb/src/deleteOne.ts index 3bb4d8879a..25845364f2 100644 --- a/packages/db-mongodb/src/deleteOne.ts +++ b/packages/db-mongodb/src/deleteOne.ts @@ -12,7 +12,7 @@ export const deleteOne: DeleteOne = async function deleteOne( { collection, req = {} as PayloadRequest, where }, ) { const Model = this.collections[collection] - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) const query = await Model.buildQuery({ payload: this.payload, diff --git a/packages/db-mongodb/src/deleteVersions.ts b/packages/db-mongodb/src/deleteVersions.ts index 0f906139ec..d5150768af 100644 --- a/packages/db-mongodb/src/deleteVersions.ts +++ b/packages/db-mongodb/src/deleteVersions.ts @@ -11,7 +11,7 @@ export const deleteVersions: DeleteVersions = async function deleteVersions( ) { const VersionsModel = this.versions[collection] const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, } diff --git a/packages/db-mongodb/src/find.ts b/packages/db-mongodb/src/find.ts index a3dcac1366..65da0675c0 100644 --- a/packages/db-mongodb/src/find.ts +++ b/packages/db-mongodb/src/find.ts @@ -16,7 +16,7 @@ export const find: Find = async function find( ) { const Model = this.collections[collection] const collectionConfig = this.payload.collections[collection].config - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) let hasNearConstraint = false diff --git a/packages/db-mongodb/src/findGlobal.ts b/packages/db-mongodb/src/findGlobal.ts index 189db9a3b7..9fe72d6d60 100644 --- a/packages/db-mongodb/src/findGlobal.ts +++ b/packages/db-mongodb/src/findGlobal.ts @@ -14,7 +14,7 @@ export const findGlobal: FindGlobal = async function findGlobal( ) { const Model = this.globals const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, } diff --git a/packages/db-mongodb/src/findGlobalVersions.ts b/packages/db-mongodb/src/findGlobalVersions.ts index ac8e997679..68b2ae4fed 100644 --- a/packages/db-mongodb/src/findGlobalVersions.ts +++ b/packages/db-mongodb/src/findGlobalVersions.ts @@ -30,7 +30,7 @@ export const findGlobalVersions: FindGlobalVersions = async function findGlobalV this.payload.globals.config.find(({ slug }) => slug === global), ) const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), limit, skip, } diff --git a/packages/db-mongodb/src/findOne.ts b/packages/db-mongodb/src/findOne.ts index 7d8f9bbe23..a5235196c3 100644 --- a/packages/db-mongodb/src/findOne.ts +++ b/packages/db-mongodb/src/findOne.ts @@ -14,7 +14,7 @@ export const findOne: FindOne = async function findOne( ) { const Model = this.collections[collection] const options: MongooseQueryOptions = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, } diff --git a/packages/db-mongodb/src/findVersions.ts b/packages/db-mongodb/src/findVersions.ts index 20636c0e51..c07a34b096 100644 --- a/packages/db-mongodb/src/findVersions.ts +++ b/packages/db-mongodb/src/findVersions.ts @@ -27,7 +27,7 @@ export const findVersions: FindVersions = async function findVersions( const Model = this.versions[collection] const collectionConfig = this.payload.collections[collection].config const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), limit, skip, } diff --git a/packages/db-mongodb/src/queryDrafts.ts b/packages/db-mongodb/src/queryDrafts.ts index b77e5b97d6..65c5774c2b 100644 --- a/packages/db-mongodb/src/queryDrafts.ts +++ b/packages/db-mongodb/src/queryDrafts.ts @@ -16,7 +16,7 @@ export const queryDrafts: QueryDrafts = async function queryDrafts( ) { const VersionModel = this.versions[collection] const collectionConfig = this.payload.collections[collection].config - const options = withSession(this, req.transactionID) + const options = await withSession(this, req) let hasNearConstraint let sort diff --git a/packages/db-mongodb/src/transactions/commitTransaction.ts b/packages/db-mongodb/src/transactions/commitTransaction.ts index 13ff010518..db6bdbb66d 100644 --- a/packages/db-mongodb/src/transactions/commitTransaction.ts +++ b/packages/db-mongodb/src/transactions/commitTransaction.ts @@ -1,6 +1,8 @@ import type { CommitTransaction } from 'payload/database' export const commitTransaction: CommitTransaction = async function commitTransaction(id) { + if (id instanceof Promise) return + if (!this.sessions[id]?.inTransaction()) { return } diff --git a/packages/db-mongodb/src/transactions/rollbackTransaction.ts b/packages/db-mongodb/src/transactions/rollbackTransaction.ts index 8875b194d2..ef9293a9e3 100644 --- a/packages/db-mongodb/src/transactions/rollbackTransaction.ts +++ b/packages/db-mongodb/src/transactions/rollbackTransaction.ts @@ -1,27 +1,35 @@ import type { RollbackTransaction } from 'payload/database' export const rollbackTransaction: RollbackTransaction = async function rollbackTransaction( - id = '', + incomingID = '', ) { + let transactionID: number | string + + if (incomingID instanceof Promise) { + transactionID = await incomingID + } else { + transactionID = incomingID + } + // if multiple operations are using the same transaction, the first will flow through and delete the session. // subsequent calls should be ignored. - if (!this.sessions[id]) { + if (!this.sessions[transactionID]) { return } // when session exists but is not inTransaction something unexpected is happening to the session - if (!this.sessions[id].inTransaction()) { + if (!this.sessions[transactionID].inTransaction()) { this.payload.logger.warn('rollbackTransaction called when no transaction exists') - delete this.sessions[id] + delete this.sessions[transactionID] return } // the first call for rollback should be aborted and deleted causing any other operations with the same transaction to fail try { - await this.sessions[id].abortTransaction() - await this.sessions[id].endSession() + await this.sessions[transactionID].abortTransaction() + await this.sessions[transactionID].endSession() } catch (error) { // ignore the error as it is likely a race condition from multiple errors } - delete this.sessions[id] + delete this.sessions[transactionID] } diff --git a/packages/db-mongodb/src/updateGlobal.ts b/packages/db-mongodb/src/updateGlobal.ts index 7d9fe862dc..e6e67dad1f 100644 --- a/packages/db-mongodb/src/updateGlobal.ts +++ b/packages/db-mongodb/src/updateGlobal.ts @@ -12,7 +12,7 @@ export const updateGlobal: UpdateGlobal = async function updateGlobal( ) { const Model = this.globals const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, new: true, } diff --git a/packages/db-mongodb/src/updateGlobalVersion.ts b/packages/db-mongodb/src/updateGlobalVersion.ts index bf5f5a9019..ee421c5937 100644 --- a/packages/db-mongodb/src/updateGlobalVersion.ts +++ b/packages/db-mongodb/src/updateGlobalVersion.ts @@ -19,7 +19,7 @@ export async function updateGlobalVersion( const VersionModel = this.versions[global] const whereToUse = where || { id: { equals: id } } const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, new: true, } diff --git a/packages/db-mongodb/src/updateOne.ts b/packages/db-mongodb/src/updateOne.ts index 5577bea20a..370ad11dc6 100644 --- a/packages/db-mongodb/src/updateOne.ts +++ b/packages/db-mongodb/src/updateOne.ts @@ -14,7 +14,7 @@ export const updateOne: UpdateOne = async function updateOne( const where = id ? { id: { equals: id } } : whereArg const Model = this.collections[collection] const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, new: true, } diff --git a/packages/db-mongodb/src/updateVersion.ts b/packages/db-mongodb/src/updateVersion.ts index a4d0cc3d57..f1b8377c10 100644 --- a/packages/db-mongodb/src/updateVersion.ts +++ b/packages/db-mongodb/src/updateVersion.ts @@ -12,7 +12,7 @@ export const updateVersion: UpdateVersion = async function updateVersion( const VersionModel = this.versions[collection] const whereToUse = where || { id: { equals: id } } const options = { - ...withSession(this, req.transactionID), + ...(await withSession(this, req)), lean: true, new: true, } diff --git a/packages/db-mongodb/src/withSession.ts b/packages/db-mongodb/src/withSession.ts index d7aff267f1..178073fa5f 100644 --- a/packages/db-mongodb/src/withSession.ts +++ b/packages/db-mongodb/src/withSession.ts @@ -1,4 +1,5 @@ import type { ClientSession } from 'mongoose' +import type { PayloadRequest } from 'payload/types' import type { MongooseAdapter } from './index' @@ -6,9 +7,15 @@ import type { MongooseAdapter } from './index' * returns the session belonging to the transaction of the req.session if exists * @returns ClientSession */ -export function withSession( +export async function withSession( db: MongooseAdapter, - transactionID?: number | string, -): { session: ClientSession } | object { - return db.sessions[transactionID] ? { session: db.sessions[transactionID] } : {} + req: PayloadRequest, +): Promise<{ session: ClientSession } | object> { + let transactionID = req.transactionID + + if (transactionID instanceof Promise) { + transactionID = await req.transactionID + } + + if (req) return db.sessions[transactionID] ? { session: db.sessions[transactionID] } : {} } diff --git a/packages/db-postgres/src/count.ts b/packages/db-postgres/src/count.ts index 32569466c0..39a75a6a11 100644 --- a/packages/db-postgres/src/count.ts +++ b/packages/db-postgres/src/count.ts @@ -18,7 +18,7 @@ export const count: Count = async function count( const tableName = this.tableNameMap.get(toSnakeCase(collectionConfig.slug)) - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const table = this.tables[tableName] const { joinAliases, joins, where } = await buildQuery({ diff --git a/packages/db-postgres/src/create.ts b/packages/db-postgres/src/create.ts index c775eeb8bf..7888c61150 100644 --- a/packages/db-postgres/src/create.ts +++ b/packages/db-postgres/src/create.ts @@ -9,7 +9,7 @@ export const create: Create = async function create( this: PostgresAdapter, { collection: collectionSlug, data, req }, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collection = this.payload.collections[collectionSlug].config const tableName = this.tableNameMap.get(toSnakeCase(collection.slug)) diff --git a/packages/db-postgres/src/createGlobal.ts b/packages/db-postgres/src/createGlobal.ts index ae3dc69426..794c61302c 100644 --- a/packages/db-postgres/src/createGlobal.ts +++ b/packages/db-postgres/src/createGlobal.ts @@ -11,7 +11,7 @@ export async function createGlobal( this: PostgresAdapter, { slug, data, req = {} as PayloadRequest }: CreateGlobalArgs, ): Promise { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const globalConfig = this.payload.globals.config.find((config) => config.slug === slug) const tableName = this.tableNameMap.get(toSnakeCase(globalConfig.slug)) diff --git a/packages/db-postgres/src/createGlobalVersion.ts b/packages/db-postgres/src/createGlobalVersion.ts index d9d353584f..2db4d97797 100644 --- a/packages/db-postgres/src/createGlobalVersion.ts +++ b/packages/db-postgres/src/createGlobalVersion.ts @@ -14,7 +14,7 @@ export async function createGlobalVersion( this: PostgresAdapter, { autosave, globalSlug, req = {} as PayloadRequest, versionData }: CreateGlobalVersionArgs, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const global = this.payload.globals.config.find(({ slug }) => slug === globalSlug) const tableName = this.tableNameMap.get(`_${toSnakeCase(global.slug)}${this.versionsSuffix}`) diff --git a/packages/db-postgres/src/createVersion.ts b/packages/db-postgres/src/createVersion.ts index d030cfcc88..9f78ddfa4f 100644 --- a/packages/db-postgres/src/createVersion.ts +++ b/packages/db-postgres/src/createVersion.ts @@ -19,7 +19,7 @@ export async function createVersion( versionData, }: CreateVersionArgs, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collection = this.payload.collections[collectionSlug].config const defaultTableName = toSnakeCase(collection.slug) diff --git a/packages/db-postgres/src/deleteMany.ts b/packages/db-postgres/src/deleteMany.ts index 0056b56e4a..60152faf21 100644 --- a/packages/db-postgres/src/deleteMany.ts +++ b/packages/db-postgres/src/deleteMany.ts @@ -12,7 +12,7 @@ export const deleteMany: DeleteMany = async function deleteMany( this: PostgresAdapter, { collection, req = {} as PayloadRequest, where }, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collectionConfig = this.payload.collections[collection].config const tableName = this.tableNameMap.get(toSnakeCase(collectionConfig.slug)) diff --git a/packages/db-postgres/src/deleteOne.ts b/packages/db-postgres/src/deleteOne.ts index 2e43e102fa..7f0b311b9a 100644 --- a/packages/db-postgres/src/deleteOne.ts +++ b/packages/db-postgres/src/deleteOne.ts @@ -15,7 +15,7 @@ export const deleteOne: DeleteOne = async function deleteOne( this: PostgresAdapter, { collection: collectionSlug, req = {} as PayloadRequest, where: whereArg }, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collection = this.payload.collections[collectionSlug].config const tableName = this.tableNameMap.get(toSnakeCase(collection.slug)) diff --git a/packages/db-postgres/src/deleteVersions.ts b/packages/db-postgres/src/deleteVersions.ts index 32c608f6a7..311f7f2fb7 100644 --- a/packages/db-postgres/src/deleteVersions.ts +++ b/packages/db-postgres/src/deleteVersions.ts @@ -13,7 +13,7 @@ export const deleteVersions: DeleteVersions = async function deleteVersion( this: PostgresAdapter, { collection, locale, req = {} as PayloadRequest, where: where }, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collectionConfig: SanitizedCollectionConfig = this.payload.collections[collection].config const tableName = this.tableNameMap.get( diff --git a/packages/db-postgres/src/find/findMany.ts b/packages/db-postgres/src/find/findMany.ts index 5aebd8e216..b998631dd6 100644 --- a/packages/db-postgres/src/find/findMany.ts +++ b/packages/db-postgres/src/find/findMany.ts @@ -31,7 +31,7 @@ export const findMany = async function find({ tableName, where: whereArg, }: Args) { - const db = adapter.sessions[req.transactionID]?.db || adapter.drizzle + const db = adapter.sessions[await req.transactionID]?.db || adapter.drizzle const table = adapter.tables[tableName] const limit = limitArg ?? 10 diff --git a/packages/db-postgres/src/transactions/commitTransaction.ts b/packages/db-postgres/src/transactions/commitTransaction.ts index cc8a0a2ab6..60230ffd08 100644 --- a/packages/db-postgres/src/transactions/commitTransaction.ts +++ b/packages/db-postgres/src/transactions/commitTransaction.ts @@ -1,6 +1,8 @@ import type { CommitTransaction } from 'payload/database' export const commitTransaction: CommitTransaction = async function commitTransaction(id) { + if (id instanceof Promise) return + // if the session was deleted it has already been aborted if (!this.sessions[id]) { return diff --git a/packages/db-postgres/src/transactions/rollbackTransaction.ts b/packages/db-postgres/src/transactions/rollbackTransaction.ts index d3f0471d9d..f8aee16b0a 100644 --- a/packages/db-postgres/src/transactions/rollbackTransaction.ts +++ b/packages/db-postgres/src/transactions/rollbackTransaction.ts @@ -1,17 +1,19 @@ import type { RollbackTransaction } from 'payload/database' export const rollbackTransaction: RollbackTransaction = async function rollbackTransaction( - id = '', + incomingID = '', ) { + const transactionID = incomingID instanceof Promise ? await incomingID : incomingID + // if multiple operations are using the same transaction, the first will flow through and delete the session. // subsequent calls should be ignored. - if (!this.sessions[id]) { + if (!this.sessions[transactionID]) { return } // end the session promise in failure by calling reject - await this.sessions[id].reject() + await this.sessions[transactionID].reject() // delete the session causing any other operations with the same transaction to fail - delete this.sessions[id] + delete this.sessions[transactionID] } diff --git a/packages/db-postgres/src/update.ts b/packages/db-postgres/src/update.ts index fce524acd1..db8bdf8225 100644 --- a/packages/db-postgres/src/update.ts +++ b/packages/db-postgres/src/update.ts @@ -12,7 +12,7 @@ export const updateOne: UpdateOne = async function updateOne( this: PostgresAdapter, { id, collection: collectionSlug, data, draft, locale, req, where: whereArg }, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collection = this.payload.collections[collectionSlug].config const tableName = this.tableNameMap.get(toSnakeCase(collection.slug)) const whereToUse = whereArg || { id: { equals: id } } diff --git a/packages/db-postgres/src/updateGlobal.ts b/packages/db-postgres/src/updateGlobal.ts index ae6febd6f0..a05b8ef647 100644 --- a/packages/db-postgres/src/updateGlobal.ts +++ b/packages/db-postgres/src/updateGlobal.ts @@ -11,7 +11,7 @@ export async function updateGlobal( this: PostgresAdapter, { slug, data, req = {} as PayloadRequest }: UpdateGlobalArgs, ): Promise { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const globalConfig = this.payload.globals.config.find((config) => config.slug === slug) const tableName = this.tableNameMap.get(toSnakeCase(globalConfig.slug)) diff --git a/packages/db-postgres/src/updateGlobalVersion.ts b/packages/db-postgres/src/updateGlobalVersion.ts index 599ec3cb0e..1fd23650e3 100644 --- a/packages/db-postgres/src/updateGlobalVersion.ts +++ b/packages/db-postgres/src/updateGlobalVersion.ts @@ -20,7 +20,7 @@ export async function updateGlobalVersion( where: whereArg, }: UpdateGlobalVersionArgs, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const globalConfig: SanitizedGlobalConfig = this.payload.globals.config.find( ({ slug }) => slug === global, ) diff --git a/packages/db-postgres/src/updateVersion.ts b/packages/db-postgres/src/updateVersion.ts index 08bb780c57..1cfad79f64 100644 --- a/packages/db-postgres/src/updateVersion.ts +++ b/packages/db-postgres/src/updateVersion.ts @@ -20,7 +20,7 @@ export async function updateVersion( where: whereArg, }: UpdateVersionArgs, ) { - const db = this.sessions[req.transactionID]?.db || this.drizzle + const db = this.sessions[await req.transactionID]?.db || this.drizzle const collectionConfig: SanitizedCollectionConfig = this.payload.collections[collection].config const whereToUse = whereArg || { id: { equals: id } } const tableName = this.tableNameMap.get( diff --git a/packages/payload/src/collections/graphql/resolvers/find.ts b/packages/payload/src/collections/graphql/resolvers/find.ts index 039eaa1d58..8038ee6a93 100644 --- a/packages/payload/src/collections/graphql/resolvers/find.ts +++ b/packages/payload/src/collections/graphql/resolvers/find.ts @@ -31,8 +31,8 @@ export default function findResolver(collection: Collection): Resolver { let { req } = context const locale = req.locale const fallbackLocale = req.fallbackLocale - req = isolateObjectProperty(req, 'locale') - req = isolateObjectProperty(req, 'fallbackLocale') + + req = isolateObjectProperty(req, ['locale', 'fallbackLocale', 'transactionID']) req.locale = args.locale || locale req.fallbackLocale = args.fallbackLocale || fallbackLocale if (!req.query) req.query = {} @@ -41,8 +41,8 @@ export default function findResolver(collection: Collection): Resolver { args.draft ?? req.query?.draft === 'false' ? false : req.query?.draft === 'true' - ? true - : undefined + ? true + : undefined if (typeof draft === 'boolean') req.query.draft = String(draft) context.req = req @@ -53,7 +53,7 @@ export default function findResolver(collection: Collection): Resolver { draft: args.draft, limit: args.limit, page: args.page, - req: isolateObjectProperty(req, 'transactionID'), + req, sort: args.sort, where: args.where, } diff --git a/packages/payload/src/database/types.ts b/packages/payload/src/database/types.ts index d215c13db1..b60f1f2d2c 100644 --- a/packages/payload/src/database/types.ts +++ b/packages/payload/src/database/types.ts @@ -157,9 +157,9 @@ export type BeginTransaction = ( options?: Record, ) => Promise -export type RollbackTransaction = (id: number | string) => Promise +export type RollbackTransaction = (id: number | string | Promise) => Promise -export type CommitTransaction = (id: number | string) => Promise +export type CommitTransaction = (id: number | string | Promise) => Promise export type QueryDraftsArgs = { collection: string diff --git a/packages/payload/src/express/types.ts b/packages/payload/src/express/types.ts index 9bb44fe86d..ec1ffb6af4 100644 --- a/packages/payload/src/express/types.ts +++ b/packages/payload/src/express/types.ts @@ -57,12 +57,9 @@ export declare type PayloadRequest = Request & { t: TFunction /** * Identifier for the database transaction for interactions in a single, all-or-nothing operation. + * Can also be used to ensure consistency when multiple operations try to create a transaction concurrently on the same request. */ - transactionID?: number | string - /** - * Used to ensure consistency when multiple operations try to create a transaction concurrently on the same request - */ - transactionIDPromise?: Promise + transactionID?: number | string | Promise /** The signed in user */ user: (U & User) | null } diff --git a/packages/payload/src/utilities/initTransaction.ts b/packages/payload/src/utilities/initTransaction.ts index b23b8f39f8..f2c4144894 100644 --- a/packages/payload/src/utilities/initTransaction.ts +++ b/packages/payload/src/utilities/initTransaction.ts @@ -5,25 +5,27 @@ import type { PayloadRequest } from '../express/types' * @returns true if beginning a transaction and false when req already has a transaction to use */ export async function initTransaction(req: PayloadRequest): Promise { - const { payload, transactionID, transactionIDPromise } = req + const { payload, transactionID } = req + if (transactionID instanceof Promise) { + // wait for whoever else is already creating the transaction + await transactionID + return false + } + if (transactionID) { // we already have a transaction, we're not in charge of committing it return false } - if (transactionIDPromise) { - // wait for whoever else is already creating the transaction - await transactionIDPromise - return false - } if (typeof payload.db.beginTransaction === 'function') { // create a new transaction - req.transactionIDPromise = payload.db.beginTransaction().then((transactionID) => { + req.transactionID = payload.db.beginTransaction().then((transactionID) => { if (transactionID) { req.transactionID = transactionID } - delete req.transactionIDPromise + + return transactionID }) - await req.transactionIDPromise + await req.transactionID return !!req.transactionID } return false diff --git a/packages/payload/src/utilities/isolateObjectProperty.ts b/packages/payload/src/utilities/isolateObjectProperty.ts index 9e077d40f3..f016a3aa18 100644 --- a/packages/payload/src/utilities/isolateObjectProperty.ts +++ b/packages/payload/src/utilities/isolateObjectProperty.ts @@ -1,20 +1,32 @@ /** * Creates a proxy for the given object that has its own property */ -export default function isolateObjectProperty(object: T, key: keyof T): T { - const delegate = {} - const handler = { +export default function isolateObjectProperty( + object: T, + key: (keyof T)[] | keyof T, +): T { + const keys = Array.isArray(key) ? key : [key] + const delegate = {} as T + + // Initialize delegate with the keys, if they exist in the original object + for (const k of keys) { + if (k in object) { + delegate[k] = object[k] + } + } + + const handler: ProxyHandler = { deleteProperty(target, p): boolean { - return Reflect.deleteProperty(p === key ? delegate : target, p) + return Reflect.deleteProperty(keys.includes(p as keyof T) ? delegate : target, p) }, get(target, p, receiver) { - return Reflect.get(p === key ? delegate : target, p, receiver) + return Reflect.get(keys.includes(p as keyof T) ? delegate : target, p, receiver) }, has(target, p) { - return Reflect.has(p === key ? delegate : target, p) + return Reflect.has(keys.includes(p as keyof T) ? delegate : target, p) }, set(target, p, newValue, receiver) { - if (p === key) { + if (keys.includes(p as keyof T)) { // in case of transactionID we must ignore any receiver, because // "If provided and target does not have a setter for propertyKey, the property will be set on receiver instead." return Reflect.set(delegate, p, newValue) diff --git a/packages/payload/src/utilities/killTransaction.ts b/packages/payload/src/utilities/killTransaction.ts index e4d6c09189..bdf055e5b8 100644 --- a/packages/payload/src/utilities/killTransaction.ts +++ b/packages/payload/src/utilities/killTransaction.ts @@ -5,7 +5,7 @@ import type { PayloadRequest } from '../express/types' */ export async function killTransaction(req: PayloadRequest): Promise { const { payload, transactionID } = req - if (transactionID) { + if (transactionID && !(transactionID instanceof Promise)) { await payload.db.rollbackTransaction(req.transactionID) delete req.transactionID } diff --git a/test/dataloader/config.ts b/test/dataloader/config.ts index 8c083a3990..3e6ecc6fbd 100644 --- a/test/dataloader/config.ts +++ b/test/dataloader/config.ts @@ -57,6 +57,48 @@ export default buildConfigWithDefaults({ }, ], }, + { + fields: [ + { + name: 'name', + type: 'text', + }, + { + name: 'items', + type: 'relationship', + relationTo: 'items', + hasMany: true, + }, + ], + slug: 'shops', + access: { read: () => true }, + }, + { + fields: [ + { + name: 'name', + type: 'text', + }, + { + name: 'itemTags', + type: 'relationship', + relationTo: 'itemTags', + hasMany: true, + }, + ], + slug: 'items', + access: { read: () => true }, + }, + { + fields: [ + { + name: 'name', + type: 'text', + }, + ], + slug: 'itemTags', + access: { read: () => true }, + }, ], onInit: async (payload) => { const user = await payload.create({ @@ -72,6 +114,19 @@ export default buildConfigWithDefaults({ collection: 'posts', data: postDoc, }) + + const tag = await payload.create({ + collection: 'itemTags', + data: { name: 'tag1' }, + }) + const item = await payload.create({ + collection: 'items', + data: { name: 'item1', itemTags: [tag.id] }, + }) + const shop = await payload.create({ + collection: 'shops', + data: { name: 'shop1', items: [item.id] }, + }) }, }) diff --git a/test/dataloader/int.spec.ts b/test/dataloader/int.spec.ts index 523a7cfcba..835a87fdda 100644 --- a/test/dataloader/int.spec.ts +++ b/test/dataloader/int.spec.ts @@ -31,6 +31,35 @@ describe('dataloader', () => { if (loginResult.token) token = loginResult.token }) + it('should allow multiple parallel queries', async () => { + for (let i = 0; i < 100; i++) { + const query = ` + query { + Shops { + docs { + name + items { + name + } + } + } + Items { + docs { + name + itemTags { + name + } + } + } + }` + const response = await client.request(query) + expect(response).toStrictEqual({ + Shops: { docs: [{ name: 'shop1', items: [{ name: 'item1' }] }] }, + Items: { docs: [{ name: 'item1', itemTags: [{ name: 'tag1' }] }] }, + }) + } + }) + it('should allow querying via graphql', async () => { const query = `query { Posts {