Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions does not work with multi tenant application #93

Open
gcherem opened this issue Mar 25, 2021 · 15 comments
Open

Transactions does not work with multi tenant application #93

gcherem opened this issue Mar 25, 2021 · 15 comments

Comments

@gcherem
Copy link

gcherem commented Mar 25, 2021

First of all, thank you for this library. It's a must-have.

I am using nest.js to create a multi tenant application, so the module creates the connection dynamically and injects into the service, and then I get the repository like this:

export class SurveyService {
  public repository: Repository<SurveyEntity>;

  constructor(
     @Inject(TENANT_CONNECTION) private connection,
     private surveyAttributeService: SurveyAttributeService
  ) { 
        this.repository = await this.connection.getRepository(SurveyEntity);  // <--- here
     }

  @Transactional()
  create(data): Promise<SurveyEntity> {
     const survey = this.repository.create(data);
     const savedSurvey = await this.repository.save(survey);
     if (data.attributes) {
       await this.surveyAttributeService.createFromArray(savedSurvey, data.attributes)
       // if an error occurs here, there is no rollback
     }
     return savedSurvey;
  }
}

The main.ts is:

import { NestFactory, Reflector } from '@nestjs/core';
import { AppModule } from './app.module';
import { Logger, ValidationPipe } from '@nestjs/common';
import { initializeTransactionalContext, patchTypeORMRepositoryWithBaseRepository } from 'typeorm-transactional-cls-hooked';
import * as compression from 'compression';
import { GqlRolesGuard } from './common/guards/gql-roles.guard';
import dotenvFlow = require('dotenv-flow');
import { ApolloErrorFilter } from './common/filters/apollo-error.filter';
import 'reflect-metadata';

async function bootstrap() {
  dotenvFlow.config({path: 'env'});
  initializeTransactionalContext(); // Initialize cls-hooked
  patchTypeORMRepositoryWithBaseRepository(); // patch Repository with BaseRepository.
  const app = await NestFactory.create(AppModule);
  app.useGlobalPipes(new ValidationPipe());
  app.useGlobalGuards(new GqlRolesGuard(new Reflector()));
  app.use(compression()); // enable gzip
  app.useGlobalFilters(new ApolloErrorFilter());
  await app.listen(process.env.APP_PORT);
  Logger.log(`Server is running on: ${await app.getUrl()}`);
}
bootstrap();

Although my application works fine, transactions just don't work. Am I missing something?

@odavid
Copy link
Owner

odavid commented Mar 28, 2021

Thanks!
I have not tested this can of setup, however, I believe you need to define the connectionName within the decorator.
You can use function that returns the connectionName, but it is a must, if it is not default

See https://github.com/odavid/typeorm-transactional-cls-hooked#using-transactional-decorator

Hope it helps...

@gcherem
Copy link
Author

gcherem commented Mar 29, 2021

Thanks @odavid for your reply. Unfortunately I have no idea how to get the connection name within the decorator, since I have no access to the request in that context (I think), The connection name is the tenant name that I retrieve from the JWT in the Authorization header. Any idea? Could you please provide an example or direct me to a sample? Thanks in advance.

@odavid
Copy link
Owner

odavid commented Apr 3, 2021

Hi @gcherem, I think that once this PR will be merged, you will be able to use the
runInTransaction function instead of the decorator

@naorye
Copy link

naorye commented May 5, 2021

I don't think this is related to multi tenant. I am using nestjs and it doesn't work as well.

@alanscordoba
Copy link

Gracias @odavid por tu respuesta. Desafortunadamente, no tengo idea de cómo obtener el nombre de la conexión dentro del decorador, ya que no tengo acceso a la solicitud en ese contexto (creo). El nombre de la conexión es el nombre del inquilino que recupero del JWT en el encabezado de Autorización. ¿Alguna idea? ¿Podría darme un ejemplo o dirigirme a una muestra? Gracias por adelantado.

Hello, I am stuck in this same situation. Some help?

@mohsinamjad
Copy link

I'm using multi tenant too just as op said,
I need some help to resolve dynamic connections?

@alanscordoba
Copy link

alanscordoba commented Aug 20, 2021

I can't get everything to run within the same transaction.

I have the following code:

