Normalize configuration data during config writes
It is important to have consistent YAML files for a sane development workflow. We should reduce unnecessary to a minimum. The current structure of our YAML depends on ConfigEntityBase::toArray(). This means the structure can change after code updates because the order of public properties is different then before.
Currently the config structure is not updated for existing config entities. Because $config->data is initialized after loading with existing data from ConfigCache.
The foreach loop updates the values in $config->data but does not reorder the keys. This means the new configuration is successfully imported but its still marked as modified in our CMI UI because the arrays are not identical.
This produces a lot of modified files in your version control after export because every developer generates different YAML files depending on the initial structure where the config entity was created/imported.
Lets replace the foreach loop with one method call and override the complete array.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | normalize-config-entity-yml-2227731-19.patch | 3.79 KB | webflo |
| #19 | normalize-config-entity-yml-2227731-12-19.interdiff.txt | 1.55 KB | webflo |
Comments
Comment #1
webflo commented... and i wrote a drush command to normalize all existing config entities after Drupal Core Updates.
Comment #3
xtfer commentedI think you've hit some patch collision. See #2219925: Rename ConfigEntityInterface::getExportProperties() to toArray().
Comment #4
webflo commentedComment #5
sweetchuckThe patch #4 is applicable to the latest 8.x
Comment #6
webflo commentedComment #7
alexpottI thought that it was this way at one point but... looking at #1824484: Allow config storage controllers to define their own properties I don't think it was.
This makes sense to me. It'd be nice to add a test somehow.
Comment #8
mtiftUpdated method name due to #2219925: Rename ConfigEntityInterface::getExportProperties() to toArray().
Comment #9
mtiftSeems like a good idea to me. Here is a reroll.
Comment #11
mtiftUpdated tests. Passes locally.
Comment #12
webflo commentedAdded a test to test the validation between config entity and config entity storage.
Comment #19
webflo commentedApplied the changed to the unit test from #11
Comment #20
webflo commentedComment #21
alexpottNice improvement. Thanks for picking this up again.
There are no API changes in this patch - it would only break BC if a configuration entity has an incomplete schema and is somehow relying on non schema values. The change makes it harder to end up with unexpected keys in configuration entities during module development.
Comment #23
alexpottThis is an unrelated random test fail. Retesting...
Comment #25
alexpottComment #28
webflo commentedPatch #19 was already RTBCed in #21 and #25
Comment #29
catchLooks good. Committed/pushed to 8.0.x, thanks!