Hooks: proper nesting with savepoints, customisation, fix tests #206
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of simply incrementing a counter
start
hook creates a nested transaction with a savepoint. Counter way fails with complex scenarios like new third test for hooks.Added
getKnex
option forstart
hook. Allows to specify where to get knex from, so the hook can be used not only withfeathers-knex
service. Useful forfeathers-objection
and tests.Fixed previous test and added two additional. Previous test was actually not working, it was passing simply because "people2" table was not existent. And initial design of replacing "COMMIT;" with "COMMITA;" was a mistake because a connection was not released afterwards (it was working initially because it was the last test, but stopped to be in #181).
Remark on history: as i understand in an attempt to fix the problem caused by the broken test #181 included a removal of some lines in
hooks.js
(a, b, c, d), that caused regression in unhandled rejections, so #196 was needed. It had advantage over initial solution in handling rejections from an establishing phase, but ate rejections from an execution phase (this should have been visible if the broken test was working and not turned off in #181). Now both establishing and execution phases should be handled right.