In #2204697-41: Move getConfigPrefix() to ConfigEntityTypeInterface outline several ways the the config_prefix
and ConfigEntityTypeInterface::getConfigPrefix()
documentation could be improved:
-
+++ b/core/lib/Drupal/Core/Config/ConfigManager.php @@ -102,7 +103,7 @@ public function __construct(EntityManagerInterface $entity_manager, ConfigFactor public function getEntityTypeIdByName($name) { $entities = array_filter($this->entityManager->getDefinitions(), function (EntityTypeInterface $entity_type) use ($name) { - return ($config_prefix = $entity_type->getConfigPrefix()) && strpos($name, $config_prefix . '.') === 0; + return ($entity_type instanceof ConfigEntityTypeInterface && $config_prefix = $entity_type->getConfigPrefix()) && strpos($name, $config_prefix . '.') === 0; }); return key($entities); }
Hmm. So if the entity type is an instance of ConfigEntityTypeInterface, but getConfigPrefix() returns something that is false, or the name doesn't begin with the config prefix expected... then what happens?
This isn't introduced by this patch, but it seems like now this is a set of things that would mean the entity type were malformed if the first were true but not one of the others. I think. Can we get a followup to at least add an inline comment to document this, and maybe investigate whether this line is something that should be refactored?
-
+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityType.php @@ -15,44 +15,17 @@ + * The default configuration prefix is constructed from the name of the module + * that provides the entity type and the ID of the entity type. If a + * config_prefix annotation is present it will be used in place of the entity + * type ID. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityTypeInterface.php @@ -0,0 +1,64 @@ + * - The config prefix, which is returned by getConfigPrefix() and is + * composed of: + * - The provider module name (limited to 50 characters by + * DRUPAL_EXTENSION_NAME_MAX_LENGTH). + * - The module-specific namespace identifier, which defaults to the + * configuration entity type ID. Entity type IDs are limited to 32 + * characters by EntityTypeInterface::ID_MAX_LENGTH. ... + * Ensures that all configuration entities are prefixed by the module that + * provides the configuration entity type. This ensures that if a + * configuration entity is contained in a extension's default configuration it + * be created during extension installation. Additionally, it allows + * dependencies to be calculated without the modules that provide + * configuration entity types being installed.
So the first and third hunks of documentation are added by this patch, and the first is what we have in HEAD. That means we're explaining what a config prefix is composed of in why in different ways in three different places. We should consolidate that and clarify it.
Comment | File | Size | Author |
---|---|---|---|
#7 | 2482215.2.patch | 3.23 KB | alexpott |
#2 | 2482215.2.patch | 3.63 KB | alexpott |
Comments
Comment #1
alexpottSo re point 1. It is impossible for
getConfigEntityPrefix()
to return false with the current code. I guess what we need to do here is remove the FALSE possibility from the@return
documentation. Also the if the name is not a configuration entity thenConfigManager::getEntityTypeIdByName()
returns NULL so it is behaving exactly as documented.Comment #2
alexpottTried to consolidate documentation onto the interface and remove sentences which are no longer true. For example, config entity prefixes now are just part of what causes optional configuration to be installed - but the bigger picture there is that this is all about dependencies - if an optional configurations dependencies are met then it will be installed.
Comment #5
willzyx CreditAttribution: willzyx commentedI think this is the problem
Comment #7
alexpottLol - that's my environment getting in the way.
Comment #8
jhodgdonThis looks good to me. Since alexpott created the patch, I went ahead and committed it (he can't).