export class TestService {
  repository: any;
  constructor(
  @Inject(TENANT_CONNECTION) private connection)
  {
    this.repository = connection.getRepository(Test)
  }`

 @Transactional()
  async agregar(tableNew: Test): Promise<Number> {
    const tableSave = this.repository.create(tableNew)
    await this.repository.save(tableSave)
    if(tableSave.default){
      await this.repository.update(
            {default:true,id:Not(tableSave.id)},
            {default:false}
      )
    }
    return tableSave.id 
}

The following is executed in the database:

query: START TRANSACTION
query: SELECT `Test`.`id` AS `Test_id`,`Test`.`default` AS `Test_default` FROM `test` `Test` WHERE `Test`.`id` IN (?) -- PARAMETERS: [0]
query: START TRANSACTION
query: INSERT INTO `test`(`id`,`default`) VALUES (?,?) -- PARAMETERS: [0,1]
query: COMMIT
query: UPDATE `test` SET `default` = ? WHERE (`default` = ? AND `id` != ?) -- PARAMETERS: [0,true,41]
query: COMMIT

As you can see, 2 transactions are executed, I need it to be only one

@mohsinamjad
Copy link

it's working for me now
here is the code snippet for multi tenant

  return runInTransaction(
      async () => {
       // async operations
      },
      { connectionName: this.connection.name },
    );

@pavel-xendit
Copy link

@mohsinamjad
This is not working for me.
I tried to put logic inside runInTransaction but still, the transaction is happening for each case. I want to have one transaction for all DB transactions.

@mohsinamjad
Copy link

mohsinamjad commented Oct 11, 2021

@pavel-xendit
weird, why the heck it's working for me then, I've tested on simple use-case though
Are you giving connectionName in the second param of runInTransacion?

my package info

"typeorm": "0.2.34",
"typeorm-transactional-cls-hooked": "0.1.21",

@pavel-xendit
Copy link

@mohsinamjad
How can I get connectionName? I passed default as connectionName.

@pavel-xendit
Copy link

@odavid @mohsinamjad
Any thoughts on how to make runIntransactions work?

@mopcweb
Copy link

mopcweb commented Dec 3, 2021

it is possible to use async-hooks or cls-hooked or some other similar package for creating request scoped context.
after that it is quite easy to use it in Transactional callback to get necessary connection

@mohsinamjad
Copy link

For multi tenant your connection name will be dynamic like in my case I set tenant ID as connection name, and you can't use same connection name is e.g default.
You have to use this.connectio.name for it.

@barokurniawan
Copy link

I believe that there is no definitive solution to this problem.

In my case, I used middleware to initialize the database connection and store it in a global variable so the user can re-use it, but that caused the transactional issue since addTransactionalDataSource should be only called when init the app .

So I came out with a solution to sync all tenant data to a JSON file (only the scheme name) and read the file data on app init so I can register all schemes with addTransactionalDataSource.

Some sample

import { TypeOrmModule } from "@nestjs/typeorm";
import { DataSource } from "typeorm";
import { addTransactionalDataSource } from "typeorm-transactional";
import * as con  from "./connections";

export const DatabaseConfig = TypeOrmModule.forRootAsync({
    useFactory() {
        return {
            type: 'postgres',
            host: process.env.DB_HOST,
            port: +process.env.DB_PORT,
            username: process.env.DB_USER,
            password: process.env.DB_PASSWORD,
            database: process.env.DB_NAME,
            entities: ["dist/**/*.entity{.ts,.js}"],
            migrations: ["dist/src/database/migrations/*{.ts,.js}"],
            logging: true,
            synchronize: false,
            migrationsRun: true,
            migrationsTransactionMode: 'all',
            useUTC: true
        };
    },
    async dataSourceFactory(options) {
        if (!options) {
            throw new Error('Invalid options passed');
        }

        let mainDs: DataSource;
       
        const tenants = ['public'];
        for (const schema of tenants) {
            const source = new DataSource({
                ...options,
                schema: schema,
            } as any);
            const ds = addTransactionalDataSource({name: schema, dataSource: source});
            con.connectionMap.set(schema, ds);

            if(schema == "public") {
                mainDs = ds;
            }
        }

        return mainDs;
    },
});

Just imagine this part const tenants = ['public']; are a list you get from a JSON file, con.connectionMap is a const connectionMap = new Map<string, DataSource>();.

The downside is that I have to restart the app every time a new tenant arrives, and hope someone jumps in and gives a pro solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants