From 037932df45c7db2e504d32ee03313c01d9f61267 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Tue, 14 Feb 2017 11:05:36 +0000
Subject: [PATCH] Issue #2851635 by tim.plunkett, alexpott, dawehner:
 DefaultSingleLazyPluginCollection retains stale instance IDs

---
 .../lib/Drupal/Core/Action/ConfigurableActionBase.php |  4 ++--
 .../Core/Plugin/DefaultSingleLazyPluginCollection.php |  7 +++----
 .../tests/src/Unit/StateTransitionValidationTest.php  |  3 +++
 .../src/Plugin/ConfigurableSearchPluginBase.php       |  4 ++--
 core/modules/workflows/tests/src/Unit/StateTest.php   |  3 +++
 .../workflows/tests/src/Unit/TransitionTest.php       |  3 +++
 .../modules/workflows/tests/src/Unit/WorkflowTest.php |  9 +++++++++
 .../Plugin/DefaultSingleLazyPluginCollectionTest.php  | 11 +++++++++++
 8 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/core/lib/Drupal/Core/Action/ConfigurableActionBase.php b/core/lib/Drupal/Core/Action/ConfigurableActionBase.php
index 6c19540bd393..2bbc9122c27e 100644
--- a/core/lib/Drupal/Core/Action/ConfigurableActionBase.php
+++ b/core/lib/Drupal/Core/Action/ConfigurableActionBase.php
@@ -17,7 +17,7 @@ abstract class ConfigurableActionBase extends ActionBase implements Configurable
   public function __construct(array $configuration, $plugin_id, $plugin_definition) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
 
