PHP Dependency Injection when multiple types of databases are used in the same class

Question

I'm working on a very unorganized (no tests, logic mixed in everywhere) PHP application. I started reading the book Modernizing Legacy Applications In PHP and am now in the position where I need to replace global variables.

In my case we have both MySQL and MongoDB mixed heavily throughout the application using global variables: $database for MySQL and $mongo for MongoDB. I'm wondering what the best way to inject both of these databases would be. Most examples (in the book and elsewhere) have something like this:

class Database {
    public function __construct() {}
}

class MySQL extends Database {
    private $mysql;

    public function __construct() {
        $default_options = [
            PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
            PDO::ATTR_EMULATE_PREPARES   => false,
        ];
        // get config from file
        $this->mysql = new PDO(...);
    }

    public function get_instance() { return $this->mysql; }

}

class MongoDB extends Database {

    private $mongodb;  

    public function __construct() {
        // get config from file
        $mongodb = new MongoClient(...);
    }

    public function get_instance() { return $this->mongodb; }
}

class User {

    private $database = null;
    private $table = "users";

    public function __construct(Database $database) {
        $this->database = $database;
    }

}

// In another file:
$user = new User(new \Database\MySQL);

That's cool, but the problem I'm having is, when User stores its attributes like name,email, id, etc. in MySQL, and it stores something else like user_type in Mongo (obviously its more than this but I'm just giving an easy example), than how does dependency injection work?

Do I have to pass in two Database classes to the constructor?

Do I have a set_database function that I have to keep switching between when I want to make a call?

class User {

    private $database = null;
    private $table = "users";

    public function __construct(Database $database) {
        $this->database = $database;
    }

    public function get_user() {
        $first_set = $this->database->run('select * from users',[]);
        $this->database->set_database(new \Database\MongoDB);
        $second_set = $this->database->run('users',[]);
    }
}

I'm trying to understand how this would work, and would continue working if other developers added databases that had to be called in the User class (like Redis for example).

Answer 1

_{Warning: the following post contains use of Singleton antipattern as a temporary tool in refactoring of large scale clusterfuck}

I think you might have missed an important part of refactoring (or at least, focused on the wrong part). You should be aiming for as specific final result in this process. A specific architectural structure that you want to see (and that all your co-workers agree on). Right now you seem to just aimlessly trying to "make it less shit".

In the beginning ...

You immediate milestone seems to be "get rid of global variables". So, focus on that. Attempting to in the same time switch to fully dependency-injected structure will confuse things and increase the chance you fail.

Instead you should start by switching to registry (you could use singletons as a temporary step, but those are harder to clean out later).

So the code that looked like:

class Foobar {
    function something() {
        global $database; 
        $database->query('SELECT ... blah');
    }
}

Would be rewritten as:

class Foobar {
    function something() {
        $database = Registry::getDatabase(); 
        $database->query('SELECT ... blah');
    }
}

This would solve one specific problem: getting rid of global variables.

Second step

When that is done, you can start working on switching to dependency-injections. But not by adding typehints. That would require huge rewrites with a large "impact surface" (easy to break something).

Instead you start introducing DI container, but only for the parts, that were previously using singletons or registry. You create a singleton, that holds the instance of DI container (you basically use it as a service locator antipattern), and the code gets rewritten as:

class Foobar {
    function something() {
        $database = Locator::get('database'); 
        $database->query('SELECT ... blah');
    }
}

This is how procedural code (that also include static-only classes) can be refactored in this direction.

Third step

Next you need to start doing the OOP thing. You need to have those (previously global) variables to be moved to shared withing the class instance:

class Foobar {
    private $database;

    public function __construct() {
        $this->database = Locator::get('database'); 
    }

    public function something() {
        $this->database->query('SELECT ... blah');
    }
}

Fourth step

And only now you can actually move to using actual dependencies in the constructors.

class Foobar {
    private $database;

    public function __construct(Database $database) {
        $this->database = $database; 
    }

    public function something() {
        $this->database->query('SELECT ... blah');
    }
}

You also need to replace every case, where you have: $thing = new Foobar(); with
$thing = Locator::get('foobar');, and then repeat the step 3 for the class in which the Foobar was used, till you reach the bootstrap.

End of cycle

When you have completed the migration process, you stop using the Locator singleton wrapper in the bootstrap file and switch to using the actual DI container instance.

You might also have noticed that each of the steps can be performed, while new features are also being added to the codebase, because at no point it requires touching large swaths of code in unrelated location. Migration of each class can be done separately without breaking the functionality of the application as whole.

But

(and this is a big but).

From the piece of code that you showed, you would have another problem, that you need to solve. It is very likely, that your User class is actually implementing active record antipattern. So, when you are do with getting rid of globals, one of the next possible steps would be separating the persistence logic from business logic and switching to data mapper pattern. For further reading on that I will just spam three older posts of mine: 1, 2 and 3.

Answer 2

You can abstract it to multiple layers but first step would be something like this:

class PdoConfig {

    /**
     * @var string
     */
    private $dsn;

    /**
     * @var string|null
     */
    private $username;

    /**
     * @var string|null
     */
    private $password;

    /**
     * @var array
     */
    private $options;

    public function __construct(string $dsn, string $username = null, string $password = null, array $options = []) {
        $this->dsn = $dsn;
        $this->username = $username;
        $this->password = $password;
        $this->options = $options;
    }

    public static function constructFromConfigurationFile(string $file_path): self {
        //read data from config
        return new self($dsn, $username, $password, $options);
    }

    public function getDsn(): string {
        return $this->dsn;
    }

    public function getUsername(): ?string {
        return $this->username;
    }

    public function getPassword(): ?string {
        return $this->password;
    }

    public function getOptions(): array {
        return $this->options;
    }
}

class MongoDbConfig {

    // pattern as in PDOOptions
}

class Database {

    /**
     * @var PDO
     */
    private $pdo;

    /**
     * @var MongoClient
     */
    private $mongo_db;

    public function __construct(PdoConfig $pdo_config, MongoDbConfig $mongo_db_config) {
        $this->pdo = new PDO(
            $pdo_config->getDsn(),
            $pdo_config->getUsername(),
            $pdo_config->getPassword(),
            $pdo_config->getOptions()
        );

        $this->mongo_db = new MongoClient(...);
    }

    public function getPdo(): PDO {
        return $this->pdo;
    }

    public function getMongoDb(): MongoClient {
        return $this->mongo_db;
    }

}

class User {

    /**
     * @var Database
     */
    private $database;

    private $table = 'users';

    public function __construct(Database $database) {
        $this->database = $database;
    }

}

$database = new Database(
    PdoConfig::constructFromConfigurationFile('/path/to/pdo_config'),
    MongoDbConfig::constructFromConfigurationFile('/path/to/mongo_config')
);

// In another file:
$user = new User($database);

But it's only the first step. Second one could be some single class (like factory) that instantiates User objects passing the Database object to it. But surely you can build more and more layers of abstraction (config parsers, file readers, data mappers and so on). Dependency injector could become handy then - personally, I like Auryn.