Commit 92812c76 authored by catch's avatar catch
Browse files

Issue #3085751 by alexpott, Deepak Goyal, rpayanm, longwave, catch, volkerk,...

Issue #3085751 by alexpott, Deepak Goyal, rpayanm, longwave, catch, volkerk, kristiaanvandeneynde, dww: [backport] Setter injection arguments are not checked for unmet dependencies

(cherry picked from commit 2a71dd80fe94e30ebff5d76aff6f2a174c793fd8)
parent 017a8155
Loading
Loading
Loading
Loading
+40 −6
Original line number Diff line number Diff line
@@ -27,16 +27,28 @@ public function process(ContainerBuilder $container) {
    do {
      $has_changed = FALSE;
      foreach ($container->getDefinitions() as $key => $definition) {
        // Ensure all the definition's arguments are valid.
        foreach ($definition->getArguments() as $argument) {
          if ($argument instanceof Reference) {
            $argument_id = (string) $argument;
            if (!$container->has($argument_id) && $argument->getInvalidBehavior() === ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) {
              // If the container does not have the argument and would throw an
              // exception, remove the service.
          if ($this->isArgumentMissingService($argument, $container)) {
            $container->removeDefinition($key);
            $container->log($this, sprintf('Removed service "%s"; reason: depends on non-existent service "%s".', $key, (string) $argument));
            $has_changed = TRUE;
            $process_aliases = TRUE;
            // Process the next definition.
            continue 2;
          }
        }

        // Ensure all the method call arguments are valid.
        foreach ($definition->getMethodCalls() as $call) {
          foreach ($call[1] as $argument) {
            if ($this->isArgumentMissingService($argument, $container)) {
              $container->removeDefinition($key);
              $container->log($this, sprintf('Removed service "%s"; reason: depends on non-existent service "%s".', $key, $argument_id));
              $container->log($this, sprintf('Removed service "%s"; reason: method call "%s" depends on non-existent service "%s".', $key, $call[0], (string) $argument));
              $has_changed = TRUE;
              $process_aliases = TRUE;
              // Process the next definition.
              continue 3;
            }
          }
        }
@@ -58,4 +70,26 @@ public function process(ContainerBuilder $container) {
    }
  }

  /**
   * Checks if a reference argument is to a missing service.
   *
   * @param mixed $argument
   *   The argument to check.
   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
   *   The container.
   *
   * @return bool
   *   TRUE if the argument is a reference to a service that is missing from the
   *   container and the reference is required, FALSE if not.
   */
  private function isArgumentMissingService($argument, ContainerBuilder $container) {
    if ($argument instanceof Reference) {
      $argument_id = (string) $argument;
      if (!$container->has($argument_id) && $argument->getInvalidBehavior() === ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) {
        return TRUE;
      }
    }
    return FALSE;
  }

}
+1 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ function new_dependency_test_update_8001() {
    'new_dependency_test.alias_dependency',
    'new_dependency_test.alias2',
    'new_dependency_test.alias_dependency2',
    'new_dependency_test.setter_injection',
  ];

  // Gather the state of the services prior to installing the
+4 −0
Original line number Diff line number Diff line
@@ -34,3 +34,7 @@ services:
    decorates: new_dependency_test.another_service_two
    decoration_inner_name: new_dependency_test.foo
    arguments: ['@new_dependency_test.foo']
  new_dependency_test.setter_injection:
    class: Drupal\new_dependency_test\SetterInjection
    calls:
      - [setter, ['@new_dependency_test_with_service.service']]
+39 −0
Original line number Diff line number Diff line
<?php

namespace Drupal\new_dependency_test;

use Drupal\new_dependency_test_with_service\NewService;

/**
 * Generic service which uses setter injection.
 */
class SetterInjection {

  /**
   * The injected service.
   *
   * @var \Drupal\new_dependency_test_with_service\NewService
   */
  protected $service;

  /**
   * SetterInjection constructor.
   *
   * @param \Drupal\new_dependency_test_with_service\NewService $service
   *   The service of the new module.
   */
  public function setter(NewService $service) {
    $this->service = $service;
  }

  /**
   * Get the simple greeting from the service.
   *
   * @return string
   *   The greeting.
   */
  public function greet() {
    return $this->service->greet();
  }

}
+3 −0
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ public function testUpdateNewDependency() {
    $this->assertEquals('Hello', $this->container->get('new_dependency_test.alias')->greet());
    $this->assertEquals('Hello World', $this->container->get('new_dependency_test.hard_dependency')->greet());
    $this->assertEquals('Hello World', $this->container->get('new_dependency_test.optional_dependency')->greet());
    $this->assertEquals('Hello', $this->container->get('new_dependency_test.setter_injection')->greet());

    // Tests that existing decorated services work as expected during update.
    $this->assertTrue(\Drupal::state()->get('new_dependency_test_update_8001.decorated_service'), 'The new_dependency_test.another_service service is decorated');
@@ -70,6 +71,7 @@ public function testUpdateNewDependency() {
      'new_dependency_test.alias_dependency' => FALSE,
      'new_dependency_test.alias2' => FALSE,
      'new_dependency_test.alias_dependency2' => FALSE,
      'new_dependency_test.setter_injection' => FALSE,
    ], $before_install);

    $after_install = \Drupal::state()->get('new_dependency_test_update_8001.has_after_install', []);
@@ -81,6 +83,7 @@ public function testUpdateNewDependency() {
      'new_dependency_test.alias_dependency' => TRUE,
      'new_dependency_test.alias2' => TRUE,
      'new_dependency_test.alias_dependency2' => TRUE,
      'new_dependency_test.setter_injection' => TRUE,
    ], $after_install);
  }