Papooch / nestjs-cls

A continuation-local storage (async context) module compatible with NestJS's dependency injection.
https://papooch.github.io/nestjs-cls/
MIT License
389 stars 23 forks source link

PgPromise adapter: add default tx options #136

Closed sam-artuso closed 2 months ago

sam-artuso commented 2 months ago

This PR adds the ability to specify default options for the @Transactional decorator, so that they don't need to be specified repeatedly.

sam-artuso commented 2 months ago

Thanks for your comments. I should have realized myself I needed to merge the default options, but that's what happens when you code while drinking beer...

On a more serious note, I need help. The tests for @nestjs-cls/transactional are failing for me. Even in the main branch. What am I doing wrong?

$ yarn test
[nestjs-cls]: Process started
[nestjs-cls]: PASS test/proxy-providers/for-feature.spec.ts
[nestjs-cls]: PASS test/plugin/plugin.spec.ts
[nestjs-cls]: PASS test/proxy-providers/for-root.spec.ts
[nestjs-cls]: PASS test/rest/for-root-async.spec.ts
[nestjs-cls]: PASS src/lib/cls.service.spec.ts
[nestjs-cls]: PASS test/proxy-providers/resolve-proxy-providers.spec.ts
[nestjs-cls]: PASS src/utils/value-from-path.spec.ts
[nestjs-cls]: PASS src/lib/cls-initializers/use-cls.decorator.spec.ts
[nestjs-cls]: PASS test/proxy-providers/proxy-providers.spec.ts
[nestjs-cls]: PASS test/rest/http-fastify.spec.ts (10.842 s)
[nestjs-cls]: PASS test/gql/gql-apollo.spec.ts (11.197 s)
[nestjs-cls]: PASS test/gql/gql-mercurius.spec.ts (11.582 s)
[nestjs-cls]: PASS test/rest/http-express.spec.ts (13.116 s)
[nestjs-cls]: 
[nestjs-cls]: Test Suites: 13 passed, 13 total
[nestjs-cls]: Tests:       153 passed, 153 total
[nestjs-cls]: Snapshots:   0 total
[nestjs-cls]: Time:        13.322 s
[nestjs-cls]: Ran all test suites.
[nestjs-cls]: Process exited (exit code 0), completed in 13s 926ms

