Aliheym / typeorm-transactional

A Transactional Method Decorator for TypeORM that uses Async Local Storage or cls-hooked to handle and propagate transactions between different repositories and service methods.
MIT License
201 stars 27 forks source link

Method to expose transactional entity manager #8

Closed foliveirafilho closed 1 year ago

foliveirafilho commented 2 years ago

Hi @Aliheym!

I have a project that was recently upgraded to TypeORM v0.3.7 from v.0.2.x and I used a pretty cool workaround to maintain the custom repos with the same structure as before, as shown here.

Along with that, we also have to use transactions and we encoutered your lib that seems to ease the transactional job.

The thing is, for the transactions to work properly with this solution, I must call entityManager.withRepository(myRepo) as show here.

So I was able to retrieve the entity manager used in the current transaction with the following code:

export function getTransactionalEntityManager(name = 'default'): EntityManager {
  const transactionalContext = getTransactionalContext();
  if (isUndefined(transactionalContext)) {
    throw new Error('Transactional context was not found!');
  }

  const entityManager = getEntityManagerByDataSourceName(transactionalContext, name);
  if (isNull(entityManager)) {
    throw new Error(`Could not find any '${name}' named data source!`);
  }

  return entityManager;
}

But I think this could be provided by the lib. What do you think? Does it make sense with the current scope of this lib?

Aliheym commented 2 years ago

Hi @foliveirafilho. Thanks for this issue, I investigated your use case.

The problem is how entityManager is set for the custom repositories. Let's check a part of the code from the gist you provided:

// file - typeorm-ex.module.ts <---- this file from gist

// ...

 providers.push({
    inject: [getDataSourceToken()],
    provide: repository,
    useFactory: (dataSource: DataSource): typeof repository => {
      const baseRepository = dataSource.getRepository<any>(entity);
      // <------ Check line below
      return new repository(baseRepository.target, baseRepository.manager, baseRepository.queryRunner);
      //
    },
  });

// ...

Here we take the baseRepository.manager and pass it to our custom repository.

We set custom repository's entity manager as baseRepository.manager only once - during creation. It's a source of issues with transactions because in the TypeORM we will get a new entity manager for each transaction. Also because of it, this library doesn't work as intended.

I think we can resolve your problem easier. We just need to change this way how entityManager is resolved for custom repositories:

// file - typeorm-ex.module.ts <---- this file from gist
// ...

 providers.push({
    inject: [getDataSourceToken()],
    provide: repository,
    useFactory: (dataSource: DataSource): typeof repository => {
      const baseRepository = dataSource.getRepository<any>(entity);

      const customRepository = new repository(
        baseRepository.target,
        baseRepository.manager,
      );

      // we look at baseRepository's `manager` property 
      // to always get actual one
      Object.defineProperty(customRepository, 'manager', {
        get() {
          return baseRepository.manager;
        },
      });

      return customRepository;
    },
  });

// ...

If you change only this part of code, you don't need other "hacks" with entity manager.

I hope, it will help you.

foliveirafilho commented 1 year ago

Hey @Aliheym, it worked! Thank you for this workaround!

The problem I see is: if TypeORM creates a new entity manager for each transaction, don't you think using the same entity manager instance in every repository transaction could be a problem?

In the way I see it, it could decrease concurrency in the system. What do you think?

Aliheym commented 1 year ago

Don't worry. I don't see any problem with it. The solution from the gist and my solution are the same when you don't use this library. We only created a getter for repository's property the "manager". That's it. No difference at all. Your prev solution almost always uses the same entity manager instance, except for transactions, when you need to call entityManager.withRepository(myRepo) to work correctly with transactions.

But when you use this library you can't use the solution from gist, because this library "patch" a baseRepository.manager and some internal stuff to work with transactions easily, robustly, and efficiently. Because of this "patch", you can't just pass manager into the constructor of custom repository (this library can't track and patch it). Our workaround is to create this "getter" to the original baseRepository.manager, which my library can handle in the transactions.

The problem I see is: if TypeORM creates a new entity manager for each transaction, don't you think using the same entity manager instance in every repository transaction could be a problem?

In the TypeORM transactions look like:

repo.transaction(manager => {
  // You can think, that here always get a new instance of `manager`
});

Because of this style of transactions, it's hard to work with transactions in the TypeORM, especially if you want to split your logic and don't want to create new repositories for each transaction. This library or our solution doesn't change this behavior. We just use some Node.js possibilities to make transactions more convenient to use, so you can use the same repositories and other entities of TypeORM in the different transactions and different part of your code. Under the hood, this library makes some magic to support it.

foliveirafilho commented 1 year ago

Great explanation! I'll provide your workaround in the gist so your lib get more popular in the community.

Again, thank you for your help. This is such a great lib!