@mnapoli opened this Pull Request on August 7th 2014 Member

Do not merge the PR yet, I just created the tests to share and to have the tests run.

This branch contains the refactoring to dependency injection contained in #5946, and goes one step further by adding a container: this is the step 2 described in #4917.

The container used here is (PHP-DI). The container is created and configured in the Piwik\StaticContainer class (which creates a singleton-like container). Doing so is ugly, but it's the best we can do until we have dependency injection everywhere.

So the StaticContainer class can be used to get the container, and then the container can be used to create/get objects. This PR contains 2 classes following DI: the Db and the Authorization classes.

Here is an example of how to get the Db object:

class Db
{
    public function __construct($adapter)
    {
        // here we have injected the dependency instead of using static methods/singletons
        $this->adapter = $adapter;
    }
    // ...
}

$container = \Piwik\StaticContainer::getContainer();
$db = $container->get('Piwik\Db\Db');

// Now we can use the $db object

I want to emphasize that this pattern is not perfect: this is the service locator pattern, we are accessing the container directly (and we shouldn't). When we get dependency injection in controllers and commands, we will not need to access the container anymore: we will just inject dependencies in the controllers (and that will be real dependency injection everywhere).

The Authorization class is a good example too because it uses the Db, so we use dependency injection there too:

class Authorization
{
    // ...

    private $db;

    public function __construct(Db $db)
    {
        // ...
    }
}

Cool!

The container's configuration is defined in the config/directory:

config/
    global.php.ini // ini config
    global.php // container config
    config.php.ini // ini config
    config.php // container config

On the long term, we could maybe try to remove the ini config and move all that into the php files, but whatever that's for an other discussion.

So global.php defines default values and entries. And then the user creates a config.php and can override anything.

If you want to learn more on what these files contain (what the syntax is, for example) check out here: http://php-di.org/doc/definition.html

For the record, I had to modify a bit how the Db singleton used to work: there was the possibility to override the Db instance to (for example) reconnect to the db. Now it works the same but without the singleton: to change the Db instance you have to set it manually in the container to override the current Db instance that is stored inside it.

For example:

$container = \Piwik\StaticContainer::getContainer();
// I added a DbFactory class to help creating the Db object
$dbFactory = $container->get('Piwik\Db\DbFactory');

$db = $container->get('Piwik\Db\Db');
$db->closeConnection();

// Reconnect to a different database
$newDb = $dbFactory->createDb($someConfig);

// Set it in the container
$container->set('Piwik\Db\Db', $db);
// Now  $container->get('Piwik\Db\Db') returns the new db object

On the long run, we might want to have the tests work differently maybe (with a dedicated configuration) but let's see that later.

@mnapoli commented on October 5th 2014 Member

Closing this as this was a POC.

This Pull Request was closed on October 5th 2014
Powered by GitHub Issue Mirror