currently we have no way to load config data via config() as it exists in .yml files without $conf and locale overrides messing with the returned data.

see this comment: https://drupal.org/node/1648930?page=1#comment-6847674

lets get a simple patch in to solve this problem.

Comments

Gábor Hojtsy’s picture

Title: allow loading of 'raw' config data » Raw (original) config data is not accessible
Category: task » bug
Priority: Normal » Major
Issue tags: +D8MI, +sprint

The issue at #1763640: Introduce config context to make original config and different overrides accessible proposed a system_config() function that would set a context to not use any overrides, and would be used in place of config(). I suspect this could be highly intertwined with the work in #1763640: Introduce config context to make original config and different overrides accessible, which now does not want to solve this problem, so not sure if the two can be solved in parallel.

Gábor Hojtsy’s picture

Issue tags: +language-config
Berdir’s picture

While debugging a config override problem in the config caching issue, I noticed that we are currently also looking for overrides for config overrides, when editing them directly.

So we are essentially looking for e.g. a config file named locale.config.en.locale.config.en.system.date when editing the en override for system.date.

Not sure if this is the same problem/needs the same solution or if we should simply add a check for this directly in LocaleConfigSubscriber to exclude those. Because it might be possible that we need a domain override for a local override or some other crazyness like that ;)

chx’s picture

The reason this is major -- I have thought of this before but never actually tested it -- if you have a override then you actually save the overridden data back! Which is arguably a data loss / huge mess. For eg dev site overrides the paypal settings to use the PayPal sandbox if you happen to deploy that then your payment go down the black hole... if this is what's happening then this is critical IMO.

sun’s picture

re: #4: @chx:
Unless you prove me wrong, I think this is actually covered by tests to not be the case. (and of course, unless someone false-"fixed" the existing tests...)

ConfigOverrideTest is supposed to assert that a config file can be loaded + changed + saved, even if there are $conf overrides that apply to the config object, but the effective config object only sees the actual/original [+ changed] values.

However, there's no need to educate about the special case of overridden values scenario — the entire purpose of @Jose's config context issue is to introduce the necessary architectural plumbing to make modules aware of potential config contexts, so it can actually be controlled what gets read and/or written with in a particular scope.

beejeebus’s picture

yep, #5 is right - the problem we have is at load time, not save time, as per my description in #0.

chx’s picture

Um, the problem I describe is not simply $config->load(); $config->save(); no, there is a form in between.

Ie. $form['foo']['#default_value'] = $config->get('foo'); in submit handler $config->set('foo', $form_values['foo']); and now $config->save(). You are, at this point, screwed, unless I am badly mistaken.

chx’s picture

On save... we could look at the current data, compare it to overrides and if there are equalities then just revert to the original data because in all likeliness #7 is happening and there is no harm to the user cos the override will happen next anyways just the same.

Damien Tournoud’s picture

Priority: Major » Critical

@chx is right. Any override scheme will bump into this very common issue.

In the current state of things, any form editing a config object is affected, so I'm bumping this issue to critical.

Being able to access the original data is a first step, but it's clearly insufficient. Using the original data in a form would cause weird usability issues: for example, if the config object is overridden by settings.php, you would still edit the old version that doesn't actually apply anywhere...

To be able to properly edit the configuration in the UI, we need to rethink the override system. At the minimum, three properties need to hold true:

  1. You are able to introspect the contexts in which a configuration object can be overridden; this is related in part with #1648930: Introduce configuration schema and use for translation, because you might want to have sub-keys of a configuration object be overridden in different ways (some keys are translatable, others can vary per domain, etc.);
  2. You can load a configuration object for a specific context (ie. load 'system.site' for French, without any other alteration);
  3. When you save this overridden configuration object, it saves the appropriate override, not the original object (which means that overrides need to be writable, they are not right now).
Gábor Hojtsy’s picture

@Damien Tournoud:

In the current state of things, any form editing a config object is affected, so I'm bumping this issue to critical.

