Issue #1831774 by damiankloip, tim.plunkett, Gábor Hojtsy, alexpott: Fixed Config import assumes that 'config_prefix' contains one dot only.

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

@@ -15,6 +15,18 @@ use Drupal\Core\Config\Config;
 
 /**
  * Defines the storage controller class for configuration entities.
+ *
+ * Configuration object names of configuration entities are comprised of two
+ * parts, separated by a dot:
+ * - config_prefix: A string denoting the owner (module/extension) of the
+ *   configuration object, followed by arbitrary other namespace identifiers
+ *   that are declared by the owning extension; e.g., 'node.type'. The
+ *   config_prefix does NOT contain a trailing dot. It is defined by the entity
+ *   type's annotation.
+ * - ID: A string denoting the entity ID within the entity type namespace; e.g.,
+ *   'article'. Entity IDs may contain dots/periods. The entire remaining string
+ *   after the config_prefix in a config name forms the entity ID. Additional or
+ *   custom suffixes are not possible.
  */
 class ConfigStorageController implements EntityStorageControllerInterface {
 
@@ -161,6 +173,22 @@ class ConfigStorageController implements EntityStorageControllerInterface {
     return $this->entityInfo['config_prefix'] . '.';
   }
 
+  /**
+   * Extracts the configuration entity ID from the full configuration name.
+   *
+   * @param string $config_name
+   *   The full configuration name to extract the ID from. E.g.
+   *   'views.view.archive'.
+   * @param string $config_prefix
+   *   The config prefix of the configuration entity. E.g. 'views.view'
+   *
+   * @return string
+   *   The ID of the configuration entity.
+   */
+  public static function getIDFromConfigName($config_name, $config_prefix) {
+    return substr($config_name, strlen($config_prefix . '.'));
+  }
+
   /**
    * Builds the query to load the entity.
    *
@@ -288,10 +316,14 @@ class ConfigStorageController implements EntityStorageControllerInterface {
       $config = config($this->getConfigPrefix() . $entity->id());
       $config->delete();
 
-      // Remove the entity from the manifest file.
-      config('manifest.' . $this->entityInfo['config_prefix'])
-        ->clear($entity->id())
-        ->save();
+      // Remove the entity from the manifest file. Entity id's can contain a dot
+      // so we can not use Config::clear() to remove the entity from the
+      // manifest.
+      $manifest = config('manifest.' . $this->entityInfo['config_prefix']);
+      $manifest_data = $manifest->get();
+      unset($manifest_data[$entity->id()]);
+      $manifest->setData($manifest_data);
+      $manifest->save();
     }
 
     $this->postDelete($entities);
@@ -483,7 +515,7 @@ class ConfigStorageController implements EntityStorageControllerInterface {
    *   A configuration object containing the old configuration data.
    */
   public function importChange($name, Config $new_config, Config $old_config) {
-    list(, , $id) = explode('.', $name);
+    $id = static::getIDFromConfigName($name, $this->entityInfo['config_prefix']);
     $entities = $this->load(array($id));
     $entity = $entities[$id];
     $entity->original = clone $entity;
@@ -514,7 +546,7 @@ class ConfigStorageController implements EntityStorageControllerInterface {
    *   A configuration object containing the old configuration data.
    */
   public function importDelete($name, Config $new_config, Config $old_config) {
-    list(, , $id) = explode('.', $name);
+    $id = static::getIDFromConfigName($name, $this->entityInfo['config_prefix']);
     $entities = $this->load(array($id));
     $entity = $entities[$id];
     $entity->delete();

core/modules/breakpoint/breakpoint.module

@@ -7,6 +7,7 @@
 
 use Drupal\breakpoint\Plugin\Core\Entity\Breakpoint;
 use Drupal\breakpoint\Plugin\Core\Entity\BreakpointGroup;
+use Drupal\Core\Config\Entity\ConfigStorageController;
 
 /**
  * Implements hook_help().
@@ -191,7 +192,7 @@ function _breakpoint_delete_breakpoints($list, $source_type) {
 
   // Remove the breakpoint.breakpoint part of the breakpoint identifier.
   foreach ($ids as &$id) {
-    $id = drupal_substr($id, drupal_strlen($entity_info['config_prefix']) + 1);
+    $id = ConfigStorageController::getIDFromConfigName($id, $entity_info['config_prefix']);
   }
   $breakpoint_groups = entity_load_multiple('breakpoint_group', $ids);
 
@@ -206,7 +207,7 @@ function _breakpoint_delete_breakpoints($list, $source_type) {
 
       // Remove the breakpoint.breakpoint part of the breakpoint identifier.
       foreach ($breakpoint_ids as &$breakpoint_id) {
-        $breakpoint_id = drupal_substr($breakpoint_id, drupal_strlen($entity_info['config_prefix']) + 1);
+        $breakpoint_id = ConfigStorageController::getIDFromConfigName($breakpoint_id, $entity_info['config_prefix']);
       }
       $breakpoints = entity_load_multiple('breakpoint', $breakpoint_ids);
 

core/modules/config/lib/Drupal/config/Tests/ConfigEntityListTest.php

@@ -43,10 +43,10 @@ class ConfigEntityListTest extends WebTestBase {
 
     // Get a list of ConfigTest entities and confirm that it contains the
     // ConfigTest entity provided by the config_test module.
-    // @see config_test.dynamic.default.yml
+    // @see config_test.dynamic.dotted.default.yml
     $list = $controller->load();
     $this->assertEqual(count($list), 1, '1 ConfigTest entity found.');
-    $entity = $list['default'];
+    $entity = $list['dotted.default'];
     $this->assertTrue(!empty($entity), '"Default" ConfigTest entity ID found.');
     $this->assertTrue($entity instanceof ConfigTest, '"Default" ConfigTest entity is an instance of ConfigTest.');
 
@@ -91,7 +91,7 @@ class ConfigEntityListTest extends WebTestBase {
     $build_operations = $controller->buildOperations($entity);
     $expected_items = array(
       'label' => 'Default',
-      'id' => 'default',
+      'id' => 'dotted.default',
       'operations' => array(
         'data' => $build_operations,
       ),
@@ -135,7 +135,7 @@ class ConfigEntityListTest extends WebTestBase {
     // the second contains the machine name, and the third contains the
     // operations list.
     $this->assertIdentical((string) $elements[0], 'Default');
-    $this->assertIdentical((string) $elements[1], 'default');
+    $this->assertIdentical((string) $elements[1], 'dotted.default');
     $this->assertTrue($elements[2]->children()->xpath('//ul'), 'Operations list found.');
 
     // Add a new entity using the operations link.
@@ -189,7 +189,7 @@ class ConfigEntityListTest extends WebTestBase {
     // Verify that the text of the label and machine name does not appear in
     // the list (though it may appear elsewhere on the page).
     $this->assertNoFieldByXpath('//td', 'Default', "No label found for deleted 'Default' entity.");
-    $this->assertNoFieldByXpath('//td', 'default', "No machine name found for deleted 'Default' entity.");
+    $this->assertNoFieldByXpath('//td', 'dotted.default', "No machine name found for deleted 'Default' entity.");
 
     // Confirm that the empty text is displayed.
     $this->assertText('There is no Test configuration yet.');

core/modules/config/lib/Drupal/config/Tests/ConfigEntityUnitTest.php

@@ -38,6 +38,11 @@ class ConfigEntityUnitTest extends DrupalUnitTestBase {
 
     $expected = $info['config_prefix'] . '.';
     $this->assertIdentical($controller->getConfigPrefix(), $expected);
+
+    // Test the static extractID() method.
+    $expected_id = 'test_id';
+    $config_name = $info['config_prefix'] . '.' . $expected_id;
+    $this->assertIdentical($controller::getIDFromConfigName($config_name, $info['config_prefix']), $expected_id);
   }
 
 }

core/modules/config/lib/Drupal/config/Tests/ConfigImportTest.php

@@ -43,11 +43,11 @@ class ConfigImportTest extends DrupalUnitTestBase {
    * Tests omission of module APIs for bare configuration operations.
    */
   function testNoImport() {
-    $dynamic_name = 'config_test.dynamic.default';
+    $dynamic_name = 'config_test.dynamic.dotted.default';
 
     // Verify the default configuration values exist.
     $config = config($dynamic_name);
-    $this->assertIdentical($config->get('id'), 'default');
+    $this->assertIdentical($config->get('id'), 'dotted.default');
 
     // Verify that a bare config() does not involve module APIs.
     $this->assertFalse(isset($GLOBALS['hook_config_test']));
@@ -57,13 +57,13 @@ class ConfigImportTest extends DrupalUnitTestBase {
    * Tests deletion of configuration during import.
    */
   function testDeleted() {
-    $dynamic_name = 'config_test.dynamic.default';
+    $dynamic_name = 'config_test.dynamic.dotted.default';
     $storage = $this->container->get('config.storage');
     $staging = $this->container->get('config.storage.staging');
 
     // Verify the default configuration values exist.
     $config = config($dynamic_name);
-    $this->assertIdentical($config->get('id'), 'default');
+    $this->assertIdentical($config->get('id'), 'dotted.default');
 
     // Create an empty manifest to delete the configuration object.
     $staging->write('manifest.config_test.dynamic', array());
@@ -144,7 +144,7 @@ class ConfigImportTest extends DrupalUnitTestBase {
    */
   function testUpdated() {
     $name = 'config_test.system';
-    $dynamic_name = 'config_test.dynamic.default';
+    $dynamic_name = 'config_test.dynamic.dotted.default';
     $storage = $this->container->get('config.storage');
     $staging = $this->container->get('config.storage.staging');
 

core/modules/config/lib/Drupal/config/Tests/ConfigInstallTest.php

@@ -34,11 +34,11 @@ class ConfigInstallTest extends DrupalUnitTestBase {
    */
   function testModuleInstallation() {
     $default_config = 'config_test.system';
-    $default_configuration_entity = 'config_test.dynamic.default';
+    $default_configuration_entity = 'config_test.dynamic.dotted.default';
     $default_config_manifest = 'manifest.config_test.dynamic';
     $expected_manifest_data = array(
-      'default' => array(
-        'name' => 'config_test.dynamic.default',
+      'dotted.default' => array(
+        'name' => 'config_test.dynamic.dotted.default',
       ),
     );
     $default_empty_config_manifest = 'manifest.config_test.empty_manifest';

core/modules/config/tests/config_test/config/config_test.dynamic.dotted.default.yml

@@ -1,4 +1,4 @@
-id: default
+id: dotted.default
 label: Default
 protected_property: Default
 # Intentionally commented out to verify default status behavior.

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

@@ -19,7 +19,7 @@ class ImageStyleStorageController extends ConfigStorageController {
    * Overrides \Drupal\Core\Config\Entity\ConfigStorageController::importDelete().
    */
   public function importDelete($name, Config $new_config, Config $old_config) {
-    list(, , $id) = explode('.', $name);
+    $id = static::getIDFromConfigName($name, $this->entityInfo['config_prefix']);
     $entities = $this->load(array($id));
     $entity = $entities[$id];
 

core/modules/shortcut/shortcut.module

@@ -270,56 +270,6 @@ function shortcut_link_access($menu_link) {
   return FALSE;
 }
 
-/**
- * Implements hook_config_import_create().
- */
-function shortcut_config_import_create($name, $new_config, $old_config) {
-  if (strpos($name, 'shortcut.set.') !== 0) {
-    return FALSE;
-  }
-
-  $entity = entity_create('shortcut', $new_config->get());
-  $entity->save();
-  return TRUE;
-}
-
-/**
- * Implements hook_config_import_change().
- */
-function shortcut_config_import_change($name, $new_config, $old_config) {
-  if (strpos($name, 'shortcut.set.') !== 0) {
-    return FALSE;
-  }
-
-  list(, , $id) = explode('.', $name);
-  $entity = entity_load('shortcut', $id);
-
-  $entity->original = clone $entity;
-  foreach ($old_config->get() as $property => $value) {
-    $entity->original->$property = $value;
-  }
-
-  foreach ($new_config->get() as $property => $value) {
-    $entity->$property = $value;
-  }
-
-  $entity->save();
-  return TRUE;
-}
-
-/**
- * Implements hook_config_import_delete().
- */
-function shortcut_config_import_delete($name, $new_config, $old_config) {
-  if (strpos($name, 'shortcut.set.') !== 0) {
-    return FALSE;
-  }
-
-  list(, , $id) = explode('.', $name);
-  entity_delete_multiple('shortcut', array($id));
-  return TRUE;
-}
-
 /**
  * Loads the data for a shortcut set.
  *

core/modules/views/lib/Drupal/views/Tests/ViewTestData.php

@@ -52,8 +52,8 @@ class ViewTestData {
         }
 
         $source_storage = new FileStorage($config_dir);
-        foreach ($source_storage->listAll() as $config_name) {
-          list(, , $id) = explode('.', $config_name);
+        foreach ($source_storage->listAll('views.view.') as $config_name) {
+          $id = str_replace('views.view.', '', $config_name);
           if (in_array($id, $views)) {
             $config_changes['create'][] = $config_name;
           }