feat(db-*, payload): better transactions (#7395)

## Description

### payload
- Removes calls to beginTransaction and commitTransaction from read
operations

### db-sqlite, db-postgres
- beginTransaction() options are passed through and used to create a
transaction
- declare module type adds beginTransaction with proper transaction
config args for postgres and sqlite
This commit is contained in:
Dan Ribbens
2024-07-29 15:35:19 -04:00
committed by GitHub
parent ada9978a8c
commit 354588898f
20 changed files with 17 additions and 88 deletions

View File

@@ -156,6 +156,7 @@ declare module 'payload' {
export interface DatabaseAdapter
extends Omit<Args, 'idType' | 'logger' | 'migrationDir' | 'pool'>,
DrizzleAdapter {
beginTransaction: (options?: PgTransactionConfig) => Promise<null | number | string>
drizzle: PostgresDB
enums: Record<string, GenericEnum>
/**

View File

@@ -144,6 +144,7 @@ declare module 'payload' {
export interface DatabaseAdapter
extends Omit<Args, 'idType' | 'logger' | 'migrationDir' | 'pool'>,
DrizzleAdapter {
beginTransaction: (options?: SQLiteTransactionConfig) => Promise<null | number | string>
drizzle: LibSQLDatabase
/**
* An object keyed on each table, with a key value pair where the constraint name is the key, followed by the dot-notation field name

View File

@@ -6,6 +6,7 @@ import type { DrizzleAdapter, DrizzleTransaction } from '../types.js'
export const beginTransaction: BeginTransaction = async function beginTransaction(
this: DrizzleAdapter,
options: DrizzleAdapter['transactionOptions'],
) {
let id
try {
@@ -41,7 +42,7 @@ export const beginTransaction: BeginTransaction = async function beginTransactio
}
transactionReady()
})
}, this.transactionOptions)
}, options || this.transactionOptions)
.catch(() => {
// swallow
})

View File

@@ -2,7 +2,6 @@ import type { AllOperations, PayloadRequest } from '../types/index.js'
import type { Permissions } from './types.js'
import { getEntityPolicies } from '../utilities/getEntityPolicies.js'
import isolateObjectProperty from '../utilities/isolateObjectProperty.js'
type GetAccessResultsArgs = {
req: PayloadRequest
@@ -45,9 +44,7 @@ export async function getAccessResults({ req }: GetAccessResultsArgs): Promise<P
type: 'collection',
entity: collection,
operations: collectionOperations,
// Do not re-use our existing req object, as we need a new req.transactionID. Cannot re-use our existing one, as this is run in parallel (Promise.all above) which is
// not supported on the same transaction ID. Not passing a transaction ID here creates a new transaction ID.
req: isolateObjectProperty(req, 'transactionID'),
req,
})
results.collections = {
...results.collections,
@@ -68,9 +65,7 @@ export async function getAccessResults({ req }: GetAccessResultsArgs): Promise<P
type: 'global',
entity: global,
operations: globalOperations,
// Do not re-use our existing req object, as we need a new req.transactionID. Cannot re-use our existing one, as this is run in parallel (Promise.all above) which is
// not supported on the same transaction ID. Not passing a transaction ID here creates a new transaction ID.
req: isolateObjectProperty(req, 'transactionID'),
req,
})
results.globals = {
...results.globals,

View File

@@ -1,8 +1,6 @@
import type { PayloadRequest } from '../../types/index.js'
import type { Permissions } from '../types.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import { adminInit as adminInitTelemetry } from '../../utilities/telemetry/events/adminInit.js'
import { getAccessResults } from '../getAccessResults.js'
@@ -17,10 +15,7 @@ export const accessOperation = async (args: Arguments): Promise<Permissions> =>
adminInitTelemetry(req)
try {
const shouldCommit = await initTransaction(req)
const results = getAccessResults({ req })
if (shouldCommit) await commitTransaction(req)
return results
return getAccessResults({ req })
} catch (e: unknown) {
await killTransaction(req)
throw e

View File

@@ -6,8 +6,6 @@ import type { Collection } from '../config/types.js'
import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import { buildAfterOperation } from './utils.js'
@@ -25,8 +23,6 @@ export const countOperation = async <TSlug extends CollectionSlug>(
let args = incomingArgs
try {
const shouldCommit = await initTransaction(args.req)
// /////////////////////////////////////
// beforeOperation - Collection
// /////////////////////////////////////
@@ -102,8 +98,6 @@ export const countOperation = async <TSlug extends CollectionSlug>(
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(args.req)

View File

@@ -1,7 +1,7 @@
import httpStatus from 'http-status'
import type { AccessResult } from '../../config/types.js'
import type { CollectionSlug, GeneratedTypes } from '../../index.js'
import type { CollectionSlug } from '../../index.js'
import type { PayloadRequest, Where } from '../../types/index.js'
import type { BeforeOperationHook, Collection, DataFromCollectionSlug } from '../config/types.js'

View File

@@ -1,4 +1,4 @@
import type { CollectionSlug, JsonObject, TypeWithID } from '../../index.js'
import type { CollectionSlug } from '../../index.js'
import type { PayloadRequest } from '../../types/index.js'
import type { BeforeOperationHook, Collection, DataFromCollectionSlug } from '../config/types.js'

View File

@@ -2,9 +2,7 @@ import type { CollectionPermission } from '../../auth/index.js'
import type { AllOperations, PayloadRequest } from '../../types/index.js'
import type { Collection } from '../config/types.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { getEntityPolicies } from '../../utilities/getEntityPolicies.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
const allOperations: AllOperations[] = ['create', 'read', 'update', 'delete']
@@ -37,8 +35,6 @@ export async function docAccessOperation(args: Arguments): Promise<CollectionPer
}
try {
const shouldCommit = await initTransaction(req)
const result = await getEntityPolicies({
id,
type: 'collection',
@@ -47,8 +43,6 @@ export async function docAccessOperation(args: Arguments): Promise<CollectionPer
req,
})
if (shouldCommit) await commitTransaction(req)
return result
} catch (e: unknown) {
await killTransaction(req)

View File

@@ -8,8 +8,6 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import { buildVersionCollectionFields } from '../../versions/buildCollectionFields.js'
import { appendVersionToQueryKey } from '../../versions/drafts/appendVersionToQueryKey.js'
@@ -38,8 +36,6 @@ export const findOperation = async <TSlug extends CollectionSlug>(
let args = incomingArgs
try {
const shouldCommit = await initTransaction(args.req)
// /////////////////////////////////////
// beforeOperation - Collection
// /////////////////////////////////////
@@ -253,8 +249,6 @@ export const findOperation = async <TSlug extends CollectionSlug>(
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(args.req)

View File

@@ -7,8 +7,6 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { NotFound } from '../../errors/index.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import replaceWithDraftIfAvailable from '../../versions/drafts/replaceWithDraftIfAvailable.js'
import { buildAfterOperation } from './utils.js'
@@ -31,8 +29,6 @@ export const findByIDOperation = async <TSlug extends CollectionSlug>(
let args = incomingArgs
try {
const shouldCommit = await initTransaction(args.req)
// /////////////////////////////////////
// beforeOperation - Collection
// /////////////////////////////////////
@@ -182,8 +178,6 @@ export const findByIDOperation = async <TSlug extends CollectionSlug>(
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(args.req)

View File

@@ -8,8 +8,6 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { APIError, Forbidden, NotFound } from '../../errors/index.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
export type Arguments = {
@@ -43,8 +41,6 @@ export const findVersionByIDOperation = async <TData extends TypeWithID = any>(
}
try {
const shouldCommit = await initTransaction(req)
// /////////////////////////////////////
// Access
// /////////////////////////////////////
@@ -141,8 +137,6 @@ export const findVersionByIDOperation = async <TData extends TypeWithID = any>(
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -7,8 +7,6 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import sanitizeInternalFields from '../../utilities/sanitizeInternalFields.js'
import { buildVersionCollectionFields } from '../../versions/buildCollectionFields.js'
@@ -44,8 +42,6 @@ export const findVersionsOperation = async <TData extends TypeWithVersion<TData>
} = args
try {
const shouldCommit = await initTransaction(req)
// /////////////////////////////////////
// Access
// /////////////////////////////////////
@@ -175,8 +171,6 @@ export const findVersionsOperation = async <TData extends TypeWithVersion<TData>
docs: result.docs.map((doc) => sanitizeInternalFields<TData>(doc)),
}
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -10,8 +10,6 @@ import { combineQueries } from '../../database/combineQueries.js'
import { APIError, Forbidden, NotFound } from '../../errors/index.js'
import { afterChange } from '../../fields/hooks/afterChange/index.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import { getLatestCollectionVersion } from '../../versions/getLatestCollectionVersion.js'
@@ -40,8 +38,6 @@ export const restoreVersionOperation = async <TData extends TypeWithID = any>(
} = args
try {
const shouldCommit = await initTransaction(req)
if (!id) {
throw new APIError('Missing ID of version to restore.', httpStatus.BAD_REQUEST)
}
@@ -200,8 +196,6 @@ export const restoreVersionOperation = async <TData extends TypeWithID = any>(
})) || result
}, Promise.resolve())
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -3,7 +3,7 @@ import type { DeepPartial } from 'ts-essentials'
import httpStatus from 'http-status'
import type { FindOneArgs } from '../../database/types.js'
import type { CollectionSlug, GeneratedTypes } from '../../index.js'
import type { CollectionSlug } from '../../index.js'
import type { PayloadRequest } from '../../types/index.js'
import type {
Collection,

View File

@@ -4,8 +4,6 @@ import type { SanitizedGlobalConfig } from '../config/types.js'
import executeAccess from '../../auth/executeAccess.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import replaceWithDraftIfAvailable from '../../versions/drafts/replaceWithDraftIfAvailable.js'
@@ -34,8 +32,6 @@ export const findOneOperation = async <T extends Record<string, unknown>>(
} = args
try {
const shouldCommit = await initTransaction(req)
// /////////////////////////////////////
// Retrieve and execute access
// /////////////////////////////////////
@@ -129,12 +125,6 @@ export const findOneOperation = async <T extends Record<string, unknown>>(
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
// /////////////////////////////////////
// Return results
// /////////////////////////////////////
return doc
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -7,9 +7,7 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { Forbidden, NotFound } from '../../errors/index.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { deepCopyObjectSimple } from '../../utilities/deepCopyObject.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
export type Arguments = {
@@ -39,8 +37,6 @@ export const findVersionByIDOperation = async <T extends TypeWithVersion<T> = an
} = args
try {
const shouldCommit = await initTransaction(req)
// /////////////////////////////////////
// Access
// /////////////////////////////////////
@@ -136,12 +132,6 @@ export const findVersionByIDOperation = async <T extends TypeWithVersion<T> = an
})) || result.version
}, Promise.resolve())
// /////////////////////////////////////
// Return results
// /////////////////////////////////////
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -7,8 +7,6 @@ import executeAccess from '../../auth/executeAccess.js'
import { combineQueries } from '../../database/combineQueries.js'
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
import { afterRead } from '../../fields/hooks/afterRead/index.js'
import { commitTransaction } from '../../utilities/commitTransaction.js'
import { initTransaction } from '../../utilities/initTransaction.js'
import { killTransaction } from '../../utilities/killTransaction.js'
import sanitizeInternalFields from '../../utilities/sanitizeInternalFields.js'
import { buildVersionGlobalFields } from '../../versions/buildGlobalFields.js'
@@ -44,8 +42,6 @@ export const findVersionsOperation = async <T extends TypeWithVersion<T>>(
const versionFields = buildVersionGlobalFields(globalConfig)
try {
const shouldCommit = await initTransaction(req)
// /////////////////////////////////////
// Access
// /////////////////////////////////////
@@ -147,8 +143,6 @@ export const findVersionsOperation = async <T extends TypeWithVersion<T>>(
docs: result.docs.map((doc) => sanitizeInternalFields<T>(doc)),
}
if (shouldCommit) await commitTransaction(req)
return result
} catch (error: unknown) {
await killTransaction(req)

View File

@@ -6,7 +6,11 @@ import type { PayloadRequest } from '../types/index.js'
export async function killTransaction(req: PayloadRequest): Promise<void> {
const { payload, transactionID } = req
if (transactionID && !(transactionID instanceof Promise)) {
await payload.db.rollbackTransaction(req.transactionID)
try {
await payload.db.rollbackTransaction(req.transactionID)
} catch (error) {
// swallow any errors while attempting to rollback
}
delete req.transactionID
}
}

View File

@@ -11,8 +11,8 @@ const Hooks: CollectionConfig = {
},
hooks: {
beforeOperation: [
({ req }) => {
if (!req.transactionID) {
({ req, operation }) => {
if (!req.transactionID && ['create', 'delete', 'update'].includes(operation)) {
throw new Error('transactionID is missing in beforeOperation hook')
}
},