Fix loose typing of constructor for transaction level, and remove public property.

[caddoo]

Description:

This is a possible first step to fix the problem with the very loosely typed dependency that the TransactionLevel class has.

This also removes that nasty public property $supportsUncommitted; and introduces some encapsulation for that concept.

The problem is identifying a common interface between:

• \Piwik\Tracker\Db
• \Piwik\Db\AdapterInterface
• \Piwik\Db

There are of course fetchOne query, however some classes have them statically and others not, so an interface isn't possible.

My idea/solution does cause a little duplication, but I think having:

• Type safety
• Consistency
• Readability

Is more important here.

Review

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[michalkleiner]

In general the changes look good, I've got a few suggestions for better consistency and perhaps less code:

• keep the order of the methods in the interface consistent in order of get/set for a property, then get/set for the other. Now it's get/set/set/get.
• consider using a trait as the implementation of the four methods is identical across 4 classes. If other db schemas need different implementation they can opt to not use the trait and implement the methods directly.

If someone else feels like we shouldn't use a trait here I'd be happy to concede here but the consistency would be nice even when not functionally important.

[caddoo]

In general the changes look good, I've got a few suggestions for better consistency and perhaps less code:

* keep the order of the methods in the interface consistent in order of get/set for a property, then get/set for the other. Now it's get/set/set/get.

* consider using a trait as the implementation of the four methods is identical across 4 classes. If other db schemas need different implementation they can opt to not use the trait and implement the methods directly.

If someone else feels like we shouldn't use a trait here I'd be happy to concede here but the consistency would be nice even when not functionally important.

Thanks for taking a look, i've reordered the methods to make it a bit more consistent.

I've added a commit with traits, take a look and tell me if you are happy with that approach, traits of course have their problems.

[michalkleiner]

This looks good, thanks! The static vs the dynamic set of calls is annoying, it could probably be done somehow via some magic, but that would be at the expense of legibility and long-term maintainability, so all good as it is.