spotorm / spot2

Spot v2.x DataMapper built on top of Doctrine's Database Abstraction Layer
http://phpdatamapper.com
BSD 3-Clause "New" or "Revised" License
601 stars 101 forks source link

Feature/multiple db connections #251

Closed dasper closed 6 years ago

dasper commented 6 years ago

Related to #49 I have a potential solution. Default connection is implicit or can be explicitly stated in each entity: protected static $connection = 'db1'; \\optional

Future implementations could alter where we set the connection param in the entityManager so we could set the connection dynamically ($mapper->connection('mysql')->all();) however I see the use case for this being incredibly low.

Note: The current unit tests work with only one database connection and I am not sure on the direction with creating new unit tests for multiple connections. Below are some basic examples on how I used and tested the setup. Any information on how to test this in isolation is appreciated.

<?php
require 'vendor/autoload.php';
date_default_timezone_set('UTC');

$cfg = new \Spot\Config();
$cfg->addConnection('db1', 'mysql://user:pass@127.0.0.1/db1');
$cfg->addConnection('db2',  'sqlite://path/to/db2.sqlite');
$cfg->addConnection('db3', 'mysql://user:pass@127.0.0.1/db3');
$spot = new \Spot\Locator($cfg);

$mapper = $spot->mapper('Entity\User'); \\default connection or set in class
$users = $mapper->all();
$changedConnection = $mapper->connection('db2')->query('select * from users limit 1');
Arzaroth commented 6 years ago

I would love to see this merged. Is there any blocking points aside from unit tests (or the lack of thereof) ? I am willing to write some if needed.

FlipEverything commented 6 years ago

I think we definitely need some tests before merging a change like this.

dasper commented 6 years ago

I am not strong at writing test cases and the fact that the existing tests presume and build only one mock db made me unsure how to move forward. If there is anything else I can do to provide assistance please let me know and I will try and address them this weekend.

vlucas commented 6 years ago

I like the idea here, but I think the more common use-case for multiple DB connections would be a read/write scenario where database replication is involved, and different connections are used for SELECT queries vs. UPDATE/INSERT etc. Thoughts on that use case?

This code is more of a different connection per entity model instead, which might be useful too, just a different approach.

dasper commented 6 years ago

I see it both ways. The inspiration for this PR was to complete a POC for my work utilizing different ORMs (Cake, Propel, Eloquent, Phalcon, etc) and having our data layer being a git sub module. A limiting factor here was we have 4 different data stores. Instead of giving up on spot, I found a simple way of completing the task.

Also, if you did want separate queries for select vs update/insert I am hoping future implementations will take advantage of $mapper->connection('datasource') to be able to dynamically choose the connection. Also, you could have the same database with just different user credentials to prohibit updates in a certain class with this invoked.

Also, I will not be offended if this gets closed or refactored by someone else. I just wanted to make sure the work I did on this POC was shared in case anyone else has a similar use case and if someone with more intimate knowledge of the project can implement it better I will be happy.

vlucas commented 6 years ago

Merged. Thanks!