Here is a sum up of our discussion:

Migration from Singletons to DI

Step 1

• refactor singleton objects into objects using dependency injection. The new classes can then be unit tested more easily (good because we want more unit tests)
• we keep the singleton and its methods to keep Backward Compatibility. The singleton now because a static proxy to the real object, just like in Laravel (where proxies are called Facades)

Quick and dirty example:

// This is the old Singleton
// It's now just a facade/proxy
class Db
{
    public static function get()
    {
        if (self::$connection === null) {
            // Create the DB object
        }
        return self::$connection;
    }

    public static function fetchAll(...)
    {
        // Forward to the real object
        self::get()->fetchAll(...);
    }
}

// This is the new object
class Database
{
    private $connection;

    public function __construct(DbConnection $connection)
    {
        $this->connection = $connection;
    }

    public function fetchAll(...)
    {
        // Do the real stuff
    }
}

What's cool:

• we keep BC compatibility for plugins and for the whole Core codebase: we shouldn't need to change any line outside the refactored class, so we can go with small steps
• we don't necessarily need a container at first, the instantiation can still be in the singleton since everything else will still use the singleton (for the 1st step)
• the new objects are fully using DI, so they are perfectly testable (and we can use mocks)
• we have to have a different name for the classes because we keep the singleton class: that's an opportunity to maybe create completely namespaced components (e.g. maybe have the new Db object inside the Db folder, so that everything related to Db is together)?
• we also get the opportunity to replace the code by libraries, e.g. for logging…

Step 2

• we add a container
• we move all the object's creation (that was in the singletons) into the container's config
• the singletons now use the container to create the proxied objects

Step 3

Slow migration from using singletons to injecting dependencies:

• we change the way controllers work (both web and API controllers): we need to split them because they are getting quite long sometimes, and it will only get worse with dependency injection
• controllers can now be created by the container (we haven't discussed that in details too much yet) and there can be dependency injection in the controllers (so no need to use the singletons anymore, great!)
• same can be done with CLI commands: they shouldn't be using the singletons anymore

Step 4

In some future version we can remove the singletons.

Architecture of plugins

We have 2 options:
- a "root" container + one child container per plugin (that would have access to the root container entries, but can't modify it)
- one container and several config files (e.g. one config per plugin) -> plugins could override Core config

We discussed both options, and kind of agreed that it's OK if plugins can override Core config. It should be recommended that plugins shouldn't do it, but leaving the door open to that can be very useful in some situations. It will be the plugins responsibility to make the right choice. So in the end one container + several config files is the simplest option.

It is also the solution is used by Symfony in its system of Bundles, and it's very powerful.

Here is for example how configs could be included:

• Global Piwik config (default options)
• Plugin 1 config
• Plugin 2 config
• Plugin … config
• User config

The latest included config can override the previous ones. That way plugins can enrich, or override the default config, and then the user can always re-override everything.

Here is an example with Authentication:

• global config (default):

AuthProviderInterface is bound to DatabaseAuthProvider (users logins are stored in database)

• the LDAP plugin provides LdapAuthProvider but doesn't override AuthProviderInterface (because that's bad practice, it's better to let the user choose)

In the plugin's config file, there is the configuration for LdapAuthProvider

• the user overrides the default config:

AuthProviderInterface is now bound the interface to LdapAuthProvider, so Piwik's authentication will now work with LDAP.

Then everywhere we want to use Auth, we inject AuthProviderInterface and the container will inject the object that was configured (so in our example, LdapAuthProvider).

Events

Regarding the events, and the fact that "Components" have to extend the interface (so that events are auto-declared), we are not all on the same page. Since this is not needed right now, we can come to a decision later on.

Container

We have discussed several options, we agree that having autowiring and annotations would be great (like in Spring), we have discussed maybe using PHP-DI (which covers that). The question is still open (I think obviously PHP-DI would fit the bill pretty nicely but I'm not objective :p).

Here are some documentation topics that are related if you want to learn more:

• how to define entries (supports autowiring, annotations and array config)
• definition overriding

And if you have any question about it just ask

+1 for DI in Piwik

Hi all, here are the 2 POC we discussed for the steps described above:

• step 1 (DI refactoring): [POC] Singleton/Static classes refactoring to dependency injection #5946
• step 2 (container): [POC] PHP-DI container #5948

Requirement #2 (and the whole event dispatching system) can be removed if DI solution makes it easy for plugins to composit existing components. For example, a plugin could create a new Auth component that composits (ie, uses) the existing injected component. The compositing object can manipulate the existing component w/o having to use events, and the problem w/ plugins & event ordering can be avoided too.

👍

If possible I think replacing INI config w/ DI config would be a good idea. Ie move all settings in INI config to DI config while still keeping it easy for end users to modify.

fyi we must keep INI format because we don't want to update all FAQs and guides explaining how to change settings etc.

We can keep it backwards compatible or still support INI, but DI config will be much more powerful since you should be able to configure actual objects.

For example, it should be possible (w/ correct Piwik architecture), to create a DAO object that uses a NoSQL solution and associate it w/ the selecting/aggregating (ie, map/reduce) logic, create DAO objects that use MySQL for sites, users and other relational data and create a DAO object that uses a key/value store for archive data all in a config file. It would be very hard and very ugly to represent such a configuration using only INI config and plugin event observers.

Heya @mnapoli,
It looks like we made big progress on introducing DI to Piwik.
what is the status on this issue?

Let's close it, it's been long out of date.

For anyone interested about using Dependency Injection in Piwik see this guide: https://github.com/piwik/developer-documentation/pull/67/files?short_path=762e172#diff-762e172d280769ca6701a87863ec30f2

@mnapoli I have two questions:

• I thought that StaticContainer class is deprecated, is it ?
• Am I able to define config in plugin ?

Not sure about our official point of view with the deprecated class StaticContainer @diosmosis what do you suggest, maybe the class is not deprecated after all, since we still use it in few places?

Am I able to define config in plugin ?

yes, you can create DI config.php files in plugins, see for example https://github.com/piwik/piwik/blob/master/plugins/Diagnostics/config/config.php

Thanks @mattab !

What is the order of including of such config files ? Can I override some service/class that is already defined in the main config from a plugin ?

Yes, plugins DI config should override core DI config.

Not sure about our official point of view with the deprecated class StaticContainer @diosmosis what do you suggest, maybe the class is not deprecated after all, since we still use it in few places?

It will be removed in 3.0. It has to be used now because Piwik dependencies are too tangled up, and because of @api static methods that can't be removed/deprecated until 3.0.