github/directus
1
1
Fork 0
mirror of https://github.com/directus/directus.git synced 2026-04-25 03:00:53 -04:00
Code Issues Packages Projects Releases Wiki Activity

Remove nested transactions (#22023)

Browse Source 
Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Brainslug <br41nslug@users.noreply.github.com>
This commit is contained in:
Rijk van Zanten
2024-04-16 04:13:33 -04:00
committed by GitHub
parent 509fed7c4d
commit 577f08e5f5
11 changed files with 57 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/witty-lamps-worry.md Normal file
View File

@@ -0,0 +1,5 @@
---
'@directus/api': patch
---
Fixed various transaction related issues in CockroachDB by preventing transactions from being nested

15
api/src/services/collections.ts
View File

@@ -2,6 +2,7 @@ import { useEnv } from '@directus/env';
import { ForbiddenError, InvalidPayloadError } from '@directus/errors';
import type { SchemaInspector, Table } from '@directus/schema';
import { createInspector } from '@directus/schema';
import { systemCollectionRows, type BaseCollectionMeta } from '@directus/system-data';
import type { Accountability, FieldMeta, RawField, SchemaOverview } from '@directus/types';
import { addFieldFlag } from '@directus/utils';
import type Keyv from 'keyv';
@@ -16,9 +17,9 @@ import emitter from '../emitter.js';
import type { AbstractServiceOptions, ActionEventParams, Collection, MutationOptions } from '../types/index.js';
import { getSchema } from '../utils/get-schema.js';
import { shouldClearCache } from '../utils/should-clear-cache.js';
import { transaction } from '../utils/transaction.js';
import { FieldsService } from './fields.js';
import { ItemsService } from './items.js';
import { systemCollectionRows, type BaseCollectionMeta } from '@directus/system-data';
export type RawCollection = {
collection: string;
@@ -78,7 +79,7 @@ export class CollectionsService {
// Create the collection/fields in a transaction so it'll be reverted in case of errors or
// permission problems. This might not work reliably in MySQL, as it doesn't support DDL in
// transactions.
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
if (payload.schema) {
// Directus heavily relies on the primary key of a collection, so we have to make sure that
// every collection that is created has a primary key. If no primary key field is created
@@ -219,7 +220,7 @@ export class CollectionsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
const collections = await this.knex.transaction(async (trx) => {
const collections = await transaction(this.knex, async (trx) => {
const service = new CollectionsService({
schema: this.schema,
accountability: this.accountability,
@@ -466,7 +467,7 @@ export class CollectionsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const collectionItemsService = new CollectionsService({
knex: trx,
accountability: this.accountability,
@@ -521,7 +522,7 @@ export class CollectionsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const service = new CollectionsService({
schema: this.schema,
accountability: this.accountability,
@@ -578,7 +579,7 @@ export class CollectionsService {
throw new ForbiddenError();
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
if (collectionToBeDeleted!.schema) {
await trx.schema.dropTable(collectionKey);
}
@@ -705,7 +706,7 @@ export class CollectionsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const service = new CollectionsService({
schema: this.schema,
accountability: this.accountability,

3
api/src/services/extensions.ts
View File

@@ -9,6 +9,7 @@ import getDatabase from '../database/index.js';
import { getExtensionManager } from '../extensions/index.js';
import type { ExtensionManager } from '../extensions/manager.js';
import type { AbstractServiceOptions } from '../types/index.js';
import { transaction } from '../utils/transaction.js';
import { ItemsService } from './items.js';
export class ExtensionReadError extends Error {
@@ -195,7 +196,7 @@ export class ExtensionsService {
}
async updateOne(id: string, data: DeepPartial<ApiOutput>) {
const result = await this.knex.transaction(async (trx) => {
const result = await transaction(this.knex, async (trx) => {
if (!isObject(data.meta)) {
throw new InvalidPayloadError({ reason: `"meta" is required` });
}

7
api/src/services/fields.ts
View File

@@ -21,6 +21,7 @@ import getLocalType from '../utils/get-local-type.js';
import { getSchema } from '../utils/get-schema.js';
import { sanitizeColumn } from '../utils/sanitize-schema.js';
import { shouldClearCache } from '../utils/should-clear-cache.js';
import { transaction } from '../utils/transaction.js';
import { ItemsService } from './items.js';
import { PayloadService } from './payload.js';
import { RelationsService } from './relations.js';
@@ -282,7 +283,7 @@ export class FieldsService {
addFieldFlag(field, flagToAdd);
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const itemsService = new ItemsService('directus_fields', {
knex: trx,
accountability: this.accountability,
@@ -436,7 +437,7 @@ export class FieldsService {
if (!isEqual(columnToCompare, hookAdjustedField.schema)) {
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
await trx.schema.alterTable(collection, async (table) => {
if (!hookAdjustedField.schema) return;
this.addColumnToTable(table, field, existingColumn);
@@ -577,7 +578,7 @@ export class FieldsService {
);
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const relations = this.schema.relations.filter((relation) => {
return (
(relation.collection === collection && relation.field === field) ||

7
api/src/services/import-export.ts
View File

@@ -25,6 +25,7 @@ import emitter from '../emitter.js';
import { useLogger } from '../logger.js';
import type { AbstractServiceOptions, ActionEventParams } from '../types/index.js';
import { getDateFormatted } from '../utils/get-date-formatted.js';
import { transaction } from '../utils/transaction.js';
import { Url } from '../utils/url.js';
import { userName } from '../utils/user-name.js';
import { FilesService } from './files.js';
@@ -78,7 +79,7 @@ export class ImportService {
const extractJSON = StreamArray.withParser();
const nestedActionEvents: ActionEventParams[] = [];
return this.knex.transaction((trx) => {
return transaction(this.knex, (trx) => {
const service = new ItemsService(collection, {
knex: trx,
schema: this.schema,
@@ -126,7 +127,7 @@ export class ImportService {
const nestedActionEvents: ActionEventParams[] = [];
return this.knex.transaction((trx) => {
return transaction(this.knex, (trx) => {
const service = new ItemsService(collection, {
knex: trx,
schema: this.schema,
@@ -274,7 +275,7 @@ export class ExportService {
const database = getDatabase();
await database.transaction(async (trx) => {
await transaction(database, async (trx) => {
const service = new ItemsService(collection, {
accountability: this.accountability,
schema: this.schema,

15
api/src/services/items.ts
View File

@@ -22,6 +22,7 @@ import emitter from '../emitter.js';
import type { AbstractService, AbstractServiceOptions, ActionEventParams, MutationOptions } from '../types/index.js';
import getASTFromQuery from '../utils/get-ast-from-query.js';
import { shouldClearCache } from '../utils/should-clear-cache.js';
import { transaction } from '../utils/transaction.js';
import { validateKeys } from '../utils/validate-keys.js';
import { AuthorizationService } from './authorization.js';
import { PayloadService } from './payload.js';
@@ -119,7 +120,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
// changes in the DB if any of the parts contained within throws an error. This also means
// that any errors thrown in any nested relational changes will bubble up and cancel the whole
// update tree
const primaryKey: PrimaryKey = await this.knex.transaction(async (trx) => {
const primaryKey: PrimaryKey = await transaction(this.knex, async (trx) => {
// We're creating new services instances so they can use the transaction as their Knex interface
const payloadService = new PayloadService(this.collection, {
accountability: this.accountability,
@@ -344,11 +345,11 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
async createMany(data: Partial<Item>[], opts: MutationOptions = {}): Promise<PrimaryKey[]> {
if (!opts.mutationTracker) opts.mutationTracker = this.createMutationTracker();
const { primaryKeys, nestedActionEvents } = await this.knex.transaction(async (trx) => {
const { primaryKeys, nestedActionEvents } = await transaction(this.knex, async (knex) => {
const service = new ItemsService(this.collection, {
accountability: this.accountability,
schema: this.schema,
knex: trx,
knex: knex,
});
const primaryKeys: PrimaryKey[] = [];
@@ -560,7 +561,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
const keys: PrimaryKey[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const service = new ItemsService(this.collection, {
accountability: this.accountability,
knex: trx,
@@ -649,7 +650,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
throw opts.preMutationError;
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const payloadService = new PayloadService(this.collection, {
accountability: this.accountability,
knex: trx,
@@ -836,7 +837,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
async upsertMany(payloads: Partial<Item>[], opts: MutationOptions = {}): Promise<PrimaryKey[]> {
if (!opts.mutationTracker) opts.mutationTracker = this.createMutationTracker();
const primaryKeys = await this.knex.transaction(async (trx) => {
const primaryKeys = await transaction(this.knex, async (trx) => {
const service = new ItemsService(this.collection, {
accountability: this.accountability,
schema: this.schema,
@@ -927,7 +928,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
);
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
await trx(this.collection).whereIn(primaryKeyField, keys).delete();
if (this.accountability && this.schema.collections[this.collection]!.accountability !== null) {

7
api/src/services/relations.ts
View File

@@ -14,6 +14,7 @@ import emitter from '../emitter.js';
import type { AbstractServiceOptions, ActionEventParams, MutationOptions } from '../types/index.js';
import { getDefaultIndexName } from '../utils/get-default-index-name.js';
import { getSchema } from '../utils/get-schema.js';
import { transaction } from '../utils/transaction.js';
import { ItemsService, type QueryOptions } from './items.js';
import { PermissionsService } from './permissions/index.js';
@@ -191,7 +192,7 @@ export class RelationsService {
one_collection: relation.related_collection || null,
};
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
if (relation.related_collection) {
await trx.schema.alterTable(relation.collection!, async (table) => {
this.alterType(table, relation, fieldSchema.nullable);
@@ -290,7 +291,7 @@ export class RelationsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
if (existingRelation.related_collection) {
await trx.schema.alterTable(collection, async (table) => {
let constraintName: string = getDefaultIndexName('foreign', collection, field);
@@ -404,7 +405,7 @@ export class RelationsService {
const nestedActionEvents: ActionEventParams[] = [];
try {
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const existingConstraints = await this.schemaInspector.foreignKeys();
const constraintNames = existingConstraints.map((key) => key.constraint_name);

3
api/src/services/roles.ts
View File

@@ -2,6 +2,7 @@ import { ForbiddenError, InvalidPayloadError, UnprocessableContentError } from '
import type { Alterations, Item, PrimaryKey, Query, User } from '@directus/types';
import { getMatch } from 'ip-matching';
import type { AbstractServiceOptions, MutationOptions } from '../types/index.js';
import { transaction } from '../utils/transaction.js';
import { ItemsService } from './items.js';
import { PermissionsService } from './permissions/index.js';
import { PresetsService } from './presets.js';
@@ -263,7 +264,7 @@ export class RolesService extends ItemsService {
opts.preMutationError = err;
}
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const itemsService = new ItemsService('directus_roles', {
knex: trx,
accountability: this.accountability,

3
api/src/services/users.ts
View File

@@ -13,6 +13,7 @@ import type { AbstractServiceOptions, MutationOptions } from '../types/index.js'
import isUrlAllowed from '../utils/is-url-allowed.js';
import { verifyJWT } from '../utils/jwt.js';
import { stall } from '../utils/stall.js';
import { transaction } from '../utils/transaction.js';
import { Url } from '../utils/url.js';
import { ItemsService } from './items.js';
import { MailService } from './mail/index.js';
@@ -239,7 +240,7 @@ export class UsersService extends ItemsService {
const keys: PrimaryKey[] = [];
await this.knex.transaction(async (trx) => {
await transaction(this.knex, async (trx) => {
const service = new UsersService({
accountability: this.accountability,
knex: trx,

3
api/src/utils/apply-diff.ts
View File

@@ -20,6 +20,7 @@ import type {
SnapshotField,
} from '../types/index.js';
import { DiffKind } from '../types/index.js';
import { transaction } from '../utils/transaction.js';
import { getSchema } from './get-schema.js';
type CollectionDelta = {
@@ -48,7 +49,7 @@ export async function applyDiff(
const runPostColumnChange = await helpers.schema.preColumnChange();
await database.transaction(async (trx) => {
await transaction(database, async (trx) => {
const collectionsService = new CollectionsService({ knex: trx, schema });
const getNestedCollectionsToCreate = (currentLevelCollection: string) =>

16
api/src/utils/transaction.ts Normal file
View File

@@ -0,0 +1,16 @@
import type { Knex } from 'knex';
/**
* Execute the given handler within the current transaction or a newly created one
* if the current knex state isn't a transaction yet.
*
* Can be used to ensure the handler is run within a transaction,
* while preventing nested transactions.
*/
export const transaction = <T = unknown>(knex: Knex, handler: (knex: Knex) => Promise<T>): Promise<T> => {
if (knex.isTransaction) {
return handler(knex);
} else {
return knex.transaction((trx) => handler(trx));
}
};