Problem

  • Many (core) developers did not notice that the config_prefix of config entities does NOT contain a trailing dot and forgot it when listing/querying for all available config object names within a config-prefix namespace.
  • Omitting the trailing dot can very/too easily lead to false-positives. Essentially a very similar problem space we have with module hook implementation names already; e.g., foo_bar_baz() can be foo's hook implementation of hook_bar_baz(), but also foo_bar's hook implementation of hook_baz().

Proposed solution

  1. No change to existing entity type meta-data/annotations.

  2. Make the entity plugin manager perform this:

    // Ensure that config_prefix ends with a trailing dot.
if (isset($definition['config_prefix'])) {
  $definition['config_prefix'] = rtrim($definition['config_prefix'], '.') . '.';
}

  3. Change all callers/actors that operate on config_prefix directly to remove the additional/custom futzing with a trailing dot.

CommentFileSizeAuthor
#1 drupal8.config-prefix-dot.do-not-test.patch5.66 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Needs work
StatusFileSize
new drupal8.config-prefix-dot.do-not-test.patch5.66 KB

Hrm. Grepping for config_prefix...

1) reveals how utterly apparent the awareness problem of a trailing dot is.

2) reveals that all usages that want to load the actual manifest file would have to strip the trailing dot again.

Suggestions?

I did not touch all instances. Left a range of comments instead.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +Configuration system, +Configurables
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/includes/update.incundefined
@@ -1426,13 +1426,13 @@ function update_variables_to_config($config_name, array $variable_map) {
-    $data[$id] = array('name' => $config_prefix . '.' . $id);
+    $data[$id] = array('name' => $config_prefix . $id); // That bug again.

Which bug?

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.phpundefined
@@ -158,7 +158,7 @@ public function loadByProperties(array $values = array()) {
   public function getConfigPrefix() {
-    return $this->entityInfo['config_prefix'] . '.';
+    return $this->entityInfo['config_prefix'];

This whole method seems to become irrelevant.

+++ b/core/lib/Drupal/Core/Entity/EntityManager.phpundefined
@@ -193,6 +193,12 @@ public function processDefinition(&$definition, $plugin_id) {
+    // @todo Drupal\Core\Config\Entity\ConfigEntityManager?

O_o

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Closed (duplicate)

#1849792: Abstract usage of 'config_prefix' on ConfigStorageController into getConfigPrefix method has solved this