Issue #2408357 by dawehner: The ProxyBuilder includes parent interfaces, which causes php errors

core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php

@@ -48,11 +48,28 @@ public function build($class_name) {
 EOS;
     $class_start = 'class {{ proxy_class_name }}';
 
-    if ($interfaces = $reflection->getInterfaceNames()) {
-      foreach ($interfaces as &$interface) {
-        $interface = '\\' . $interface;
+    // For cases in which the implemented interface is a child of another
+    // interface, getInterfaceNames() also returns the parent. This causes a
+    // PHP error.
+    // In order to avoid that, check for each interface, whether one of its
+    // parents is also in the list and exclude it.
+    if ($interfaces = $reflection->getInterfaces()) {
+      foreach ($interfaces as $interface_name => $interface) {
+        // Exclude all parents from the list of implemented interfaces of the
+        // class.
+        if ($parent_interfaces = $interface->getInterfaceNames()) {
+          foreach ($parent_interfaces as $parent_interface) {
+            if (isset($interfaces[$parent_interface])) {}
+            unset($interfaces[$parent_interface]);
+          }
+        }
       }
-      $class_start .= ' implements ' . implode(', ', $interfaces);
+
+      $interface_names = [];
+      foreach ($interfaces as $interface) {
+        $interface_names[] = '\\' . $interface->getName();
+      }
+      $class_start .= ' implements ' . implode(', ', $interface_names);
     }
 
     $output .= $this->buildUseStatements();
@@ -102,7 +119,7 @@ public function build($class_name) {
     $output .= implode("\n", $methods);
 
     // Indent the output.
-    $output = implode("\n", array_map(function($value) {
+    $output = implode("\n", array_map(function ($value) {
       if ($value === '') {
         return $value;
       }
@@ -174,7 +191,7 @@ protected function buildMethod(\ReflectionMethod $reflection_method) {
 
     $output .= $this->buildMethodBody($reflection_method);
 
-    $output .= "\n". '}';
+    $output .= "\n" . '}';
     return $output;
   }
 

core/lib/Drupal/Component/ProxyBuilder/ProxyDumper.php

@@ -15,6 +15,13 @@
  */
 class ProxyDumper implements DumperInterface {
 
+  /**
+   * Keeps track of already existing proxy classes.
+   *
+   * @var array
+   */
+  protected $buildClasses = [];
+
   /**
    * The proxy builder.
    *
@@ -56,7 +63,16 @@ public function getProxyFactoryCode(Definition $definition, $id) {
    * {@inheritdoc}
    */
   public function getProxyCode(Definition $definition) {
-    return $this->builder->build($definition->getClass());
+    // Maybe the same class is used in different services, which are both marked
+    // as lazy (just think about 2 database connections).
+    // In those cases we should not generate proxy code the second time.
+    if (!isset($this->buildClasses[$definition->getClass()])) {
+      $this->buildClasses[$definition->getClass()] = TRUE;
+      return $this->builder->build($definition->getClass());
+    }
+    else {
+      return '';
+    }
   }
 
 }

core/tests/Drupal/Tests/Component/ProxyBuilder/ProxyBuilderTest.php

@@ -164,6 +164,19 @@ public function testMethod($parameter)
     $this->assertEquals($this->buildExpectedClass($class, $method_body, $interface_string), $result);
   }
 
+  /**
+   * @covers ::build()
+   */
+  public function testBuildWithNestedInterface() {
+    $class = 'Drupal\Tests\Component\ProxyBuilder\TestServiceWithChildInterfaces';
+
+    $result = $this->proxyBuilder->build($class);
+    $method_body = '';
+
+    $interface_string = ' implements \Drupal\Tests\Component\ProxyBuilder\TestChildInterface';
+    $this->assertEquals($this->buildExpectedClass($class, $method_body, $interface_string), $result);
+  }
+
   /**
    * @covers ::buildMethod()
    * @covers ::buildParameter()
@@ -348,3 +361,14 @@ public static function testMethod($parameter) {
 
 }
 
+interface TestBaseInterface {
+
+}
+
+interface TestChildInterface extends TestBaseInterface {
+
+}
+
+class TestServiceWithChildInterfaces implements TestChildInterface {
+
+}

core/tests/Drupal/Tests/Component/ProxyBuilder/ProxyDumperTest.php

@@ -85,6 +85,9 @@ public function testGetProxyFactoryCode() {
     $this->assertEquals($expected, $result);
   }
 
+  /**
+   * @covers ::getProxyCode()
+   */
   public function testGetProxyCode() {
     $definition = new Definition('Drupal\Tests\Component\ProxyBuilder\TestService');
     $definition->setLazy(TRUE);
@@ -99,6 +102,26 @@ public function testGetProxyCode() {
     $this->assertEquals($class, $result);
   }
 
+  /**
+   * @covers ::getProxyCode()
+   */
+  public function testGetProxyCodeWithSameClassMultipleTimes() {
+    $definition = new Definition('Drupal\Tests\Component\ProxyBuilder\TestService');
+    $definition->setLazy(TRUE);
+
+    $class = 'class Drupal_Tests_Component_ProxyBuilder_TestService_Proxy {}';
+    $this->proxyBuilder->expects($this->once())
+      ->method('build')
+      ->with('Drupal\Tests\Component\ProxyBuilder\TestService')
+      ->willReturn($class);
+
+    $result = $this->proxyDumper->getProxyCode($definition);
+    $this->assertEquals($class, $result);
+
+    $result = $this->proxyDumper->getProxyCode($definition);
+    $this->assertEquals('', $result);
+  }
+
 }
 
 class TestService {