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.

Comments

webflo’s picture

FileSize
701 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,843 pass(es). View

... and i wrote a drush command to normalize all existing config entities after Drupal Core Updates.

The last submitted patch, normalize-config-entity-yml.patch, failed testing.

xtfer’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
webflo’s picture

Status: Needs work » Needs review
FileSize
637 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,841 pass(es). View
Sweetchuck’s picture

Issue tags: -Needs reroll

The patch #4 is applicable to the latest 8.x

webflo’s picture

Issue tags: +Configuration system
alexpott’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

I 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.

mtift’s picture

mtift’s picture

Status: Needs work » Needs review
FileSize
656 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,686 pass(es), 1 fail(s), and 0 exception(s). View

Seems like a good idea to me. Here is a reroll.

Status: Needs review » Needs work

The last submitted patch, 9: normalize-config-entity-yml-2227731-8.patch, failed testing.

mtift’s picture

Status: Needs work » Needs review
FileSize
2.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,719 pass(es). View
639 bytes

Updated tests. Passes locally.

webflo’s picture

FileSize
1.54 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,867 pass(es), 1 fail(s), and 0 exception(s). View
2.24 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,861 pass(es), 1 fail(s), and 0 exception(s). View

Added a test to test the validation between config entity and config entity storage.

Status: Needs review » Needs work

The last submitted patch, 12: normalize-config-entity-yml-2227731-12.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: normalize-config-entity-yml-2227731-12.patch, failed testing.

The last submitted patch, 12: normalize-config-entity-yml-2227731-12-test-only.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: normalize-config-entity-yml-2227731-12.patch, failed testing.

webflo’s picture

Status: Needs work » Needs review
FileSize
1.55 KB
3.79 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,603 pass(es). View

Applied the changed to the unit test from #11

webflo’s picture

Issue tags: -Needs tests +Amsterdam2014, +sprint
alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Nice 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.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: normalize-config-entity-yml-2227731-19.patch, failed testing.

alexpott’s picture

This is an unrelated random test fail. Retesting...

[10-Oct-2014 14:24:59 UTC] Uncaught PHP Exception InvalidArgumentException: "The URI "base://AKI@&hO@" is invalid. You must use a valid URI scheme. Use base:// for a path, e.g., to a Drupal file that needs the base path. Do not use this for internal paths controlled by Drupal." at /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Url.php line 207

Status: Needs work » Needs review
alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: normalize-config-entity-yml-2227731-19.patch, failed testing.

Status: Needs work » Needs review
webflo’s picture

Status: Needs review » Reviewed & tested by the community

Patch #19 was already RTBCed in #21 and #25

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed/pushed to 8.0.x, thanks!

  • catch committed c999c2d on 8.0.x
    Issue #2227731 by webflo, mtift: Fixed Normalize configuration data...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.