While discussing #2266817: Deprecate empty AccessInterface and remove usages/#2222719: Use parameter matching via reflection for access checks instead of pulling variables from request attributes, it is clear that it is difficult to wire dependencies in Drupal, while adhering to good OOP practices. A dependency on symfony/expression-language was previously removed (#2747083: drupal/core-dependency-injection wrongly requires symfony/expression-language) due to being unused, and not required. In this issue, I am suggesting how to make use of it.

Problem/Motivation

The structure of many components in Drupal mean that services have dependencies on objects that aren't constructed by the dependency injection container, and therefore do not have direct access to them. As a result of this, services are required to depend on the parent service, and attempt to extract the actual dependency from that service.

An example would be that you were designing a service that needed to load nodes from the database, your service now has a dependency on Drupal\node\NodeStorage. This dependency is constructed by Drupal\Core\Entity\EntityTypeManager as a result of Drupal\Core\Entity\Annotation\EntityType expecting a list of classes via handlers = { ... }. This means that the only way to access this service is by using a dependency on entity_type.manager, abstracted by the Drupal\Core\Entity\EntityTypeManagerInterface, and call EntityTypeManagerInterface::getStorage(): EntityStorageInterface to get your real dependency. Also, since we are strictly depending on the node storage, we don't really want it to look like any other entity storage can be injected, and so we need to instead depend on Drupal\node\NodeStorageInterface.

This implementation breaks Law of Demeter and SOLID principals, as we are injecting a service, only to retrieve another other object - this requires nested stubbing/mocking in order to unit test. As well as this, Lack of interface segregation means that your service is aware of all of the methods on both EntityTypeManagerInterface and EntityStorageInterface, when you only actually want to call the rather obscure and nested NodeStorageInterface::loadMultiple(): EntityInterface[].

So your class must look like this:

# services.yml
mymodule.service:
  class: Drupal\mymodule\Service
  arguments:
  - '@entity_type.manager'
class Service {
  protected $nodeStorage;
  public function __construct(EntityTypeManagerInterface $entityTypeManager) {
    $this->nodeStorage = $entityTypeManager->getStorage('node');
    if (!$this->nodeStorage instanceof NodeStorageInterface) {
      throw new \InvalidArgumentException(...);
    }
  }

And really, you still only know that you are getting EntityInterface[] when you do call $this->nodeStorage->loadMultiple($nodeIds); , rather than actual nodes (Drupal\node\Entity\Node[]).

Another example is #2124749: [meta] Stop using $request->attributes->get(MAGIC_KEY) as a public API, addressed via Drupal\Core\Routing\RouteMatchInterface (#2238217: Introduce a RouteMatch class), which probably could be better segregated also. The solution being pursued here is using reflection to decide which dependencies to pass into a service method, which does solve the issue with Law of Demeter, however causes more SOLID issues.

It can sometimes be difficult to wire dependencies in a massive application, however the more components fail to stick to the basics will mean that the they aren't truly decoupled, and it will become much harder to refactor later.

Proposed resolution

Symfony's dependency injection component takes advantage of the expression language component, to allow you to depend on things like nested services or properties. This means that your code can be written cleanly, and you can just let the container care about wiring:

# services.yml
mymodule.service:
  class: Drupal\mymodule\Service
  arguments:
  - "@=service('entity_type.manager').getStorage('node')"
class Service {
  protected $nodeLoader;
  public function __construct(NodeMultipleLoaderInterface $nodeLoader) {
    $this->nodeLoader = $nodeLoader;
  }

This means that the code is completely future-proof to being re-architected (at the container level), which may be necessary where expressions are used repeatedly. For instance, if this is happening a lot, then perhaps node module is pushed to expose node.storage:

# node.services.yml
node.storage:
  factory: ['@entity_type.manager', getStorage]
  arguments: ['node']

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Anonymous’s picture

GroovyCarrot created an issue. See original summary.

Anonymous’s picture

Just to add another good use-case for this:

  public function __construct(ConfigFactoryInterface $configFactory) {
    $this->config = $configFactory->get('mymodule.settings');
  }

You could instead just inject the config you need:

services:
  my_service:
    class: D\mm\MyClass
    arguments:
      - "@=service('config.factory').get('mymodule.settings')"
  public function __construct(Config $config) {
    $this->config = $config;
  }
alexpott’s picture

I really like this idea. Does it actually work with the Drupal container?

pounard’s picture

If you are using the standard symfony dic, it should work with Drupal container, I guess.

alexpott’s picture

Our container is based on the Symfony DIC but not quite the same - hence #2747083: drupal/core-dependency-injection wrongly requires symfony/expression-language

dawehner’s picture

I really like this for things like entity storages. One thing we should though keep in mind: Calling out to other services on service constructor time can deal to really hard to debug problems.

pounard’s picture

I guess so, but under the hood you should have a real container builder ? Anyway if this doesn't work it would seem that you seriously mess up the original Symfony's container, but I guess this would be easily fixable.

Anonymous’s picture

I did have a rough go a prototyping it a while back, but found that either some effort had been taken to remove the compatibility in the first place, or when overriding functionality in Symfony, the decision had been made to leave it out.

// YamlFileLoader::resolveServices()
} elseif (is_string($value) &&  0 === strpos($value, '@=')) {
    // Not supported.
    //return new Expression(substr($value, 2));
    throw new InvalidArgumentException(sprintf("'%s' is an Expression, but expressions are not supported.", $value));
}

This is kind of related to a ticket I raised recently #2828099: Service container aliases do not work regarding functionality that has been lost from Symfony. I think that dawehner's (#4) comment is a good idea, it is probably worth either trying to apply any unit tests in Symfony for that method, or writing new unit tests to ensure that the functionality in Symfony, and new functionality for Drupal all works as expected.

dawehner’s picture

To be clear we didn't rewrote the container for fun. We had to because the concept of compiling the running container to disk is highly problematic in a system like Drupal, where the webserver is able to alter this behaviour. Note: You can install modules via the UI. This causes a container rebuild, which requires a newly written container in the default symfony implementation. Now imagine having multiple webserver somehow propagating this information. Putting the container information into a cache, and by that sharing it automatically via the database solved a lot of problems.

pounard’s picture

@dawehner I agree with you about the why, but expressions parser and factories should not be altered behaviours? If so it's probably a but that can be fixed, no matter if the container was overriden or not. I trust core developers for being able to untangle this and restore the common Symfony DIC behaviour.

Anonymous’s picture

There's certainly a lot more problems that Drupal has to solve than the average Symfony application. I just would say that this is probably a key step in pushing a unit testing initiative in Drupal; so that services can be properly isolated from one-another in the system, and you can properly stub direct/actual dependencies.

dawehner’s picture

Oh yeah I think we just from a compability point of view, we should add support for it. The question of how often we should use it itself in core then is an additional question. Let's do it?

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

geek-merlin’s picture

Some now and then i think that'd be fine.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joachim’s picture

The main use case for this, of injecting entity storage handlers, no longer applies, as there was a change in storage handlers that means they should no longer be injected but only instantiated when required.

sayco’s picture

Any updates on this?
Opposite to the comment above, I think entity storage is not the only case, there are multiple cases such as taking objects from factories - the config, queue, plugins, etc.
If there is a chance that it is easy to integrate (compatible with Drupal core container), then it is worth giving a try, especially if good practice/SOLID is at stake.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.