mirror of
https://github.com/Unleash/unleash.git
synced 2025-01-31 00:16:47 +01:00
chore: handle transactions already started at the controller layer (#4953)
## About the changes This PR adds a method to safeguard us from opening a new transaction while inside another transaction, resulting in two isolated transactions that will not be atomic (if one fails, the other might still complete successfully).bbbe4d4637/lib/knex-builder/make-knex.js (L143C5-L144C88)
We're currently opening transactions at the controller layer2746bd1517/src/lib/features/export-import-toggles/export-import-controller.ts (L206-L208)
but in some other places, we do it at the store level:2746bd1517/src/lib/db/access-store.ts (L577)
## Alternative We can remove store-level transactions and move them to the controller following this approach:cb034976b9/src/lib/services/index.ts (L282-L284)
cb034976b9/src/lib/features/export-import-toggles/export-import-controller.ts (L206-L208)
This option is more expensive because we have to: 1. Write the factory methods that propagate the transaction to the stores (therefore creating the store factory methods as well) 2. Identify the methods for creating the transactions at the store level and backtrack the calls until the controller layer
This commit is contained in:
parent
2746bd1517
commit
52fa872fe6
@ -25,6 +25,7 @@ import {
|
||||
NamePermissionRef,
|
||||
PermissionRef,
|
||||
} from 'lib/services/access-service';
|
||||
import { inTransaction } from './transaction';
|
||||
|
||||
const T = {
|
||||
ROLE_USER: 'role_user',
|
||||
@ -574,7 +575,7 @@ export class AccessStore implements IAccessStore {
|
||||
};
|
||||
});
|
||||
|
||||
await this.db.transaction(async (tx) => {
|
||||
await inTransaction(this.db, async (tx) => {
|
||||
if (userRows.length > 0) {
|
||||
await tx(T.ROLE_USER)
|
||||
.insert(userRows)
|
||||
@ -620,7 +621,7 @@ export class AccessStore implements IAccessStore {
|
||||
})),
|
||||
);
|
||||
|
||||
await this.db.transaction(async (tx) => {
|
||||
await inTransaction(this.db, async (tx) => {
|
||||
if (groupRows.length > 0) {
|
||||
await tx(T.GROUP_ROLE)
|
||||
.insert(groupRows)
|
||||
@ -656,7 +657,7 @@ export class AccessStore implements IAccessStore {
|
||||
role_id: role,
|
||||
}));
|
||||
|
||||
await this.db.transaction(async (tx) => {
|
||||
await inTransaction(this.db, async (tx) => {
|
||||
await tx(T.ROLE_USER)
|
||||
.where('project', projectId)
|
||||
.andWhere('user_id', userId)
|
||||
@ -707,7 +708,7 @@ export class AccessStore implements IAccessStore {
|
||||
created_by: createdBy,
|
||||
}));
|
||||
|
||||
await this.db.transaction(async (tx) => {
|
||||
await inTransaction(this.db, async (tx) => {
|
||||
await tx(T.GROUP_ROLE)
|
||||
.where('project', projectId)
|
||||
.andWhere('group_id', groupId)
|
||||
|
@ -12,6 +12,7 @@ import {
|
||||
} from '../types/models/api-token';
|
||||
import { ALL_PROJECTS } from '../util/constants';
|
||||
import { Db } from './db';
|
||||
import { inTransaction } from './transaction';
|
||||
|
||||
const TABLE = 'api_tokens';
|
||||
const API_LINK_TABLE = 'api_token_project';
|
||||
@ -139,7 +140,7 @@ export class ApiTokenStore implements IApiTokenStore {
|
||||
}
|
||||
|
||||
async insert(newToken: IApiTokenCreate): Promise<IApiToken> {
|
||||
const response = await this.db.transaction(async (tx) => {
|
||||
const response = await inTransaction(this.db, async (tx) => {
|
||||
const [row] = await tx<ITokenInsert>(TABLE).insert(
|
||||
toRow(newToken),
|
||||
['created_at'],
|
||||
|
@ -26,6 +26,30 @@ export type WithTransactional<S> = S & {
|
||||
transactional: <R>(fn: (service: S) => R) => Promise<R>;
|
||||
};
|
||||
|
||||
/**
|
||||
* @deprecated this is a temporal solution to deal with transactions at the store level.
|
||||
* Ideally, we should handle transactions at the service level (each service method should be transactional).
|
||||
* The controller should define the transactional scope as follows:
|
||||
* https://github.com/Unleash/unleash/blob/cb034976b93abc799df774858d716a49f645d669/src/lib/features/export-import-toggles/export-import-controller.ts#L206-L208
|
||||
*
|
||||
* To be able to use .transactional method, services should be instantiated like this:
|
||||
* https://github.com/Unleash/unleash/blob/cb034976b93abc799df774858d716a49f645d669/src/lib/services/index.ts#L282-L284
|
||||
*
|
||||
* This function makes sure that `fn` is executed in a transaction.
|
||||
* If the db is already in a transaction, it will execute `fn` in that transactional scope.
|
||||
*
|
||||
* https://github.com/knex/knex/blob/bbbe4d4637b3838e4a297a457460cd2c76a700d5/lib/knex-builder/make-knex.js#L143C5-L144C88
|
||||
*/
|
||||
export async function inTransaction<R>(
|
||||
db: Knex,
|
||||
fn: (db: Knex) => R,
|
||||
): Promise<R> {
|
||||
if (db.isTransaction) {
|
||||
return fn(db);
|
||||
}
|
||||
return db.transaction(async (tx) => fn(tx));
|
||||
}
|
||||
|
||||
export function withTransactional<S>(
|
||||
serviceFactory: (db: Knex) => S,
|
||||
db: Knex,
|
||||
@ -33,6 +57,8 @@ export function withTransactional<S>(
|
||||
const service = serviceFactory(db) as WithTransactional<S>;
|
||||
|
||||
service.transactional = async <R>(fn: (service: S) => R) =>
|
||||
// Maybe: inTransaction(db, async (trx: Knex.Transaction) => fn(serviceFactory(trx)));
|
||||
// this assumes that the caller didn't start a transaction already and opens a new one.
|
||||
db.transaction(async (trx: Knex.Transaction) => {
|
||||
const transactionalService = serviceFactory(trx);
|
||||
return fn(transactionalService);
|
||||
|
@ -104,7 +104,7 @@ export interface IUnleashOptions {
|
||||
versionCheck?: Partial<IVersionOption>;
|
||||
telemetry?: boolean;
|
||||
authentication?: Partial<IAuthOption>;
|
||||
ui?: object;
|
||||
ui?: IUIConfig;
|
||||
frontendApi?: IFrontendApi;
|
||||
import?: Partial<IImportOption>;
|
||||
experimental?: Partial<IExperimentalOptions>;
|
||||
|
Loading…
Reference in New Issue
Block a user