[@nestjs-cls/transactional]: Process started
[@nestjs-cls/transactional]: FAIL test/inject-transaction.spec.ts
[@nestjs-cls/transactional]:   ● InjectTransaction with multiple named connections › when using the @Transactional decorator › should start two transactions independently with decorator
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     TypeError: this.cls.setProxy is not a function
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       221 |         this.cls.set(this.transactionInstanceSymbol, txInstance);
[@nestjs-cls/transactional]:       222 |         if (this._options.enableTransactionProxy) {
[@nestjs-cls/transactional]:     > 223 |             this.cls.setProxy(
[@nestjs-cls/transactional]:           |                      ^
[@nestjs-cls/transactional]:       224 |                 getTransactionToken(this._options.connectionName),
[@nestjs-cls/transactional]:       225 |                 txInstance,
[@nestjs-cls/transactional]:       226 |             );
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at TransactionHost.setTxInstance (src/lib/transaction-host.ts:223:22)
[@nestjs-cls/transactional]:       at src/lib/transaction-host.ts:193:37
[@nestjs-cls/transactional]:           at async Promise.all (index 0)
[@nestjs-cls/transactional]:       at CallingService.twoUnrelatedTransactionsWithDecorators (test/inject-transaction.spec.ts:58:26)
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:   ● InjectTransaction with multiple named connections › when using the @Transactional decorator › should start two transactions independently with startTransaction
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     TypeError: this.cls.setProxy is not a function
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       221 |         this.cls.set(this.transactionInstanceSymbol, txInstance);
[@nestjs-cls/transactional]:       222 |         if (this._options.enableTransactionProxy) {
[@nestjs-cls/transactional]:     > 223 |             this.cls.setProxy(
[@nestjs-cls/transactional]:           |                      ^
[@nestjs-cls/transactional]:       224 |                 getTransactionToken(this._options.connectionName),
[@nestjs-cls/transactional]:       225 |                 txInstance,
[@nestjs-cls/transactional]:       226 |             );
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at TransactionHost.setTxInstance (src/lib/transaction-host.ts:223:22)
[@nestjs-cls/transactional]:       at src/lib/transaction-host.ts:193:37
[@nestjs-cls/transactional]:           at async Promise.all (index 0)
[@nestjs-cls/transactional]:       at CallingService.twoUnrelatedTransactionsWithStartTransaction (test/inject-transaction.spec.ts:78:26)
[@nestjs-cls/transactional]:       at Object.<anonymous> (test/inject-transaction.spec.ts:170:17)
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:   ● InjectTransaction with multiple named connections › when using the @Transactional decorator › ignore transactions for other named connection
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     TypeError: this.cls.setProxy is not a function
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       221 |         this.cls.set(this.transactionInstanceSymbol, txInstance);
[@nestjs-cls/transactional]:       222 |         if (this._options.enableTransactionProxy) {
[@nestjs-cls/transactional]:     > 223 |             this.cls.setProxy(
[@nestjs-cls/transactional]:           |                      ^
[@nestjs-cls/transactional]:       224 |                 getTransactionToken(this._options.connectionName),
[@nestjs-cls/transactional]:       225 |                 txInstance,
[@nestjs-cls/transactional]:       226 |             );
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at TransactionHost.setTxInstance (src/lib/transaction-host.ts:223:22)
[@nestjs-cls/transactional]:       at src/lib/transaction-host.ts:193:37
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]: FAIL test/multiple-connections.spec.ts
[@nestjs-cls/transactional]:   ● Transactional › when using the @Transactional decorator › should start two transactions independently with decorator
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     expect(received).toEqual(expected) // deep equality
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Expected  - 7
[@nestjs-cls/transactional]:     + Received  + 1
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Array [
[@nestjs-cls/transactional]:     -   Array [
[@nestjs-cls/transactional]:     -     "BEGIN TRANSACTION;",
[@nestjs-cls/transactional]:     -     "SELECT 1",
[@nestjs-cls/transactional]:     -     "COMMIT TRANSACTION;",
[@nestjs-cls/transactional]:     -   ],
[@nestjs-cls/transactional]:     - ]
[@nestjs-cls/transactional]:     + Array []
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       153 |             });
[@nestjs-cls/transactional]:       154 |             const queries1 = mockDbConnection1.getClientsQueries();
[@nestjs-cls/transactional]:     > 155 |             expect(queries1).toEqual([
[@nestjs-cls/transactional]:           |                              ^
[@nestjs-cls/transactional]:       156 |                 ['BEGIN TRANSACTION;', 'SELECT 1', 'COMMIT TRANSACTION;'],
[@nestjs-cls/transactional]:       157 |             ]);
[@nestjs-cls/transactional]:       158 |             const queries2 = mockDbConnection2.getClientsQueries();
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at Object.<anonymous> (test/multiple-connections.spec.ts:155:30)
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:   ● Transactional › when using the @Transactional decorator › should start two transactions independently with startTransaction
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     expect(received).toEqual(expected) // deep equality
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Expected  - 7
[@nestjs-cls/transactional]:     + Received  + 1
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Array [
[@nestjs-cls/transactional]:     -   Array [
[@nestjs-cls/transactional]:     -     "BEGIN TRANSACTION;",
[@nestjs-cls/transactional]:     -     "SELECT 3",
[@nestjs-cls/transactional]:     -     "COMMIT TRANSACTION;",
[@nestjs-cls/transactional]:     -   ],
[@nestjs-cls/transactional]:     - ]
[@nestjs-cls/transactional]:     + Array []
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       169 |             });
[@nestjs-cls/transactional]:       170 |             const queries1 = mockDbConnection1.getClientsQueries();
[@nestjs-cls/transactional]:     > 171 |             expect(queries1).toEqual([
[@nestjs-cls/transactional]:           |                              ^
[@nestjs-cls/transactional]:       172 |                 ['BEGIN TRANSACTION;', 'SELECT 3', 'COMMIT TRANSACTION;'],
[@nestjs-cls/transactional]:       173 |             ]);
[@nestjs-cls/transactional]:       174 |             const queries2 = mockDbConnection2.getClientsQueries();
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at Object.<anonymous> (test/multiple-connections.spec.ts:171:30)
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:   ● Transactional › when using the @Transactional decorator › ignore transactions for other named connection
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     expect(received).toEqual(expected) // deep equality
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Expected  - 7
[@nestjs-cls/transactional]:     + Received  + 1
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:     - Array [
[@nestjs-cls/transactional]:     -   Array [
[@nestjs-cls/transactional]:     -     "BEGIN TRANSACTION;",
[@nestjs-cls/transactional]:     -     "SELECT 5",
[@nestjs-cls/transactional]:     -     "COMMIT TRANSACTION;",
[@nestjs-cls/transactional]:     -   ],
[@nestjs-cls/transactional]:     - ]
[@nestjs-cls/transactional]:     + Array []
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       186 |             });
[@nestjs-cls/transactional]:       187 |             const queries1 = mockDbConnection1.getClientsQueries();
[@nestjs-cls/transactional]:     > 188 |             expect(queries1).toEqual([
[@nestjs-cls/transactional]:           |                              ^
[@nestjs-cls/transactional]:       189 |                 ['BEGIN TRANSACTION;', 'SELECT 5', 'COMMIT TRANSACTION;'],
[@nestjs-cls/transactional]:       190 |             ]);
[@nestjs-cls/transactional]:       191 |             const queries2 = mockDbConnection2.getClientsQueries();
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]:       at Object.<anonymous> (test/multiple-connections.spec.ts:188:30)
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]: PASS test/propagation.spec.ts
[@nestjs-cls/transactional]: PASS test/transactional.spec.ts
[@nestjs-cls/transactional]: PASS test/named-connection.spec.ts
[@nestjs-cls/transactional]: 
[@nestjs-cls/transactional]: Test Suites: 2 failed, 3 passed, 5 total
[@nestjs-cls/transactional]: Tests:       6 failed, 25 passed, 31 total
[@nestjs-cls/transactional]: Snapshots:   0 total
[@nestjs-cls/transactional]: Time:        0.91 s, estimated 1 s
[@nestjs-cls/transactional]: Ran all test suites.
[@nestjs-cls/transactional]: Process exited (exit code 1), completed in 1s 452ms
The command failed for workspaces that are depended upon by other workspaces; can't satisfy the dependency graph
Failed with errors in 15s 382ms
sam-artuso commented 2 months ago

Additionally, TypeScript is complaining about lines 3, 4 and 126 of packages/transactional-adapters/transactional-adapter-pg-promise/test/transactional-adapter-pg-promise.spec.ts. You added those line last month, so you might have context on what's wrong with them?

Papooch commented 2 months ago

Oh damn, I updated typescript recently, but all tests were passing for me, so I thought all is fine. No new version has been released since then, so there's no immediate danger. I'll investigate.

Papooch commented 2 months ago

@sam-artuso I was not able to reproduce the problems on my side, but I have some ideas:

Make sure you run yarn install to update the dependencies on your side as well

And run yarn build before running the test (that's likely why it is failing), to build dependent libraries. It's likely that your version@nestjs-cls/transactional is still using the old built files from nestjs-cls.

sam-artuso commented 2 months ago

@Papooch this is now read to review 🙏🏼