Issue #2361775 by alexpott, swentel: Third party settings dependencies cause config entity deletion

core/config/schema/core.data_types.schema.yml

@@ -269,6 +269,11 @@ config_entity:
     dependencies:
       type: config_dependencies
       label: 'Dependencies'
+    third_party_settings:
+      type: sequence
+      label: 'Third party settings'
+      sequence:
+        - type: '[%parent.%parent.%type].third_party.[%key]'
 
 block_settings:
   type: mapping
@@ -393,11 +398,6 @@ field_config_base:
       label: 'Default value callback'
     settings:
       type: field.field_settings.[%parent.field_type]
-    third_party_settings:
-      type: sequence
-      label: 'Third party settings'
-      sequence:
-        - type: field_config.third_party.[%key]
     field_type:
       type: string
       label: 'Field type'

core/config/schema/core.entity.schema.yml

@@ -93,11 +93,6 @@ core.entity_view_display.*.*.*:
       sequence:
         - type: boolean
           label: 'Value'
-    third_party_settings:
-      type: sequence
-      label: 'Third party settings'
-      sequence:
-        - type: entity_view_display.third_party.[%key]
 
 # Overview configuration information for form mode displays.
 core.entity_form_display.*.*.*:
@@ -146,11 +141,6 @@ core.entity_form_display.*.*.*:
       sequence:
         - type: boolean
           label: 'Component'
-    third_party_settings:
-      type: sequence
-      label: 'Third party settings'
-      sequence:
-        - type: entity_form_display.third_party.[%key]
 
 # Default schema for entity display field with undefined type.
 field.formatter.settings.*:

core/lib/Drupal/Core/Config/ConfigManager.php

@@ -220,8 +220,10 @@ public function uninstall($type, $name) {
       if (isset($entity_dependencies[$type]) && in_array($name, $entity_dependencies[$type])) {
         $affected_dependencies[$type] = array($name);
       }
-      // Inform the entity.
-      $entity->onDependencyRemoval($affected_dependencies);
+      // Inform the entity and, if the entity is changed, re-save it.
+      if ($entity->onDependencyRemoval($affected_dependencies)) {
+        $entity->save();
+      }
     }
 
     // Recalculate the dependencies, some config entities may have fixed their

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

@@ -7,12 +7,12 @@
 
 namespace Drupal\Core\Config\Entity;
 
-use Drupal\Component\Plugin\ConfigurablePluginInterface;
 use Drupal\Component\Utility\String;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Config\Schema\SchemaIncompleteException;
 use Drupal\Core\Entity\Entity;
 use Drupal\Core\Config\ConfigDuplicateUUIDException;
+use Drupal\Core\Config\Entity\ThirdPartySettingsTrait;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Entity\EntityWithPluginCollectionInterface;
@@ -24,7 +24,7 @@
  *
  * @ingroup entity_api
  */
-abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface {
+abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface, ThirdPartySettingsInterface {
 
   use PluginDependencyTrait {
     addDependency as addDependencyTrait;
@@ -87,6 +87,15 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface
    */
   protected $langcode = LanguageInterface::LANGCODE_NOT_SPECIFIED;
 
+  /**
+   * Third party entity settings.
+   *
+   * An array of key/value pairs keyed by provider.
+   *
+   * @var array
+   */
+  protected $third_party_settings = array();
+
   /**
    * Overrides Entity::__construct().
    */
@@ -259,6 +268,9 @@ public function toArray() {
         $properties[$name] = $this->get($name);
       }
     }
+    if (empty($this->third_party_settings)) {
+      unset($properties['third_party_settings']);
+    }
     return $properties;
   }
 
@@ -430,6 +442,13 @@ public function getConfigTarget() {
    * {@inheritdoc}
    */
   public function onDependencyRemoval(array $dependencies) {
+    $changed = FALSE;
+    if (!empty($this->third_party_settings)) {
+      $old_count = count($this->third_party_settings);
+      $this->third_party_settings = array_diff_key($this->third_party_settings, array_flip($dependencies['module']));
+      $changed = $old_count != count($this->third_party_settings);
+    }
+    return $changed;
   }
 
   /**
@@ -452,4 +471,51 @@ protected static function invalidateTagsOnDelete(EntityTypeInterface $entity_typ
     Cache::invalidateTags($entity_type->getListCacheTags());
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function setThirdPartySetting($module, $key, $value) {
+    $this->third_party_settings[$module][$key] = $value;
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getThirdPartySetting($module, $key, $default = NULL) {
+    if (isset($this->third_party_settings[$module][$key])) {
+      return $this->third_party_settings[$module][$key];
+    }
+    else {
+      return $default;
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getThirdPartySettings($module) {
+    return isset($this->third_party_settings[$module]) ? $this->third_party_settings[$module] : array();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function unsetThirdPartySetting($module, $key) {
+    unset($this->third_party_settings[$module][$key]);
+    // If the third party is no longer storing any information, completely
+    // remove the array holding the settings for this module.
+    if (empty($this->third_party_settings[$module])) {
+      unset($this->third_party_settings[$module]);
+    }
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getThirdPartyProviders() {
+    return array_keys($this->third_party_settings);
+  }
+
 }

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

@@ -157,10 +157,13 @@ public function calculateDependencies();
    * This method allows configuration entities to remove dependencies instead
    * of being deleted themselves. Configuration entities can use this method to
    * avoid being unnecessarily deleted during an extension uninstallation.
-   * Implementations should save the entity if dependencies have been
-   * successfully removed. For example, entity displays remove references to
-   * widgets and formatters if the plugin that supplies them depends on a
-   * module that is being uninstalled.
+   * For example, entity displays remove references to widgets and formatters if
+   * the plugin that supplies them depends on a module that is being
+   * uninstalled.
+   *
+   * If this method returns TRUE then the entity needs to be re-saved by the
+   * caller for the changes to take effect. Implementations should not save the
+   * entity.
    *
    * @todo https://www.drupal.org/node/2336727 this method is only fired during
    *   extension uninstallation but it could be used during config entity
@@ -170,6 +173,9 @@ public function calculateDependencies();
    *   An array of dependencies that will be deleted keyed by dependency type.
    *   Dependency types are, for example, entity, module and theme.
    *
+   * @return bool
+   *   TRUE if the entity has changed, FALSE if not.
+   *
    * @see \Drupal\Core\Config\Entity\ConfigDependencyManager
    * @see \Drupal\Core\Config\ConfigManager::uninstall()
    * @see \Drupal\Core\Entity\EntityDisplayBase::onDependencyRemoval()

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

-<?php
-
-/**
- * @file
- * Contains \Drupal\Core\Config\Entity\ThirdPartySettingsTrait.
- */
-
-namespace Drupal\Core\Config\Entity;
-
-/**
- * Provides generic implementation of ThirdPartySettingsInterface.
- *
- * The name of the property used to store third party settings is
- * 'third_party_settings'. You need to provide configuration schema for that
- * setting to ensure it is persisted. See 'third_party_settings' defined on
- * field_config_base and other 'field_config.third_party.*' types.
- *
- * @see \Drupal\Core\Config\Entity\ThirdPartySettingsInterface
- */
-trait ThirdPartySettingsTrait {
-
-  /**
-   * Third party entity settings.
-   *
-   * An array of key/value pairs keyed by provider.
-   *
-   * @var array
-   */
-  protected $third_party_settings = array();
-
-  /**
-   * Sets the value of a third-party setting.
-   *
-   * @param string $module
-   *   The module providing the third-party setting.
-   * @param string $key
-   *   The setting name.
-   * @param mixed $value
-   *   The setting value.
-   *
-   * @return $this
-   */
-  public function setThirdPartySetting($module, $key, $value) {
-    $this->third_party_settings[$module][$key] = $value;
-    return $this;
-  }
-
-  /**
-   * Gets the value of a third-party setting.
-   *
-   * @param string $module
-   *   The module providing the third-party setting.
-   * @param string $key
-   *   The setting name.
-   * @param mixed $default
-   *   The default value
-   *
-   * @return mixed
-   *   The value.
-   */
-  public function getThirdPartySetting($module, $key, $default = NULL) {
-    if (isset($this->third_party_settings[$module][$key])) {
-      return $this->third_party_settings[$module][$key];
-    }
-    else {
-      return $default;
-    }
-  }
-
-  /**
-   * Gets all third-party settings of a given module.
-   *
-   * @param string $module
-   *   The module providing the third-party settings.
-   *
-   * @return array
-   *   An array of key-value pairs.
-   */
-  public function getThirdPartySettings($module) {
-    return isset($this->third_party_settings[$module]) ? $this->third_party_settings[$module] : array();
-  }
-
-  /**
-   * Unsets a third-party setting.
-   *
-   * @param string $module
-   *   The module providing the third-party setting.
-   * @param string $key
-   *   The setting name.
-   *
-   * @return mixed
-   *   The value.
-   */
-  public function unsetThirdPartySetting($module, $key) {
-    unset($this->third_party_settings[$module][$key]);
-    // If the third party is no longer storing any information, completely
-    // remove the array holding the settings for this module.
-    if (empty($this->third_party_settings[$module])) {
-      unset($this->third_party_settings[$module]);
-    }
-    return $this;
-  }
-
-  /**
-   * Gets the list of third parties that store information.
-   *
-   * @return array
-   *   The list of third parties.
-   */
-  public function getThirdPartyProviders() {
-    return array_keys($this->third_party_settings);
-  }
-
-}

core/lib/Drupal/Core/Config/TypedConfigManager.php

@@ -240,6 +240,8 @@ protected function replaceName($name, $data) {
    * their value or some of these special strings:
    * - '%key', will be replaced by the element's key.
    * - '%parent', to reference the parent element.
+   * - '%type', to reference the schema definition type. Can only be used in
+   *   combination with %parent.
    *
    * There may be nested configuration keys separated by dots or more complex
    * patterns like '%parent.name' which references the 'name' value of the
@@ -249,6 +251,9 @@ protected function replaceName($name, $data) {
    * - 'name.subkey', indicates a nested value of the current element.
    * - '%parent.name', will be replaced by the 'name' value of the parent.
    * - '%parent.%key', will be replaced by the parent element's key.
+   * - '%parent.%type', will be replaced by the schema type of the parent.
+   * - '%parent.%parent.%type', will be replaced by the schema type of the
+   *   parent's parent.
    *
    * @param string $value
    *   Variable value to be replaced.
@@ -273,9 +278,11 @@ protected function replaceVariable($value, $data) {
       else {
         // Get nested value and continue processing.
         if ($name == '%parent') {
+          /** @var \Drupal\Core\Config\Schema\ArrayElement $parent */
           // Switch replacement values with values from the parent.
           $parent = $data['%parent'];
           $data = $parent->getValue();
+          $data['%type'] = $parent->getDataDefinition()->getDataType();
           // The special %parent and %key values now need to point one level up.
           if ($new_parent = $parent->getParent()) {
             $data['%parent'] = $new_parent;

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

@@ -8,7 +8,6 @@
 namespace Drupal\Core\Entity;
 
 use Drupal\Core\Config\Entity\ConfigEntityBase;
-use Drupal\Core\Config\Entity\ThirdPartySettingsTrait;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Entity\Display\EntityDisplayInterface;
 use Drupal\field\Entity\FieldConfig;
@@ -20,8 +19,6 @@
  */
 abstract class EntityDisplayBase extends ConfigEntityBase implements EntityDisplayInterface {
 
-  use ThirdPartySettingsTrait;
-
   /**
    * The 'mode' for runtime EntityDisplay objects used to render entities with
    * arbitrary display options rather than a configured view mode or form mode.
@@ -418,7 +415,7 @@ private function fieldHasDisplayOptions(FieldDefinitionInterface $definition) {
    * {@inheritdoc}
    */
   public function onDependencyRemoval(array $dependencies) {
-    $changed = FALSE;
+    $changed = parent::onDependencyRemoval($dependencies);
     foreach ($dependencies['config'] as $entity) {
       if ($entity instanceof FieldConfigInterface) {
         // Remove components for fields that are being deleted.
@@ -437,9 +434,7 @@ public function onDependencyRemoval(array $dependencies) {
         }
       }
     }
-    if ($changed) {
-      $this->save();
-    }
+    return $changed;
   }
 
   /**

core/lib/Drupal/Core/Field/FieldConfigBase.php

@@ -9,7 +9,6 @@
 
 use Drupal\Component\Plugin\DependentPluginInterface;
 use Drupal\Core\Config\Entity\ConfigEntityBase;
-use Drupal\Core\Config\Entity\ThirdPartySettingsTrait;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\FieldableEntityInterface;
 use Drupal\Core\Field\TypedData\FieldItemDataDefinition;
@@ -20,8 +19,6 @@
  */
 abstract class FieldConfigBase extends ConfigEntityBase implements FieldConfigInterface {
 
-  use ThirdPartySettingsTrait;
-
   /**
    * The field ID.
    *

core/modules/config/src/Tests/ConfigSchemaTest.php

@@ -26,7 +26,7 @@ class ConfigSchemaTest extends KernelTestBase {
    *
    * @var array
    */
-  public static $modules = array('system', 'language', 'locale', 'field', 'image', 'config_schema_test');
+  public static $modules = array('system', 'language', 'locale', 'field', 'image', 'config_test', 'config_schema_test');
 
   /**
    * {@inheritdoc}
@@ -172,7 +172,7 @@ function testSchemaMapping() {
     $expected['mapping']['effects']['sequence'][0]['mapping']['uuid']['type'] = 'string';
     $expected['mapping']['third_party_settings']['type'] = 'sequence';
     $expected['mapping']['third_party_settings']['label'] = 'Third party settings';
-    $expected['mapping']['third_party_settings']['sequence'][0]['type'] = 'image_style.third_party.[%key]';
+    $expected['mapping']['third_party_settings']['sequence'][0]['type'] = '[%parent.%parent.%type].third_party.[%key]';
     $expected['type'] = 'image.style.*';
 
     $this->assertEqual($definition, $expected);
@@ -203,6 +203,19 @@ function testSchemaMapping() {
 
     $this->assertEqual($definition, $expected, 'Retrieved the right metadata for the first effect of image.style.medium');
 
+    $test = \Drupal::service('config.typed')->get('config_test.dynamic.third_party')->get('third_party_settings.config_schema_test');
+    $definition = $test->getDataDefinition()->toArray();
+    $expected = array();
+    $expected['type'] = 'config_test.dynamic.*.third_party.config_schema_test';
+    $expected['label'] = 'Mapping';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Mapping';
+    $expected['definition_class'] = '\Drupal\Core\TypedData\MapDataDefinition';
+    $expected['mapping'] = [
+      'integer' => ['type' => 'integer'],
+      'string' => ['type' => 'string'],
+    ];
+    $this->assertEqual($definition, $expected, 'Retrieved the right metadata for config_test.dynamic.third_party:third_party_settings.config_schema_test');
+
     // More complex, several level deep test.
     $definition = \Drupal::service('config.typed')->getDefinition('config_schema_test.someschema.somemodule.section_one.subsection');
     // This should be the schema of config_schema_test.someschema.somemodule.*.*.

core/modules/config/tests/config_schema_test/config/install/config_test.dynamic.third_party.yml

+id: third_party
+label: Default
+weight: 0
+protected_property: Default
+third_party_settings:
+  config_schema_test:
+    integer: 1
+    string: 'string'

core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml

@@ -209,3 +209,11 @@ test_with_parents.plugin_types.*:
 
 config_schema_test.hook:
   type: string
+
+config_test.dynamic.*.third_party.config_schema_test:
+  type: mapping
+  mapping:
+    integer:
+      type: integer
+    string:
+      type: string

core/modules/config/tests/config_test/src/Entity/ConfigTest.php

@@ -122,7 +122,7 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
    * {@inheritdoc}
    */
   public function onDependencyRemoval(array $dependencies) {
-    $changed = FALSE;
+    $changed = parent::onDependencyRemoval($dependencies);
     $fix_deps = \Drupal::state()->get('config_test.fix_dependencies', array());
     foreach ($dependencies['config'] as $entity) {
       if (in_array($entity->getConfigDependencyName(), $fix_deps)) {
@@ -133,9 +133,7 @@ public function onDependencyRemoval(array $dependencies) {
         }
       }
     }
-    if ($changed) {
-      $this->save();
-    }
+    return $changed;
   }
 
   /**

core/modules/contact/config/schema/contact.schema.yml

@@ -22,11 +22,6 @@ contact.form.*:
     weight:
       type: integer
       label: 'Weight'
-    third_party_settings:
-      type: sequence
-      label: 'Third party settings'
-      sequence:
-        - type: contact_form.third_party.[%key]
 
 contact.settings:
   type: mapping

core/modules/contact/src/Entity/ContactForm.php

@@ -9,7 +9,6 @@
 
 use Drupal\Core\Config\Entity\ConfigEntityBundleBase;
 use Drupal\contact\ContactFormInterface;
-use Drupal\Core\Config\Entity\ThirdPartySettingsTrait;
 
 /**
  * Defines the contact form entity.
@@ -42,8 +41,6 @@
  */
 class ContactForm extends ConfigEntityBundleBase implements ContactFormInterface {
 
-  use ThirdPartySettingsTrait;
-
   /**
    * The form ID.
    *

core/modules/contact/tests/modules/contact_storage_test/config/schema/contact_storage_test.schema.yml

 # Schema for configuration files of the Contact Storage Test module.
 
-contact_form.third_party.contact_storage_test:
+contact.form.*.third_party.contact_storage_test:
   type: mapping
   label: 'Per-contact form storage settings'
   mapping:

core/modules/content_translation/config/schema/content_translation.schema.yml

 # Schema for the Content Translation module.
 
-field_config.third_party.content_translation:
+field.field.*.*.*.third_party.content_translation:
   type: mapping
   label: 'Content translation field settings'
   mapping:
@@ -11,7 +11,7 @@ field_config.third_party.content_translation:
         - type: string
           label: 'Field column for which to synchronize translations'
 
-language.content_settings.third_party.content_translation:
+language.content_settings.*.*.third_party.content_translation:
   type: mapping
   label: 'Content translation content settings'
   mapping:

core/modules/filter/src/Entity/FilterFormat.php

@@ -400,7 +400,7 @@ public function removeFilter($instance_id) {
    * {@inheritdoc}
    */
   public function onDependencyRemoval(array $dependencies) {
-    $changed = FALSE;
+    $changed = parent::onDependencyRemoval($dependencies);
     $filters = $this->filters();
     foreach ($filters as $filter) {
       // Remove disabled filters, so that this FilterFormat config entity can
@@ -410,9 +410,7 @@ public function onDependencyRemoval(array $dependencies) {
         $changed = TRUE;
       }
     }
-    if ($changed) {
-      $this->save();
-    }
+    return $changed;
   }
 
   /**

core/modules/image/config/schema/image.schema.yml

@@ -22,11 +22,6 @@ image.style.*:
               type: integer
             uuid:
               type: string
-    third_party_settings:
-      type: sequence
-      label: 'Third party settings'
-      sequence:
-        - type: image_style.third_party.[%key]
 
 image.effect.*:
   type: mapping

core/modules/image/src/Entity/ImageStyle.php

@@ -9,7 +9,6 @@
 
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Config\Entity\ConfigEntityBase;
-use Drupal\Core\Config\Entity\ThirdPartySettingsTrait;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityWithPluginCollectionInterface;
 use Drupal\Core\Routing\RequestHelper;
@@ -54,8 +53,6 @@
  */
 class ImageStyle extends ConfigEntityBase implements ImageStyleInterface, EntityWithPluginCollectionInterface {
 
-  use ThirdPartySettingsTrait;
-
   /**
    * The name of the image style to use as replacement upon delete.
    *

core/modules/image/tests/modules/image_module_test/config/schema/image_module_test.schema.yml

-image_style.third_party.image_module_test:
+image.style.*.third_party.image_module_test:
   type: mapping
   label: 'Schema for image_module_test module additions to image_style entity'
   mapping: