Issue #2204345 by tim.plunkett: Adjust PluginBag's instance handling methods.

core/lib/Drupal/Component/Plugin/PluginBag.php

@@ -115,8 +115,10 @@ public function remove($instance_id) {
    *
    * @param string $id
    *   The ID of the plugin instance to add.
+   * @param array|null $configuration
+   *   (optional) The configuration used by this instance. Defaults to NULL.
    */
-  public function addInstanceId($id) {
+  public function addInstanceId($id, $configuration = NULL) {
     if (!isset($this->instanceIDs[$id])) {
       $this->instanceIDs[$id] = $id;
     }
@@ -132,21 +134,11 @@ public function getInstanceIds() {
     return $this->instanceIDs;
   }
 
-  /**
-   * Sets all instance IDs.
-   *
-   * @param array $instance_ids
-   *   An associative array of instance IDs.
-   */
-  public function setInstanceIds(array $instance_ids) {
-    $this->instanceIDs = $instance_ids;
-  }
-
   /**
    * Removes an instance ID.
    *
    * @param string $instance_id
-   *   An image effect instance IDs.
+   *   The ID of the plugin instance to remove.
    */
   public function removeInstanceId($instance_id) {
     unset($this->instanceIDs[$instance_id]);

core/lib/Drupal/Core/Plugin/DefaultPluginBag.php

@@ -138,15 +138,6 @@ public function setConfiguration($configuration) {
     return $this;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function setInstanceIds(array $instance_ids) {
-    parent::setInstanceIds($instance_ids);
-    // Ensure the new order matches the original order.
-    $this->instanceIDs = $this->originalOrder = array_intersect_assoc($this->originalOrder, $this->instanceIDs);
-  }
-
   /**
    * Updates the configuration for a plugin instance.
    *
@@ -166,6 +157,19 @@ public function setInstanceConfiguration($instance_id, array $configuration) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function addInstanceId($id, $configuration = NULL) {
+    parent::addInstanceId($id);
+    if ($configuration !== NULL) {
+      $this->setInstanceConfiguration($id, $configuration);
+    }
+    if (!isset($this->originalOrder[$id])) {
+      $this->originalOrder[$id] = $id;
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Plugin/DefaultSinglePluginBag.php

@@ -93,4 +93,14 @@ public function setConfiguration($configuration) {
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function addInstanceId($id, $configuration = NULL) {
+    parent::addInstanceId($id, $configuration);
+    if ($configuration !== NULL) {
+      $this->setConfiguration($configuration);
+    }
+  }
+
 }

core/modules/image/lib/Drupal/image/ImageEffectBag.php

@@ -42,8 +42,7 @@ public function updateConfiguration(array $configuration) {
       $configuration['uuid'] = $uuid_generator->generate();
     }
     $instance_id = $configuration['uuid'];
-    $this->setInstanceConfiguration($instance_id, $configuration);
-    $this->addInstanceId($instance_id);
+    $this->addInstanceId($instance_id, $configuration);
     return $instance_id;
   }
 

core/tests/Drupal/Tests/Core/Plugin/DefaultPluginBagTest.php

@@ -127,6 +127,34 @@ public function testGetConfiguration() {
     $this->assertSame($expected, array_keys($ids), 'After sorting, the order of the instances is also sorted.');
   }
 
+  /**
+   * Tests the addInstanceId() method.
+   */
+  public function testAddInstanceId() {
+    $this->setupPluginBag($this->exactly(4));
+    $expected = array(
+      'banana' => 'banana',
+      'cherry' => 'cherry',
+      'apple' => 'apple',
+    );
+    $this->defaultPluginBag->addInstanceId('apple');
+    $result = $this->defaultPluginBag->getInstanceIds();
+    $this->assertSame($expected, $result);
+    $this->assertSame($expected, array_intersect_key($result, $this->defaultPluginBag->getConfiguration()));
+
+    $expected = array(
+      'cherry' => 'cherry',
+      'apple' => 'apple',
+      'banana' => 'banana',
+    );
+    $this->defaultPluginBag->removeInstanceId('banana');
+    $this->defaultPluginBag->addInstanceId('banana', $this->config['banana']);
+
+    $result = $this->defaultPluginBag->getInstanceIds();
+    $this->assertSame($expected, $result);
+    $this->assertSame($expected, array_intersect_key($result, $this->defaultPluginBag->getConfiguration()));
+  }
+
   /**
    * Tests the removeInstanceId() method.
    *
@@ -175,27 +203,6 @@ public function testClear() {
     $this->defaultPluginBag->getConfiguration();
   }
 
-  /**
-   * Tests the setInstanceIds() method.
-   */
-  public function testSetInstanceIds() {
-    $this->setupPluginBag($this->any());
-    // Set the instance IDs in a different order than the original.
-    $this->defaultPluginBag->setInstanceIds(array(
-      'apple' => 'apple',
-      'cherry' => 'cherry',
-    ));
-
-    $expected = array(
-      'cherry' => 'cherry',
-      'apple' => 'apple',
-    );
-    $config = $this->defaultPluginBag->getConfiguration();
-    $instance_ids = $this->defaultPluginBag->getInstanceIds();
-    $this->assertSame($expected, $instance_ids);
-    $this->assertSame(array_keys($expected), array_keys($config));
-  }
-
   /**
    * Tests the set() method.
    */