Note that Jose Reyero's patches in #1763640: Introduce config context to make original config and different overrides accessible had a complete solution to the forms and overrides it affected before it was deemed too complex and turned down. Looks like we are getting to a system of similar complexity in smaller steps (and definitely more dramatic issue categorisation).

Being able to access the original data is a first step, but it's clearly insufficient. Using the original data in a form would cause weird usability issues: for example, if the config object is overridden by settings.php, you would still edit the old version that doesn't actually apply anywhere...

I think that is by design. We cannot tell if the override data from settings.php was dynamic or static, and we should definitely not save it anywhere if it was dynamic. It would only make sense to write in the original config storage, if it was static, and in that case, people could just as well change the config value and possibly alter out the settings form for the values they don't want users to change. Also there is any possibility of overrides, depending on various values, the override system is open-ended. (Even if overrides would only be taken from settings.php $conf, they could depend on anything and override each other as well based on different criteria).

I'm not sure I understand your 3 points.

(1) introspecting contexts when something is overridden can be open-ended; eg. "use black theme background when it is afternoon" uses a time context and has logic around it not discrete values (or at least all the discrete second values are pretty numerous to count and the override is not expressed as a direct mapping from the seconds in time);

(2/3) on loading from one specific context and saving to one specific context, the config system does not know or care about how these context values are stored; they might even be derived from other values and not actually stored (eg. imagine an organic groups setting that overrides something for all group entities and then still editable per node as well); therefore I don't really know how and why would overrides need to be stored directly via the base config system; the locale context for example uses a file naming pattern that includes the language code and the patch at #1648930: Introduce configuration schema and use for translation includes code to write to those files *in the locale system* because the locale system has control over the storage structure, format, placement, etc. of the overrides; it is not the responsibility of the config system, it is a different layer. On loading per specific contexts, well, general context support in #1763640: Introduce config context to make original config and different overrides accessible did not get support from CMI folks, so looks like Drupal will only ever have code where it tells the language context or it does not tell any context at all. Obviously this issue will need to introduce code where it tells "please no context" as well, so those would be the three options in core unless the more versatile context proposal in #1763640: Introduce config context to make original config and different overrides accessible gets any support.

sun’s picture

I don't understand why this issue is not an inherent part of #1763640: Introduce config context to make original config and different overrides accessible? There's no way around an incarnation of context, in order to handle config override values properly and in a controlled fashion. Comparing values against original and "assuming" cannot be the design we're aiming for, right? I've the impression we're walking in circles?

I also don't understand why this was bumped to critical. AFAICS, there's no regression to D7, which exposes the same behavior for variables that are overridden via $conf and which are loaded and saved through a form.

Gábor Hojtsy’s picture

Well, it is a separate issue in how @beejeebus set out new directions in #1763640: Introduce config context to make original config and different overrides accessible given no support for Jose's more general context system. @beejeebus' current proposal only deals with language contexts specifically and very closely baking the language override key into CMI to handle, so it is not anymore dealing with the more general concept of overrides.

I agree we are going in circles, but that did not start with this issue. Most of the CMI-D8MI issues evolve around "oh, we don't want to make it this complex", "well, yeah, otherwise language features cannot be implemented", "oh, we do not want to make it this complex anyway" intertwined with "maybe if we sit around long enough, it will solve itself". Reality is we are just (re)introducing bugs and will need to deal with these things for the release. The longer we ignore them, the less experience we'll have with the solution and the less opportunity we'll have to adjust it so it makes more sense.

As for why this is critical, I think it is a combination of CMI used for lots more things, so huge masses of things can potentially break on your site and it going against the design of the override system (due to the missing context system).

beejeebus’s picture

Status: Active » Closed (won't fix)

#1763640-87: Introduce config context to make original config and different overrides accessible - at this point i'm willing to concede on all points except the scope. lets go back to a more complex, more powerful system based on jose's work in that issue.

Gábor Hojtsy’s picture

Status: Closed (won't fix) » Closed (duplicate)

I think duplicate is correct, since we have all intentions to fix the "Raw (original) config data is not accessible" problem.

Gábor Hojtsy’s picture

Issue tags: -sprint

Remove from D8MI sprint also.