Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spin-off from: #1668820: Concept, base class, and interface for Configurables (e.g., node types, image styles, vocabularies, views, etc)
Problem
- Each hook_config_import_*() callback requires a decent amount of code to extract the ID (e.g.,
thumbnail) from a config object's name (e.g.,image.style.thumbnail). - Functions working with
StorageInterface::listAll()equally need to iterate over the whole result set once more to remove the prefix from the config object name.
Goal
- Simplify retrieving the ID from a config object name.
Comments
Comment #1
yched CreditAttribution: yched commentedAw, yes please :-). The current strpos()-based checks feel, you know... clunky.
Comment #2
matason CreditAttribution: matason commentedHey @sun, I've been looking into this and could do with some guidance :)
One way of tackling the first part of this issue might be to have config_import_invoke_owner() invoke more specific hooks so that 'owner' modules could implement more specific hooks thus eliminating the need for them to interrogate what's been passed - this won't work (and you probably wouldn't want it to work) for all parts of the settings file name (hook_huge_long_function_name_here()) but we could have image for example implement image_config_import_style_create() and that be the place for handling all image.style.* configuration.
As regards the second part of the issue regarding listAll(), can you help me understand the use cases?
Comment #3
tim.plunkettComment #4
andypostAlso currently there's ability to have image.style.thumb but name (ID) inside is different from file name.
Comment #5
sun#4 is irrelevant for this issue.
I'd like to see one or two more conversions to Configurables first.
#1588422: Convert contact categories to configuration system landed already.
#1782244: Convert image styles $style array into ImageStyle (extends ConfigEntity) seems to be close.
#1552396: Convert vocabularies into configuration might be close, too, wasn't able to look at that for a while.
But as far as I can see, all use-cases that originally triggered this issue are now baked into different entity handler classes. I'm playing with the idea of getting rid of hook_config_import_*() entirely and moving the operations into Config\Entity instead, potentially ConfigStorageController, potentially providing a base implementation that works for 80% of all cases.
Config\Entity\* adds a dependency on entity info, but that is backed by code, not config, potentially plugins later. The major difference is that it has built-in knowledge about the 'config.prefix'. The direct consequence is that the initial check for whether a config object is a dynamic Configurable in all of these callbacks becomes unnecessary.
To clarify upfront, the callbacks would probably stay to account for unforeseen edge-cases, but there's probably no reason for why we cannot or should not add inherent knowledge about Config\Entity to the config import mechanism; at least I can't see any - knowing about that is no worse than knowing about the module/hook system and hook_X() callbacks.
Related:
#1763974: Convert entity type info into plugins
Comment #6
tim.plunkettI agree with everything said in #5.
Comment #7
catchBumping to major, the string-guessing proliferation is a mess at the moment.
Comment #8
xjmThis drives me no end of crazy every time I see it; I'll start taking a look at this.
Comment #9
xjmComment #10
tim.plunkettThis might be blocked on the resolution of #1831774: Config import assumes that 'config_prefix' contains one dot only, since we haven't actually enforced any standards for a config object name.
Comment #11
tim.plunkett#1831774: Config import assumes that 'config_prefix' contains one dot only ended up providing the method to do this, see ConfigStorageController::getIDFromConfigName().
Comment #12
xjmNice, finally.