Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom DB adapter in tracker mode #14876

Merged
merged 1 commit into from Sep 17, 2019

Conversation

@tsteur
Copy link
Member

commented Sep 10, 2019

In regular mode we already use the above logic see https://github.com/matomo-org/matomo/blob/3.x-dev/core/Db/Adapter.php#L68-L74

This means in regular mode any plugin can already define a custom adapter and configure it. This was not working in tracking mode where the two PDO and MySQLI modes were hard coded. This was likely because back in the days we weren't using auto loading in tracker mode which we are doing now already for many years.

In regular mode we already use the above logic see https://github.com/matomo-org/matomo/blob/3.x-dev/core/Db/Adapter.php#L68-L74

This means in regular mode any plugin can already define a custom adapter and configure it. This was not working in tracking mode where the two PDO and MySQLI modes were hard coded. This was likely because back in the days we weren't using auto loading in tracker mode which we are doing now already for many years.
@tsteur tsteur added the Needs Review label Sep 10, 2019
@tsteur tsteur added this to the 3.12.0 milestone Sep 10, 2019
case 'MYSQLI':
require_once PIWIK_INCLUDE_PATH . '/core/Tracker/Db/Mysqli.php';
return new Mysqli($configDb);
$className = 'Piwik\Tracker\Db\\' . str_replace(' ', '\\', ucwords(str_replace(array('_', '\\'), ' ', strtolower($configDb['adapter']))));

This comment has been minimized.

Copy link
@diosmosis

diosmosis Sep 17, 2019

Member

Does this mean it's not possible to define a db adapter in a plugin (because the namespace has to be in Piwik\Tracker\Db? I'm wondering if getting the class name through DI or an event might be better.

This comment has been minimized.

Copy link
@tsteur

tsteur Sep 17, 2019

Author Member

It works when you load the file where the class for this adapter is loaded yourself. DI could work as well, I just didn't want to change too much logic as in the regular DB factory it works already like this and also has the namespace Piwik\Db.... I reckon it's unlikely other people write their own adapters so maybe it's not too important, but could of course also change it towards DI but actually not sure if DI is already loaded at the time the DB is created? Likely it is ... but maybe not during installation etc? Not sure...

This comment has been minimized.

Copy link
@diosmosis

diosmosis Sep 17, 2019

Member

No worries, if it's already an established method I can merge

@diosmosis diosmosis merged commit d126061 into 3.x-dev Sep 17, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@diosmosis diosmosis deleted the dballowcustomadapter branch Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.