-    $this->configuration += $this->defaultConfiguration();
+    $this->setConfiguration($configuration);
   }
 
   /**
@@ -38,7 +38,7 @@ public function getConfiguration() {
    * {@inheritdoc}
    */
   public function setConfiguration(array $configuration) {
-    $this->configuration = $configuration;
+    $this->configuration = $configuration + $this->defaultConfiguration();
   }
 
   /**
diff --git a/core/lib/Drupal/Core/Plugin/DefaultSingleLazyPluginCollection.php b/core/lib/Drupal/Core/Plugin/DefaultSingleLazyPluginCollection.php
index bbcb0f2a6150..e2bff29422aa 100644
--- a/core/lib/Drupal/Core/Plugin/DefaultSingleLazyPluginCollection.php
+++ b/core/lib/Drupal/Core/Plugin/DefaultSingleLazyPluginCollection.php
@@ -52,10 +52,7 @@ class DefaultSingleLazyPluginCollection extends LazyPluginCollection {
    */
   public function __construct(PluginManagerInterface $manager, $instance_id, array $configuration) {
     $this->manager = $manager;
-    $this->instanceId = $instance_id;
-    // This is still needed by the parent LazyPluginCollection class.
-    $this->instanceIDs = array($instance_id => $instance_id);
-    $this->configuration = $configuration;
+    $this->addInstanceId($instance_id, $configuration);
   }
 
   /**
@@ -95,6 +92,8 @@ public function setConfiguration($configuration) {
    */
   public function addInstanceId($id, $configuration = NULL) {
     $this->instanceId = $id;
+    // Reset the list of instance IDs since there can be only one.
+    $this->instanceIDs = [];
     parent::addInstanceId($id, $configuration);
     if ($configuration !== NULL) {
       $this->setConfiguration($configuration);
diff --git a/core/modules/content_moderation/tests/src/Unit/StateTransitionValidationTest.php b/core/modules/content_moderation/tests/src/Unit/StateTransitionValidationTest.php
index 2e034477878e..6ab2ad9875f2 100644
--- a/core/modules/content_moderation/tests/src/Unit/StateTransitionValidationTest.php
+++ b/core/modules/content_moderation/tests/src/Unit/StateTransitionValidationTest.php
@@ -62,6 +62,9 @@ protected function setUpModerationInformation(ContentEntityInterface $entity) {
     // mocked.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_manager = $this->prophesize(WorkflowTypeManager::class);
diff --git a/core/modules/search/src/Plugin/ConfigurableSearchPluginBase.php b/core/modules/search/src/Plugin/ConfigurableSearchPluginBase.php
index 3a39cbc16e95..4f259831d0ce 100644
--- a/core/modules/search/src/Plugin/ConfigurableSearchPluginBase.php
+++ b/core/modules/search/src/Plugin/ConfigurableSearchPluginBase.php
@@ -23,7 +23,7 @@ abstract class ConfigurableSearchPluginBase extends SearchPluginBase implements
   public function __construct(array $configuration, $plugin_id, $plugin_definition) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
 
-    $this->configuration = NestedArray::mergeDeep($this->defaultConfiguration(), $this->configuration);
+    $this->setConfiguration($configuration);
   }
 
   /**
@@ -44,7 +44,7 @@ public function getConfiguration() {
    * {@inheritdoc}
    */
   public function setConfiguration(array $configuration) {
-    $this->configuration = $configuration;
+    $this->configuration = NestedArray::mergeDeep($this->defaultConfiguration(), $configuration);
   }
 
   /**
diff --git a/core/modules/workflows/tests/src/Unit/StateTest.php b/core/modules/workflows/tests/src/Unit/StateTest.php
index 82feca34627b..ae9fc4620233 100644
--- a/core/modules/workflows/tests/src/Unit/StateTest.php
+++ b/core/modules/workflows/tests/src/Unit/StateTest.php
@@ -27,6 +27,9 @@ protected function setUp() {
     // mocked.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_type->deleteState(Argument::any())->willReturn(NULL);
diff --git a/core/modules/workflows/tests/src/Unit/TransitionTest.php b/core/modules/workflows/tests/src/Unit/TransitionTest.php
index 3202e8a46644..c343fed9273b 100644
--- a/core/modules/workflows/tests/src/Unit/TransitionTest.php
+++ b/core/modules/workflows/tests/src/Unit/TransitionTest.php
@@ -27,6 +27,9 @@ protected function setUp() {
     // mocked.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_manager = $this->prophesize(WorkflowTypeManager::class);
diff --git a/core/modules/workflows/tests/src/Unit/WorkflowTest.php b/core/modules/workflows/tests/src/Unit/WorkflowTest.php
index 89ee31f6baae..e2a7401cec11 100644
--- a/core/modules/workflows/tests/src/Unit/WorkflowTest.php
+++ b/core/modules/workflows/tests/src/Unit/WorkflowTest.php
@@ -27,6 +27,9 @@ protected function setUp() {
     // mocked.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_manager = $this->prophesize(WorkflowTypeManager::class);
@@ -215,6 +218,9 @@ public function testDeleteState() {
     // correctly.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_type->deleteState('draft')->shouldBeCalled();
@@ -636,6 +642,9 @@ public function testDeleteTransition() {
     // correctly.
     $container = new ContainerBuilder();
     $workflow_type = $this->prophesize(WorkflowTypeInterface::class);
+    $workflow_type->setConfiguration(Argument::any())->will(function ($arguments) {
+      $this->getConfiguration()->willReturn($arguments[0]);
+    });
     $workflow_type->decorateState(Argument::any())->willReturnArgument(0);
     $workflow_type->decorateTransition(Argument::any())->willReturnArgument(0);
     $workflow_type->deleteTransition('publish')->shouldBeCalled();
diff --git a/core/tests/Drupal/Tests/Core/Plugin/DefaultSingleLazyPluginCollectionTest.php b/core/tests/Drupal/Tests/Core/Plugin/DefaultSingleLazyPluginCollectionTest.php
index 22ecd3c4da50..ab7bfdc1dbf5 100644
--- a/core/tests/Drupal/Tests/Core/Plugin/DefaultSingleLazyPluginCollectionTest.php
+++ b/core/tests/Drupal/Tests/Core/Plugin/DefaultSingleLazyPluginCollectionTest.php
@@ -58,6 +58,17 @@ public function testAddInstanceId() {
     $this->assertEquals(['id' => 'banana', 'key' => 'other_value'], $this->defaultPluginCollection->get('banana')->getConfiguration());
   }
 
+  /**
+   * @covers ::getInstanceIds
+   */
+  public function testGetInstanceIds() {
+    $this->setupPluginCollection($this->any());
+    $this->assertEquals(['apple' => 'apple'], $this->defaultPluginCollection->getInstanceIds());
+
+    $this->defaultPluginCollection->addInstanceId('banana', ['id' => 'banana', 'key' => 'other_value']);
+    $this->assertEquals(['banana' => 'banana'], $this->defaultPluginCollection->getInstanceIds());
+  }
+
 }
 
 class ConfigurablePlugin extends PluginBase implements ConfigurablePluginInterface {
-- 
GitLab