Currently, class Drupal\Core\Database\Database has a lot of static stuff going on.
Initialized database connections, the pool of connection info arrays, and some meta stuff are all managed as static properties, and accessed via static methods.
As part of
#2354475: [meta] Refactor the installer, (multi)site management, and pre-container bootstrap →
, I would like to turn the entire thing into a service, managed in the newly introduced CoreContainer.
This means, we need to have an actual object behind the static methods.
We still need to keep the static methods around for BC, but the logic needs to happen in the object.
Proposed solution
Phase 1
- Introduce
class DatabaseManager, with real object methods for all the static methods of class Database.
- Have a
protected Database::$databaseManager instanceof DatabaseManager.
For phase 1, this will be lazy-instantiated the first time it is needed.
- Let the static methods of
class Database call the respective method in self::$databaseManager.
Only parseConnectionInfo() will remain static, because it's a pure function.
Phase 2
- Introduce
class ConnectionInfo to wrap the connection info for a single database.
Methods will be getDriverClass() and openConnection(), and also toArray() for BC.
- Optionally, introduce
class ConnectionInfoPool to wrap the connection info of all databases.
The benefit is limited because all the manipulators (e.g. renameConnection()) still need to touch both the ConnectionInfoPool and the DatabaseManager.
So far, the changes are all invisible to the public API. In general, components are still forced to use the static methods like Database::getConnection(). The DatabaseManager object is not exposed.
However, unit tests can now test the DatabaseManager, ConnectionInfo and ConnectionInfoPool directly, without having to mess with the global state of the static properties. This is already a benefit.
Phase 3 (follow-up)
This phase will only happen when the CoreContainer / MiniContainer is introduced, as proposed in
#2354475: [meta] Refactor the installer, (multi)site management, and pre-container bootstrap →
- Introduce the CoreContainer / MiniContainer architecture (other issue).
This also brings a mechanic to initialize global state.
- Introduce the core service $coreContainer->DatabaseManager.
- Replace the lazy instantiation of
Database::$databaseManager with an explicit Database::setDatabaseManager(), to be called from a CoreContainer mechanic.
- Refactor unit tests, so that methods like
Database::renameConnection() are no longer needed.
I think this last phase is what will make the changes worthwhile.
But the prior phases 1 and 2 are a good preparation, and I think it is a good idea to keep them out of the CoreContainer scope.
