Latest update breaks MongoRepository

[preterer]

Hi,

I'm using both SQL and noSQL in my application, both wrapped with TypeORM.

I'm using this plugin, with patchTypeORMRepositoryWithBaseRepository, since I'm using some entities from old external packages.

Everything worked fine, untill latest update, where getEntityManagerOrTransactionManager got ability to return undefined

It seems that because of that, TypeORM's MongoRepository always gets undefined as it's entityManager, when it's called in a transaction. Before the update, it would just return the _entityManager passed from repository object, but now it's unable to.

Workaround is to redefine MongoRepository manager as a "normal" property, after patchTypeORMRepositoryWithBaseRepository was called, so I suggest adding it to the function itself.

Object.defineProperty(MongoRepository.prototype, 'manager', { configurable: true, writable: true });

[preterer]

I added a PR #65 related to this issue

[odavid]

Hi @preterer - Thanks (and sorry for breaking things). I've made some changes to mitigate the issues caused by 0.1.13 and released 0.1.14 just now.

Can you please let me know if it solved your issues? If not, I will first create a new version that is a clone of 0.1.12 until I get these fixes right.

I believe also #63 should be fixed now with 0.1.14

[preterer]

Still not working. MongoRepository extends Repository and doesn't have it's own implementation of manager, so any changes done to that property in Repository will break MongoRepository unless they are overwritten later on.

[odavid]

@preterer - sorry to bug you again, Can you try: 0.1.15?

[preterer]

Yeah, it doesn't help.

I checked the commit, it does the opposite of what has to be done and forces MongoRepository to use your getEntityManagerOrTransactionManager, which is the reason it fails in the first place.

I re-read code of this plugin, and it seems the issue is not with MongoRepository, but rather having 2 TypeORM connections in one app used in a single transactional method, because @Transactional only saves one connections manager.

So what happens is:

• We have 2 connections (ConnectionA, ConnectionB)
• @Transactional() saves ConnectionA manager to context context.set(`${TYPE_ORM_KEY_PREFIX}ConnectionA`)
• method requires ConnectionA manager, everything works
• method requires ConnectionB manager:
  • context is defined
  • context.active is true
  • context.get(`${TYPE_ORM_KEY_PREFIX}${connectionName}`) returns undefined
  • manager is undefined

I didn't really think about it before, because typeorm doesn't support mongodb transactions anyway (they're a new feature)..

[preterer]

So my guess is in case of 2 (or more) SQL connections, you would just annotate the method twice (or more, one for each connection) giving names of connections every time.

However in case of Mongo, you will get a Transactions aren't supported by MongoDB. Error. That leads me to think my PR is still the easiest solution to the problem.

[preterer]

Oh yea, also the 0.1.15 patch prevents my workaround. I get TypeError: Cannot redefine property: manager now.

[odavid]

Thanks,

Will revert this and figure it out... for the previous matter - will try to find a solution Might bug you again ;)

On Fri, 27 Nov 2020 at 15:12 preterer notifications@github.com wrote:

Oh yea, also the 0.1.15 patch prevents my workaround. I get TypeError: Cannot redefine property: manager now.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/odavid/typeorm-transactional-cls-hooked/issues/64#issuecomment-734828716, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGSOAKRKASHZREIKAOKUVDSR6QTPANCNFSM4UEWH7JA .

[odavid]

@preterer, I've released 0.1.16 that patches MongoRepository as configurable and writable (Used your original PR code, but set it after patching the Repository)

Will be glad to get your feedback, when you have the time. In any case, I need to find some time to write more real world tests to this library...

Thanks again and great weekend...

[preterer]

@odavid Hello, the newest update seems to work fine. I'll close this issue.