Update backend to use the dependency injection approach
- Dominant language
- JavaScript
- Stars
- 1.4k
- Forks
- 383
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 80
Description
## Feature Description
Currently, if we check our PHP codebase, we will see that we have a lot of tough coupled code and many controversial decisions there. This is our technical debt that was added as a result of an intention to implement new features quicker without spending too much time thinking about architecture. But as we grow, this technical debt becomes a burden that prevents us from improving the quality of our backend code and the developer experience in general.
A possible solution that can help us to free from tough coupled code and to streamline the development is an integration of the Dependency Injection approach into our codebase. This will not only make our classes less coupled but will also let us write better code that can be better tested with phpunit and e2e tests.
Here is a proof of concept that I have created: #3871.
---------------
_Do not alter or remove anything below. The following sections will be managed by moderators only._
## Acceptance criteria
* The `psr/container` dependency is added to the main `composer.json`.
* Two new exception classes that implement PSR-11 exception interfaces are created `Google\Site_Kit\Core\DI\Exception\Container_Exception` and `Google\Site_Kit\Core\DI\Exception\NotFound_Exception` (see https://www.php-fig.org/psr/psr-11/)
* `NotFound_Exception` extends `Container_Exception` to follow exception hierarchy
* These exceptions do NOT implement `WP_Errorable` interface - they are infrastructure exceptions representing programming errors that should be caught during development
* A new dependency injection container class `Google\Site_Kit\Core\DI\DI_Container` should be implemented, with the following requirements:
* It should implement `Psr\Container\ContainerInterface`.
* A public method `set_value( $id, $value )` which sets an arbitrary value in the DI container
* If the container is sealed, throws `Container_Exception` with message: "Cannot modify sealed container"
* If an entry with `$id` already exists, throws `Container_Exception` with message: "Entry already exists: {$id}"
* A public method `set_service( $id, $service )` which sets a service creator in the DI container. `$service` should be either a function to create the service class instance, or a class name, in case of services which can be instantiated with a simple `new $class_name()` call
* If the container is sealed, throws `Container_Exception` with message: "Cannot modify sealed container"
* If an entry with `$id` already exists, throws `Container_Exception` with message: "Entry already exists: {$id}"
* If `$service` is not a string or callable, throws `Container_Exception` with message: "Service creator must be a string (class name) or callable"
* If `$service` is a string, validates that the class exists using `class_exists()`. If not, throws `Container_Exception` with message: "Service class does not exist: {$service}"
* A public method `set_factory( $id, $factory )` which sets a factory creator in the DI container. `$factory` should work like `$service` above, either as a function to create a new instance, or just the class name
* A public method `get( $id )` (needed via `Psr\Container\ContainerInterface`) that returns the entry under that `$id`. Specifically:
* If the entry is a simple value (via above `set_value`), just return it
* If the entry is a service (via above `set_service`), only create it once and then always return that instance (almost like a singleton). When calling the creator function, pass the instance of the `DI_Container` to it, and if the resulting instance implements `DI_Aware_Interface`, then automatically call `set_di()` on it with the container
* If the entry is a factory (via above `set_factory`), always create a new instance and return it. When calling the creator function, pass the instance of the `DI_Container` to it and set the container if the `DI_Aware_Interface` interface is implemented
* If the entry is not found, throw the `NotFound_Exception` exception with message: "Entry not found: {$id}"
* If creating a new instance of the service throws an exception, re-throw it as `Container_Exception` with the original exception as the previous exception
* **Circular Dependency Detection:** Maintain an internal stack of entry IDs currently being created. Before creating a service or factory, check if `$id` is already in the creation stack. If so, throw `Container_Exception` with message: "Circular dependency detected: {path}" where `{path}` shows the dependency chain
* A public method `has( $id )` (needed via `Psr\Container\ContainerInterface`) which simply checks whether there's an entry for the given `$id`
* A public method `seal()` which "seals" the DI container. After this method is called, the container becomes immutable and all subsequent calls to `set_value()`, `set_service()`, or `set_factory()` throw `Container_Exception` with message: "Cannot modify sealed container". This method should be called in `Plugin::register()` after all initial services are registered
* A public method `is_sealed()` which returns `true` if the container has been sealed, `false` otherwise
* See [this POC class](https://github.com/google/site-kit-wp/pull/3871/files#diff-07f3b167c81d60f5e7bffd389bcd51244050bcf0eef35e8394e208a0095e2571R21) for a starting point
* A new interface `Google\Site_Kit\Core\DI\DI_Aware_Interface` should be implemented, for classes that need access to the above `DI_Container`, with the following requirements:
* A public method `set_di( ContainerInterface $di_container )` that sets the DI container within the instance.
* A public method `get_di()` that gets the DI container within the instance.
* A public method `get_di_entry( $id )` that returns the result of calling the `get` method on the DI container within the instance.
* See [this POC class](https://github.com/google/site-kit-wp/pull/3871/files#diff-656fa89d58b3e10799a827aa2d929cc6eee6eecaef9e2eec05fc6a60067e523fR15) for a starting point.
* The above `DI_Aware_Interface` should be accompanied by a new trait `Google\Site_Kit\Core\DI\DI_Aware_Trait` that implements these methods. In addition, the trait should also implement the magic `__get` method, to pass through to `get_di_entry`. That method should only exist to make initial migration of the code easy and it should eventually be removed. It should therefore already be annotated with `@deprecated` within its doc block.
* To start with, the above infrastructure should only be used in a few very basic infrastructure classes that do not require too complex dependencies within themselves. For now, the `Google\Site_Kit\Plugin` class should be updated to adjust the instantiation of these classes. The class should as part of this issue do the following:
* Implement the above `DI_Aware_Interface` (using the trait).
* Within its constructor, instantiate a new `DI_Container`, set the `$main_file` value under `MAIN_FILE` (via `set_value`) and set the context service creator function under `context` (via `set_service`).
* Within its `register` method, set the options service creator function under `options` (via `set_service`).
* In other words, the `Context` and `Options` classes should also be adjusted to implement `DI_Aware_Interface`, but those should be the only ones for now. Obviously, this doesn't make the DI approach very useful yet, but it's a good start to review the approach, and we don't want to cause too much of a mess by changing too many parts at the same time.
## Implementation Brief
*
### Test Coverage
*
## QA Brief
*
## Changelog entry
*
Contributor guide
Assessment
This issue has not been assessed yet.