add methods for checking if a store is expecting to be in transactional mode

src/lib/db/group-store.ts

@@ -9,8 +9,7 @@ import Group, {
     IGroupUser,
     IGroupUserModel,
 } from '../types/group';
-import Transaction = Knex.Transaction;
-import { Transactor } from './transactional';
+import { expectTransaction, Transactor } from './transactional';
 
 const T = {
     GROUPS: 'groups',
@@ -178,7 +177,6 @@ export default class GroupStore
         groupId: number,
         users: IGroupUserModel[],
         userName: string,
-        transaction?: Transaction,
     ): Promise<void> {
         const rows = users.map((user) => {
             return {
@@ -187,14 +185,11 @@ export default class GroupStore
                 created_by: userName,
             };
         });
-        return (transaction || this.db).batchInsert(T.GROUP_USER, rows);
+        return this.db.batchInsert(T.GROUP_USER, rows);
     }
 
-    async deleteUsersFromGroup(
-        deletableUsers: IGroupUser[],
-        transaction?: Transaction,
-    ): Promise<void> {
-        return (transaction || this.db)(T.GROUP_USER)
+    async deleteUsersFromGroup(deletableUsers: IGroupUser[]): Promise<void> {
+        return this.db(T.GROUP_USER)
             .whereIn(
                 ['group_id', 'user_id'],
                 deletableUsers.map((user) => [user.groupId, user.userId]),
@@ -209,10 +204,9 @@ export default class GroupStore
         deletableUsers: IGroupUser[],
         userName: string,
     ): Promise<void> {
-        await this.db.transaction(async (tx) => {
-            await this.addUsersToGroup(groupId, newUsers, userName, tx);
-            await this.deleteUsersFromGroup(deletableUsers, tx);
-        });
+        expectTransaction(this.db);
+        await this.addUsersToGroup(groupId, newUsers, userName);
+        await this.deleteUsersFromGroup(deletableUsers);
     }
 
     async getNewGroupsForExternalUser(

src/lib/db/transactional.ts

@@ -20,3 +20,17 @@ export abstract class Transactor<T> implements Transactional<T> {
         return clone as T;
     }
 }
+
+export const expectTransaction = (db: Knex | Knex.Transaction): void => {
+    if (db.isTransaction) {
+        return;
+    }
+    const isRunningInTest = process.env.NODE_ENV === 'test';
+    const errorMessage =
+        'A store method that was expected to be run in a transaction was run outside of a transaction';
+    if (isRunningInTest) {
+        throw new Error(errorMessage);
+    } else {
+        console.error(errorMessage);
+    }
+};

src/lib/services/group-service.ts

@@ -15,6 +15,7 @@ import { IEventStore } from '../types/stores/event-store';
 import NameExistsError from '../error/name-exists-error';
 import { IUserStore } from '../types/stores/user-store';
 import { IUser } from '../types/user';
+import { Knex } from 'knex';
 
 export class GroupService {
     private groupStore: IGroupStore;
@@ -25,14 +26,17 @@ export class GroupService {
 
     private logger: Logger;
 
+    private db: Knex;
+
     constructor(
         stores: Pick<IUnleashStores, 'groupStore' | 'eventStore' | 'userStore'>,
-        { getLogger }: Pick<IUnleashConfig, 'getLogger'>,
+        { getLogger }: Pick<IUnleashConfig, 'getLogger'>, // db: Knex,
     ) {
         this.logger = getLogger('service/group-service.js');
         this.groupStore = stores.groupStore;
         this.eventStore = stores.eventStore;
         this.userStore = stores.userStore;
+        // this.db = db;
     }
 
     async getAll(): Promise<IGroupModel[]> {
@@ -118,19 +122,21 @@ export class GroupService {
         );
         const deletableUserIds = deletableUsers.map((g) => g.userId);
 
-        await this.groupStore.updateGroupUsers(
-            newGroup.id,
-            group.users.filter(
-                (user) => !existingUserIds.includes(user.user.id),
-            ),
-            group.users.filter(
-                (user) =>
-                    existingUserIds.includes(user.user.id) &&
-                    !deletableUserIds.includes(user.user.id),
-            ),
-            deletableUsers,
-            userName,
-        );
+        this.db.transaction(async (tx) => {
+            await this.groupStore.transactional(tx).updateGroupUsers(
+                newGroup.id,
+                group.users.filter(
+                    (user) => !existingUserIds.includes(user.user.id),
+                ),
+                group.users.filter(
+                    (user) =>
+                        existingUserIds.includes(user.user.id) &&
+                        !deletableUserIds.includes(user.user.id),
+                ),
+                deletableUsers,
+                userName,
+            );
+        });
 
         await this.eventStore.store({
             type: GROUP_UPDATED,

src/test/fixtures/fake-feature-toggle-store.ts

@@ -4,6 +4,7 @@ import {
 } from '../../lib/types/stores/feature-toggle-store';
 import NotFoundError from '../../lib/error/notfound-error';
 import { FeatureToggle, FeatureToggleDTO, IVariant } from 'lib/types/model';
+import { Knex } from 'knex';
 
 export default class FakeFeatureToggleStore implements IFeatureToggleStore {
     features: FeatureToggle[] = [];
@@ -135,4 +136,11 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore {
         feature.variants = newVariants;
         return feature;
     }
+
+    transactional(
+        /* eslint-disable-next-line */
+        transaction: Knex.Transaction<any, any[]>,
+    ): IFeatureToggleStore {
+        throw new Error('Method not implemented.');
+    }
 }

src/test/transactional.test.ts

@@ -29,6 +29,102 @@ test('should actually do something transactional mode', async () => {
     expect(createdGroup).toBeDefined();
 });
 
+test('should fail if we run a method that requires transactional outside of a transaction', async () => {
+    const newUser = await stores.userStore.insert({
+        name: 'Tyler Durden',
+    });
+    const oldUser = await stores.userStore.insert({
+        name: 'Bob Poulson',
+    });
+
+    const group = await stores.groupStore.create({
+        name: 'TestGroup',
+    });
+
+    await expect(async () =>
+        stores.groupStore.updateGroupUsers(
+            group.id,
+            [
+                {
+                    user: newUser,
+                },
+            ],
+            [],
+            [
+                {
+                    groupId: group.id,
+                    userId: oldUser.id,
+                },
+            ],
+            'David Fincher',
+        ),
+    ).rejects.toThrow(Error);
+});
+
+test('should not fail if we run a method that requires transactional outside of a transaction in prod mode', async () => {
+    const newUser = await stores.userStore.insert({
+        name: 'Tyler Durden',
+    });
+    const oldUser = await stores.userStore.insert({
+        name: 'Bob Poulson',
+    });
+
+    const group = await stores.groupStore.create({
+        name: 'TestGroup',
+    });
+
+    process.env.NODE_ENV = 'prod';
+
+    await stores.groupStore.updateGroupUsers(
+        group.id,
+        [
+            {
+                user: newUser,
+            },
+        ],
+        [],
+        [
+            {
+                groupId: group.id,
+                userId: oldUser.id,
+            },
+        ],
+        'David Fincher',
+    );
+});
+
+test('should not fail if we run a method that requires transactional inside of a transaction in test mode', async () => {
+    const newUser = await stores.userStore.insert({
+        name: 'Tyler Durden',
+    });
+    const oldUser = await stores.userStore.insert({
+        name: 'Bob Poulson',
+    });
+
+    const group = await stores.groupStore.create({
+        name: 'TestGroup',
+    });
+
+    await db.db.transaction(async (trx) => {
+        await stores.groupStore.transactional(trx).updateGroupUsers(
+            group.id,
+            [
+                {
+                    user: newUser,
+                },
+            ],
+            [],
+            [
+                {
+                    groupId: group.id,
+                    userId: oldUser.id,
+                },
+            ],
+            'David Fincher',
+        );
+    });
+});
+
 test('should fail entire transaction if encountering an error', async () => {
     await db.db.transaction(async (trx) => {
         const featureDTO = {