From 3dad7119096cc9bf6280fc5f17b4122415627dc4 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Mon, 5 Feb 2018 07:16:54 +1000
Subject: [PATCH] Issue #2939208 by alexpott, tim.plunkett: Services registered
 with ContainerBuilder::setDefinition() are removed from container causing a
 BC break

---
 .../DependencyInjection/ContainerBuilder.php  | 21 +++++++++++---
 .../DependencyInjection/YamlFileLoader.php    |  2 --
 .../Drupal/KernelTests/KernelTestBaseTest.php |  4 +++
 .../ContainerBuilderTest.php                  | 29 +++++++++++++++++++
 4 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
index 00b6f761ae92..ae3e57a67ee8 100644
--- a/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
+++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
@@ -88,10 +88,7 @@ public function register($id, $class = null) {
     if (strtolower($id) !== $id) {
       throw new \InvalidArgumentException("Service ID names must be lowercase: $id");
     }
-    $definition = parent::register($id, $class);
-    // As of Symfony 3.4 all services are private by default.
-    $definition->setPublic(TRUE);
-    return $definition;
+    return parent::register($id, $class);
   }
 
   /**
@@ -104,6 +101,22 @@ public function setAlias($alias, $id) {
     return $alias;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function setDefinition($id, Definition $definition) {
+    $definition = parent::setDefinition($id, $definition);
+    // As of Symfony 3.4 all definitions are private by default.
+    // \Symfony\Component\DependencyInjection\Compiler\ResolvePrivatesPassOnly
+    // removes services marked as private from the container even if they are
+    // also marked as public. Drupal requires services that are public to
+    // remain in the container and not be removed.
+    if ($definition->isPublic()) {
+      $definition->setPrivate(FALSE);
+    }
+    return $definition;
+  }
+
   /**
    * {@inheritdoc}
    */
diff --git a/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php b/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
index 55687bba3e12..27e9d789f0f4 100644
--- a/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
+++ b/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
@@ -158,8 +158,6 @@ private function parseDefinition($id, $service, $file)
             $definition = new ChildDefinition($service['parent']);
         } else {
             $definition = new Definition();
-            // As of Symfony 3.4 all services are private by default.
-            $definition->setPublic(TRUE);
         }
 
         if (isset($service['class'])) {
diff --git a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
index f44c9650f6a3..2ba85e92e150 100644
--- a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
+++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
@@ -137,6 +137,10 @@ public function testRegister() {
     $this->assertInstanceOf('Symfony\Component\HttpFoundation\Request', $new_request);
     $this->assertSame($new_request, \Drupal::request());
     $this->assertSame($request, $new_request);
+
+    // Ensure getting the router.route_provider does not trigger a deprecation
+    // message that errors.
+    $this->container->get('router.route_provider');
   }
 
   /**
diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php
index 08465d6381a8..51629a5aabff 100644
--- a/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php
+++ b/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php
@@ -5,6 +5,7 @@
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Tests\UnitTestCase;
 use Drupal\Tests\Core\DependencyInjection\Fixture\BarClass;
+use Symfony\Component\DependencyInjection\Definition;
 
 /**
  * @coversDefaultClass \Drupal\Core\DependencyInjection\ContainerBuilder
@@ -70,6 +71,34 @@ public function testRegister() {
     $this->assertTrue($service->isPublic());
   }
 
+  /**
+   * @covers ::setDefinition
+   */
+  public function testSetDefinition() {
+    // Test a service with defaults.
+    $container = new ContainerBuilder();
+    $definition = new Definition();
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertTrue($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+
+    // Test a service with public set to false.
+    $definition = new Definition();
+    $definition->setPublic(FALSE);
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertFalse($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+
+    // Test a service with private set to true. Drupal does not support this.
+    // We only support using setPublic() to make things not available outside
+    // the container.
+    $definition = new Definition();
+    $definition->setPrivate(TRUE);
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertTrue($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+  }
+
   /**
    * @covers ::setAlias
    */
-- 
GitLab