Issue #2195753 by tim.plunkett: Changes to config entities that use plugins...

Issue #2195753 by tim.plunkett: Changes to config entities that use plugins are not propagated to the plugins.

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

@@ -101,11 +101,7 @@ public function sortHelper($aID, $bID) {
   }
 
   /**
-   * Returns the current configuration of all plugins in this bag.
-   *
-   * @return array
-   *   An associative array keyed by instance ID, whose values are up-to-date
-   *   plugin configurations.
+   * {@inheritdoc}
    */
   public function getConfiguration() {
     $instances = array();
@@ -129,6 +125,16 @@ public function getConfiguration() {
     return $instances;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function setConfiguration($configuration) {
+    foreach ($configuration as $instance_id => $instance_configuration) {
+      $this->setInstanceConfiguration($instance_id, $instance_configuration);
+    }
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -149,7 +155,7 @@ public function setInstanceIds(array $instance_ids) {
    * @param array $configuration
    *   The plugin configuration to set.
    */
-  public function setConfiguration($instance_id, array $configuration) {
+  public function setInstanceConfiguration($instance_id, array $configuration) {
     $this->configurations[$instance_id] = $configuration;
     $instance = $this->get($instance_id);
     if ($instance instanceof ConfigurablePluginInterface) {

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

@@ -7,8 +7,6 @@
 
 namespace Drupal\Component\Plugin;
 
-use Drupal\Component\Utility\MapArray;
-
 /**
  * Provides a default plugin bag for a plugin type.
  *
@@ -34,19 +32,28 @@ class DefaultSinglePluginBag extends PluginBag {
    */
   protected $configuration;
 
+  /**
+   * The instance ID used for this plugin bag.
+   *
+   * @var string
+   */
+  protected $instanceId;
+
   /**
    * Constructs a new DefaultSinglePluginBag object.
    *
    * @param \Drupal\Component\Plugin\PluginManagerInterface $manager
    *   The manager to be used for instantiating plugins.
-   * @param array $instance_ids
-   *   The IDs of the plugin instances with which we are dealing.
+   * @param string $instance_id
+   *   The ID of the plugin instance.
    * @param array $configuration
    *   An array of configuration.
    */
-  public function __construct(PluginManagerInterface $manager, array $instance_ids, array $configuration) {
+  public function __construct(PluginManagerInterface $manager, $instance_id, array $configuration) {
     $this->manager = $manager;
-    $this->instanceIDs = MapArray::copyValuesToKeys($instance_ids);
+    $this->instanceId = $instance_id;
+    // This is still needed by the parent PluginBag class.
+    $this->instanceIDs = array($instance_id => $instance_id);
     $this->configuration = $configuration;
   }
 
@@ -57,4 +64,29 @@ protected function initializePlugin($instance_id) {
     $this->set($instance_id, $this->manager->createInstance($instance_id, $this->configuration));
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getConfiguration() {
+    $plugin = $this->get($this->instanceId);
+    if ($plugin instanceof ConfigurablePluginInterface) {
+      return $plugin->getConfiguration();
+    }
+    else {
+      return $this->configuration;
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setConfiguration($configuration) {
+    $plugin = $this->get($this->instanceId);
+    if ($plugin instanceof ConfigurablePluginInterface) {
+      $plugin->setConfiguration($configuration);
+    }
+    $this->configuration = $configuration;
+    return $this;
+  }
+
 }

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

@@ -34,6 +34,24 @@ abstract class PluginBag implements \Iterator, \Countable {
    */
   abstract protected function initializePlugin($instance_id);
 
+  /**
+   * Returns the current configuration of all plugins in this bag.
+   *
+   * @return array
+   *   An array of up-to-date plugin configuration.
+   */
+  abstract public function getConfiguration();
+
+  /**
+   * Sets the configuration for all plugins in this bag.
+   *
+   * @param array $configuration
+   *   An array of up-to-date plugin configuration.
+   *
+   * @return $this
+   */
+  abstract public function setConfiguration($configuration);
+
   /**
    * Clears all instantiated plugins.
    */

core/lib/Drupal/Core/Action/ActionInterface.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\Core\Action;
 
+use Drupal\Component\Plugin\PluginInspectionInterface;
 use Drupal\Core\Executable\ExecutableInterface;
 
 /**
@@ -15,7 +16,7 @@
  * @see \Drupal\Core\Annotation\Action
  * @see \Drupal\Core\Action\ActionManager
  */
-interface ActionInterface extends ExecutableInterface {
+interface ActionInterface extends ExecutableInterface, PluginInspectionInterface {
 
   /**
    * Executes the plugin for an array of objects.

core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php

@@ -27,6 +27,20 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface
    */
   protected $originalId;
 
+  /**
+   * The name of the property that is used to store plugin configuration.
+   *
+   * This is needed when the entity utilizes a PluginBag, to dictate where the
+   * plugin configuration should be stored.
+   *
+   * @todo Move this to a trait along with
+   *   \Drupal\Core\Config\Entity\EntityWithPluginBagInterface, and give it a
+   *   default value of 'configuration'.
+   *
+   * @var string
+   */
+  protected $pluginConfigKey;
+
   /**
    * The enabled/disabled status of the configuration entity.
    *
@@ -101,6 +115,15 @@ public function get($property_name) {
    * {@inheritdoc}
    */
   public function set($property_name, $value) {
+    // @todo When \Drupal\Core\Config\Entity\EntityWithPluginBagInterface moves
+    //   to a trait, switch to class_uses() instead.
+    if ($this instanceof EntityWithPluginBagInterface) {
+      if ($property_name == $this->pluginConfigKey) {
+        // If external code updates the settings, pass it along to the plugin.
+        $this->getPluginBag()->setConfiguration($value);
+      }
+    }
+
     $this->{$property_name} = $value;
   }
 
@@ -192,6 +215,14 @@ public function getExportProperties() {
   public function preSave(EntityStorageControllerInterface $storage_controller) {
     parent::preSave($storage_controller);
 
+    // @todo When \Drupal\Core\Config\Entity\EntityWithPluginBagInterface moves
+    //   to a trait, switch to class_uses() instead.
+    if ($this instanceof EntityWithPluginBagInterface) {
+      // Any changes to the plugin configuration must be saved to the entity's
+      // copy as well.
+      $this->set($this->pluginConfigKey, $this->getPluginBag()->getConfiguration());
+    }
+
     // Ensure this entity's UUID does not exist with a different ID, regardless
     // of whether it's new or updated.
     $matching_entities = $storage_controller->getQuery()

core/lib/Drupal/Core/Config/Entity/EntityWithPluginBagInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\Entity\EntityWithPluginBagInterface.
+ */
+
+namespace Drupal\Core\Config\Entity;
+
+/**
+ * Provides an interface for an object utilizing a plugin bag.
+ *
+ * @see \Drupal\Component\Plugin\PluginBag
+ *
+ * @todo Turn this into a trait.
+ */
+interface EntityWithPluginBagInterface extends ConfigEntityInterface {
+
+  /**
+   * Returns the plugin bag used by this entity.
+   *
+   * @return \Drupal\Component\Plugin\PluginBag
+   *
+   * @todo Make this protected.
+   */
+  public function getPluginBag();
+
+}

core/lib/Drupal/Core/Entity/EntityFormController.php

@@ -359,15 +359,7 @@ public function buildEntity(array $form, array &$form_state) {
     // controller of the current request.
     $form_state['controller'] = $this;
 
-    // Copy top-level form values to entity properties, without changing
-    // existing entity properties that are not being edited by
-    // this form.
-    // @todo: This relies on a method that only exists for config and content
-    //   entities, in a different way. Consider moving this logic to a config
-    //   entity specific implementation.
-    foreach ($form_state['values'] as $key => $value) {
-      $entity->set($key, $value);
-    }
+    $this->copyFormValuesToEntity($entity, $form_state);
 
     // Invoke all specified builders for copying form values to entity
     // properties.
@@ -380,6 +372,26 @@ public function buildEntity(array $form, array &$form_state) {
     return $entity;
   }
 
+  /**
+   * Copies top-level form values to entity properties
+   *
+   * This should not change existing entity properties that are not being edited
+   * by this form.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity the current form should operate upon.
+   * @param array $form_state
+   *   An associative array containing the current state of the form.
+   */
+  protected function copyFormValuesToEntity(EntityInterface $entity, array $form_state) {
+    // @todo: This relies on a method that only exists for config and content
+    //   entities, in a different way. Consider moving this logic to a config
+    //   entity specific implementation.
+    foreach ($form_state['values'] as $key => $value) {
+      $entity->set($key, $value);
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/block/lib/Drupal/block/BlockBase.php

@@ -108,6 +108,10 @@ public function buildConfigurationForm(array $form, array &$form_state) {
       '#default_value' => ($this->configuration['label_display'] === BlockInterface::BLOCK_LABEL_VISIBLE),
       '#return_value' => BlockInterface::BLOCK_LABEL_VISIBLE,
     );
+    $form['cache'] = array(
+      '#type' => 'value',
+      '#value' => $this->configuration['cache'],
+    );
 
     // Add plugin-specific settings for this block type.
     $form += $this->blockForm($form, $form_state);

core/modules/block/lib/Drupal/block/BlockPluginBag.php

@@ -25,10 +25,19 @@ class BlockPluginBag extends DefaultSinglePluginBag {
   protected $blockId;
 
   /**
-   * {@inheritdoc}
+   * Constructs a new BlockPluginBag.
+   *
+   * @param \Drupal\Component\Plugin\PluginManagerInterface $manager
+   *   The manager to be used for instantiating plugins.
+   * @param string $instance_id
+   *   The ID of the plugin instance.
+   * @param array $configuration
+   *   An array of configuration.
+   * @param string $block_id
+   *   The unique ID of the block entity using this plugin.
    */
-  public function __construct(PluginManagerInterface $manager, array $instance_ids, array $configuration, $block_id) {
-    parent::__construct($manager, $instance_ids, $configuration);
+  public function __construct(PluginManagerInterface $manager, $instance_id, array $configuration, $block_id) {
+    parent::__construct($manager, $instance_id, $configuration);
 
     $this->blockId = $block_id;
   }

core/modules/block/lib/Drupal/block/Entity/Block.php

@@ -10,7 +10,7 @@
 use Drupal\Core\Config\Entity\ConfigEntityBase;
 use Drupal\block\BlockPluginBag;
 use Drupal\block\BlockInterface;
-use Drupal\Core\Entity\EntityStorageControllerInterface;
+use Drupal\Core\Config\Entity\EntityWithPluginBagInterface;
 
 /**
  * Defines a Block configuration entity class.
@@ -40,7 +40,7 @@
  *   }
  * )
  */
-class Block extends ConfigEntityBase implements BlockInterface {
+class Block extends ConfigEntityBase implements BlockInterface, EntityWithPluginBagInterface {
 
   /**
    * The ID of the block.
@@ -91,6 +91,11 @@ class Block extends ConfigEntityBase implements BlockInterface {
    */
   protected $pluginBag;
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $pluginConfigKey = 'settings';
+
   /**
    * The visibility settings.
    *
@@ -99,19 +104,20 @@ class Block extends ConfigEntityBase implements BlockInterface {
   protected $visibility;
 
   /**
-   * Overrides \Drupal\Core\Config\Entity\ConfigEntityBase::__construct();
+   * {@inheritdoc}
    */
-  public function __construct(array $values, $entity_type) {
-    parent::__construct($values, $entity_type);
-
-    $this->pluginBag = new BlockPluginBag(\Drupal::service('plugin.manager.block'), array($this->plugin), $this->get('settings'), $this->id());
+  public function getPlugin() {
+    return $this->getPluginBag()->get($this->plugin);
   }
 
   /**
    * {@inheritdoc}
    */
-  public function getPlugin() {
-    return $this->pluginBag->get($this->plugin);
+  public function getPluginBag() {
+    if (!$this->pluginBag) {
+      $this->pluginBag = new BlockPluginBag(\Drupal::service('plugin.manager.block'), $this->plugin, $this->get('settings'), $this->id());
+    }
+    return $this->pluginBag;
   }
 
   /**
@@ -147,15 +153,6 @@ public function getExportProperties() {
     return $properties;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function preSave(EntityStorageControllerInterface $storage_controller) {
-    parent::preSave($storage_controller);
-
-    $this->set('settings', $this->getPlugin()->getConfiguration());
-  }
-
   /**
    * Sorts active blocks by weight; sorts inactive blocks by name.
    */

core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php

@@ -83,6 +83,10 @@ public function testBlockInterface() {
         '#default_value' => TRUE,
         '#return_value' => 'visible',
       ),
+      'cache' => array(
+        '#type' => 'value',
+        '#value' => DRUPAL_NO_CACHE,
+      ),
       'display_message' => array(
         '#type' => 'textfield',
         '#title' => t('Display message'),

core/modules/filter/lib/Drupal/filter/Entity/FilterFormat.php

@@ -9,6 +9,7 @@
 
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Config\Entity\ConfigEntityBase;
+use Drupal\Core\Config\Entity\EntityWithPluginBagInterface;
 use Drupal\Core\Entity\EntityStorageControllerInterface;
 use Drupal\filter\FilterFormatInterface;
 use Drupal\filter\FilterBag;
@@ -44,7 +45,7 @@
  *   }
  * )
  */
-class FilterFormat extends ConfigEntityBase implements FilterFormatInterface {
+class FilterFormat extends ConfigEntityBase implements FilterFormatInterface, EntityWithPluginBagInterface {
 
   /**
    * Unique machine name of the format.
@@ -133,6 +134,11 @@ class FilterFormat extends ConfigEntityBase implements FilterFormatInterface {
    */
   protected $filterBag;
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $pluginConfigKey = 'filters';
+
   /**
    * {@inheritdoc}
    */
@@ -144,12 +150,20 @@ public function id() {
    * {@inheritdoc}
    */
   public function filters($instance_id = NULL) {
+    $filter_bag = $this->getPluginBag();
+    if (isset($instance_id)) {
+      return $filter_bag->get($instance_id);
+    }
+    return $filter_bag;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getPluginBag() {
     if (!isset($this->filterBag)) {
       $this->filterBag = new FilterBag(\Drupal::service('plugin.manager.filter'), $this->filters);
     }
-    if (isset($instance_id)) {
-      return $this->filterBag->get($instance_id);
-    }
     return $this->filterBag;
   }
 
@@ -159,7 +173,7 @@ public function filters($instance_id = NULL) {
   public function setFilterConfig($instance_id, array $configuration) {
     $this->filters[$instance_id] = $configuration;
     if (isset($this->filterBag)) {
-      $this->filterBag->setConfiguration($instance_id, $configuration);
+      $this->filterBag->setInstanceConfiguration($instance_id, $configuration);
     }
     return $this;
   }
@@ -169,9 +183,13 @@ public function setFilterConfig($instance_id, array $configuration) {
    */
   public function getExportProperties() {
     $properties = parent::getExportProperties();
-    // Sort and export the configuration of all filters.
-    $properties['filters'] = $this->filters()->sort()->getConfiguration();
-
+    // @todo Make self::$weight and self::$cache protected and add them here.
+    $names = array(
+      'filters',
+    );
+    foreach ($names as $name) {
+      $properties[$name] = $this->get($name);
+    }
     return $properties;
   }
 
@@ -195,6 +213,9 @@ public function disable() {
    * {@inheritdoc}
    */
   public function preSave(EntityStorageControllerInterface $storage_controller) {
+    // Ensure the filters have been sorted before saving.
+    $this->filters()->sort();
+
     parent::preSave($storage_controller);
 
     $this->name = trim($this->label());

core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php

@@ -18,8 +18,8 @@
  *   type = Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR,
  *   settings = {
  *     "allowed_html" = "<a> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd> <h4> <h5> <h6>",
- *     "filter_html_help" = 1,
- *     "filter_html_nofollow" = 0
+ *     "filter_html_help" = TRUE,
+ *     "filter_html_nofollow" = FALSE
  *   },
  *   weight = -10
  * )

core/modules/image/lib/Drupal/image/Entity/ImageStyle.php

@@ -8,6 +8,7 @@
 namespace Drupal\image\Entity;
 
 use Drupal\Core\Config\Entity\ConfigEntityBase;
+use Drupal\Core\Config\Entity\EntityWithPluginBagInterface;
 use Drupal\Core\Entity\EntityStorageControllerInterface;
 use Drupal\image\ImageEffectBag;
 use Drupal\image\ImageEffectInterface;
@@ -45,7 +46,7 @@
  *   }
  * )
  */
-class ImageStyle extends ConfigEntityBase implements ImageStyleInterface {
+class ImageStyle extends ConfigEntityBase implements ImageStyleInterface, EntityWithPluginBagInterface {
 
   /**
    * The name of the image style to use as replacement upon delete.
@@ -89,6 +90,11 @@ class ImageStyle extends ConfigEntityBase implements ImageStyleInterface {
    */
   protected $effectsBag;
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $pluginConfigKey = 'effects';
+
   /**
    * Overrides Drupal\Core\Entity\Entity::id().
    */
@@ -355,6 +361,13 @@ public function getEffects() {
     return $this->effectsBag;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getPluginBag() {
+    return $this->getEffects();
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -369,7 +382,12 @@ public function saveImageEffect(array $configuration) {
    */
   public function getExportProperties() {
     $properties = parent::getExportProperties();
-    $properties['effects'] = $this->getEffects()->getConfiguration();
+    $names = array(
+      'effects',
+    );
+    foreach ($names as $name) {
+      $properties[$name] = $this->get($name);
+    }
     return $properties;
   }
 
@@ -394,4 +412,5 @@ public function setName($name) {
     $this->set('name', $name);
     return $this;
   }
+
 }

core/modules/image/lib/Drupal/image/Form/ImageStyleEditForm.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\image\Form;
 
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityStorageControllerInterface;
 use Drupal\image\ConfigurableImageEffectInterface;
 use Drupal\image\ImageEffectManager;
@@ -248,4 +249,16 @@ protected function updateEffectWeights(array $effects) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function copyFormValuesToEntity(EntityInterface $entity, array $form_state) {
+    foreach ($form_state['values'] as $key => $value) {
+      // Do not copy effects here, see self::updateEffectWeights().
+      if ($key != 'effects') {
+        $entity->set($key, $value);
+      }
+    }
+  }
+
 }

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

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

core/modules/search/lib/Drupal/search/Entity/SearchPage.php

@@ -8,6 +8,7 @@
 namespace Drupal\search\Entity;
 
 use Drupal\Core\Config\Entity\ConfigEntityBase;
+use Drupal\Core\Config\Entity\EntityWithPluginBagInterface;
 use Drupal\Core\Entity\EntityStorageControllerInterface;
 use Drupal\Component\Plugin\ConfigurablePluginInterface;
 use Drupal\search\Plugin\SearchIndexingInterface;
@@ -49,7 +50,7 @@
  *   }
  * )
  */
-class SearchPage extends ConfigEntityBase implements SearchPageInterface {
+class SearchPage extends ConfigEntityBase implements SearchPageInterface, EntityWithPluginBagInterface {
 
   /**
    * The name (plugin ID) of the search page entity.
@@ -112,17 +113,23 @@ class SearchPage extends ConfigEntityBase implements SearchPageInterface {
   /**
    * {@inheritdoc}
    */
-  public function __construct(array $values, $entity_type) {
-    parent::__construct($values, $entity_type);
+  protected $pluginConfigKey = 'configuration';
 
-    $this->pluginBag = new SearchPluginBag($this->searchPluginManager(), array($this->plugin), $this->configuration, $this->id());
+  /**
+   * {@inheritdoc}
+   */
+  public function getPlugin() {
+    return $this->getPluginBag()->get($this->plugin);
   }
 
   /**
    * {@inheritdoc}
    */