Rework nested transaction handling

[martok]

This re-implements nested transaction handling (without any changes to the public API).

First, Adapter::transaction needs to call rollback and re-throw always, not only for the outermost transaction. This is needed so that nested transactions always resolve correctly. This is the same logic Record::transaction already had.

The largest part collects all nested transaction counting etc. with all special cases in a TransactionManager class that is used by the Adapter implementations. The adapters then only plug in the the database-specific handlers where needed.
Where supported by the DBMS, savepoints are used for nested transactions so even nested transactions can be rolled back individually. The DB specific versions are used for all native Adapters, for PDO the standard SQL syntax is used. When it is detected at runtime that the DB doesn't support it (i.e. connected via DBO to something that doesn't), a fallback mode is activated automatically.

I have tested this with all backends except SQL Server which I don't have access to. PHPUnit test is also added.

[nicksagona]

Wow - really great stuff here. Looking it over now.

[nicksagona]

The only thing that's jumping out at me as a possible issue is the hard-wired dependency of the TransactionManager object in the constructors of the adapters:

For example:
https://github.com/martok/pop-db-contrib/blob/pr/txn-nesting/src/Adapter/Mysql.php#L47

While the TM object is not likely something to be swapped out or overridden often, I think perhaps it might be better have it be injected in a way that the option to override it is a possibility if needed for whatever reason, from a pure OOP standpoint.

Any thoughts on that?

[martok]

Any thoughts on that?

Hm. True.

I had originally designed the manager as a Trait used by each implementation that would directly implement the isTransaction and getTransactionDepth functions of the AdapterInterface and provide the helpers as private methods (so instead of $this->transactionManager->enter you'd call $this->enterTransaction). I didn't do that purely on stylistic reasons as nothing else in Pop uses traits and went with the same way the Gateways for Record are assigned. And anyway, a trait would be even closer dependency.

How about a getTransactionManager method that initializes on first use and can be overridden? That would also handily reduce code duplication as all classes ended up with the same $useSavepoints=true call.

I'll rebase and force-push this PR if you agree.

Roughly,

// AbstractAdapter.php
protected function getTransactionManager(): TransactionManager
{
    return ($this->transactionManager ??= new TransactionManager());
}
// Mysql.php
public function beginTransaction(?int $flags = null, ?string $name = null): Mysql
{
    $this->getTransactionManager()->enter(

[nicksagona]

Yep I like that. Works for me.

[nicksagona]

Ok, that all looks good. I'm going to merge it in. Post-merge, I am considering slightly adjusting the organization/namespacing of your new functionality, just to keep it inline and similar to other parts of this component (like the Adapter\Profiler.) I think doing that will help your new functionality to have a good foundation so that if anyone else wants to come along and build upon or extend what you've done, they can do so easily.

Appreciate your contributions here to help make this component better - thanks!

[martok]

I've just checked out the split interface/abstract/implementation version and it looks good to me.

Thank you!