Problem/Motivation
DomainConfigFactoryOverride::getOverrideEditable() seeds the editable override with two different things: initWithData() receives the base configuration merged with the domain's existing override, and setModuleOverride($domain_data) receives the existing override row on its own. So $moduleOverrides on that object means "the keys this domain overrides", and DomainConfigOverrideEditable::setOverride(), called from set(), keeps that view up to date as the caller changes keys. Core reads the same property from hasOverrides() and from getOriginal() when overrides are applied.
None of that is tested, and the comments describing it no longer match the code.
No coverage
Across the whole project there is no test calling setOverride() and none calling hasOverrides(). The only call to getOriginal() is in a Domain Config Language UI functional test and passes $apply_overrides = FALSE, so it never exercises the merge with $moduleOverrides at all. The contract of the property exists only in prose.
A consequence worth recording: since #3587744: domain_config: Make DomainConfigOverrideEditable::save() write a sparse, cast-aware diff against base made save() recompute $this->moduleOverrides as a diff, calling setOverride() no longer causes anything to be stored. That regression reached 3.1.0-alpha1 and is still present, and no test failed, because nothing covers the method.
Stale comments
In DomainConfigFactoryOverride::getOverrideEditable(), the comment justifying the pre-merge says that save() casts the data through the schema and copies the cast values back onto $moduleOverrides for every key already present there. That path was removed by #3587744: domain_config: Make DomainConfigOverrideEditable::save() write a sparse, cast-aware diff against base. The pre-merge is still correct and still needed, but the reason given for it no longer exists.
In DomainConfigOverrideEditable::save(), the comment above the diff recounts how moduleOverrides used to be built up per key and written verbatim, and which case the old conditional fallback missed. That is release history rather than a description of the current design.
The class docblock of DomainConfigOverrideEditableTest carries the same narrative.
Proposed resolution
- Rewrite the three comments to describe what the code does now. Keep the issue links, drop the history.
- Give
setOverride()a docblock that says what it actually does, which is maintain the in-memory view of the domain's override row rather than set a value in the configuration object. - Add kernel coverage for the view: that a freshly loaded editable override reports the stored override keys through
hasOverrides(), that a key set afterwards joins them, and that a key the domain does not override does not. - Add coverage for what
save()does and does not persist through that path, so the behavior is pinned either way rather than left to prose.
setOverride() in domain:3.1.0 and make the method protected in domain:4.0.0. It is bookkeeping for set() and has no other caller in this project, and the one thing an outside caller might reasonably have wanted from it, writing a key the diff would drop, is handled declaratively by the pinned keys list in #3618091: Allow a config key to stay overridden when its value equals the base configuration.The visibility change needs the full cycle rather than being a free change: DomainConfigOverrideEditable is not marked @internal, and DomainConfigFactoryOverrideInterface::getOverrideEditable() hands instances of it to callers, so the class is public API even though this particular method is an implementation detail. Landing the deprecation in 3.1.0 keeps it in step with updateExistingKeysInNestedArray() and the constructor's $base_data argument, both deprecated in that version by #3587744: domain_config: Make DomainConfigOverrideEditable::save() write a sparse, cast-aware diff against base.
Once the method is protected, its name is worth changing too. setOverride() reads as "set a value in this override", which is what set() does and is exactly the misreading that let the #3587744: domain_config: Make DomainConfigOverrideEditable::save() write a sparse, cast-aware diff against base regression go unnoticed. What it actually does is set one key in the module override layer, so setModuleOverrideValue($key, $value) would sit beside core's own Config::setModuleOverride(array $data) as the single-key variant and say which layer it touches. Renaming in 4.0.0 costs nothing beyond the visibility change already planned there, and nothing outside this class would be able to call it by then.
The coverage comes first regardless, so both changes are made against tests rather than against a reading of the code.
Remaining tasks
A change record for the deprecation.
User interface changes
None.
API changes
Public access to DomainConfigOverrideEditable::setOverride() is deprecated in domain:3.1.0, and the method becomes protected in domain:4.0.0. Callers should use set(), which is what maintains the override view through it. It is also renamed in domain:4.0.0, to setModuleOverrideValue() or similar, which is safe once nothing outside the class can reach it. Nothing else in this issue changes behavior; the rest is comments and test coverage.
Data model changes
None.
AI-Generated: Yes (Claude Code was used to help draft this issue summary. I reviewed it before posting; the finding came out of auditing a related merge request and there is no merge request on this issue yet.)
Issue fork domain-3618151
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mably commentedComment #3
mably commentedComment #6
mably commentedComment #9
mably commented