computerminds / cm_config_tools

DEPRECATED: Mirror of http://cgit.drupalcode.org/cm_config_tools
1 stars 0 forks source link

Unmanaged config can override existing config on installation #2

Open anotherjames opened 8 years ago

anotherjames commented 8 years ago

(Caveat: this applies to the code currently in PR #1, but will find its way into 8.x-1.x before long hopefully.)

Unexpected behaviour and forced diversion from desired workflow: Any 'unmanaged' config will get installed/overridden on module installation; so it's important to ensure it matches any existing values (e.g. in case users have changed its values on remote environments), which we are trying to avoid being necessary elsewhere in the workflow.


The idea of 'unmanaged' config was purely to ensure that a piece of config exists, to satisfy dependencies or to ensure some initial values, without having to care about the values in it and allow them to be overridden. The current state of code (in the branches in the above caveat) is correct for this on importing via the ExtensionConfigHandler (e.g. on running drush cmci), but not when installing a module (i.e. when the [decorated] config.installer service handles config installation).

Problem: To deal with this, the installDefaultConfig, installOptionalConfig and installCollectionDefaultConfig methods on core's original ConfigInstaller that we currently decorate would need overriding (and then would no longer be decorated either). ... those 3 methods are pretty big and quite tricky to work with. They are fundamentally not designed to map config to their providing extensions (oh gosh, don't tell me we're going down the config_provider/config_sync route after all, without realising it..?!).

Install profiles are fundamentally allowed to override existing config, so I think it's okay for a profile to declare something as unmanaged but to override it on installation (especially as installation of an install profile will only happen at the start of a project, before users have had chance to change any values of the config).

However, the ideal for modules is that they cannot override existing config on installation, just as is the case on running drush cmci. But currently we allow them to do so (in order to allow managed config to override existing config), as there is currently no distinction on module install between managed and unmanaged config (as long as the config is actually present in the /config/install sub-directory of the module).


Workaround: Declare config as unmanaged, and remove it from /config/install so that it does not get installed (it can always get put back in there by exports after the module is installed on live environments). If necessary, the config could always be checked for existence and created as necessary in a specific update hook. I think this is ok really.


An aside to the problem: The getConfigToCreate method has quite a narrow usage, and takes a storage class as parameter (which does not know what extension it came from, unless we were to interrogate its source directory, which isn't really valid/reliable anyway, so we can't then check what config is listed as unmanaged) ...so we can't even just override that.

That would then leave no non-trivial methods being decorated, and worse still, could end up with different 'decorated' properties in our decorating class to the original decorated class which isn't good. So basically we'd need to drop the class decoration for just extending the class and replacing the service definition, which is sad but necessary.