Problem/Motivation

Config dependencies are checked for existence. They are not checked for correctness.

What core checks today

#3324150: Add validation constraints to config_entity.dependencies added constraints to
config_dependencies_base in core/config/schema/core.data_types.schema.yml:

  • NotBlank and ValidKeys
  • ConfigExists on each config dependency
  • ExtensionName and ExtensionExists on each module and
    theme dependency

Together these answer one question: does each listed dependency exist?

What core does not check

Whether the listed dependencies are the ones core itself would calculate. Nothing compares stored
dependencies to ConfigEntityBase::calculateDependencies(). Grep for
calculateDependencies in Core/Config/Plugin/Validation/Constraint and
Core/Validation/Plugin/Validation/Constraint. There are no hits.

Why that matters

ConfigEntityBase::preSave() recalculates dependencies on every save. It skips this
during a sync:

if (!$this->isSyncing()) {
  // Ensure the correct dependencies are present. If the configuration is
  // being written during a configuration synchronization then there is no
  // need to recalculate the dependencies.
  $this->calculateDependencies();

So a config import stores whatever the source says. Then, on module uninstall,
ConfigManager::uninstall() deletes every config entity it cannot repair:

foreach ($entities['delete'] as $entity) {
  $entity->setUninstalling(TRUE);
  $entity->delete();
}

ConfigEntityBase::onDependencyRemoval() cannot repair a plain module dependency. It
returns FALSE, so the config entity is deleted. One wrong line therefore destroys
content:

  • Add dependencies.module: [foo] to node.type.article.
  • Uninstall foo.
  • The content type is deleted. Its nodes go with it.

Where this can happen

Anywhere config reaches a site from outside it:

  • a Git-backed config workflow
  • a recipe
  • a hand-edited sync directory

Steps to reproduce

  1. Add dependencies: {module: [views]} to node.type.article in the sync
    directory.
  2. Run drush config:import. It is accepted: sync skips recalculation, and
    views exists.
  3. Uninstall views. The content type and its nodes are deleted.

Proposed resolution

Add a CalculatedConfigDependencies constraint. It compares stored dependencies to
calculated ones and reports a violation per difference, in both directions.

The name follows the existing RequiredConfigDependencies in that namespace, which
checks that dependencies contain given entity types. This one checks that they are the dependencies
calculateDependencies() produces. (ValidConfigDependencies would be a poor name, given the pre-existing validation that #3324150: Add validation constraints to config_entity.dependencies added.

Exclude enforced from the comparison

An enforced dependency is not calculated, so it must be left out of both sides. There is a trap
here. ConfigEntityBase::getDependencies() merges enforced into what it
returns:

$dependencies = $this->dependencies;
if (isset($dependencies['enforced'])) {
  // Merge the enforced dependencies into the list of dependencies.
  $enforced_dependencies = $dependencies['enforced'];
  unset($dependencies['enforced']);
  $dependencies = NestedArray::mergeDeep($dependencies, $enforced_dependencies);
}

So removing enforced after calling that method is too late. An enforced entry then
reads back as a calculated one. Remove it from the stored values before calculating instead.

What makes this hard

We built this in a distribution. One problem needs solving first:
calculateDependencies() cannot always run before a change set is applied. Some
implementations read other config, which the same set may still be about to create.

The main case is a bundle. EntityTypeInterface::getBundleConfigDependency() loads
the bundle entity and throws a LogicException when it is missing, and four
calculateDependencies() implementations call it:

Caller Config entity types affected
FieldConfigBase::calculateDependencies() field_config, base_field_override
EntityDisplayBase::calculateDependencies() entity_form_display, entity_view_display
ContentModeration::calculateDependencies() workflow
ContentLanguageSettings::calculateDependencies() language_content_settings

So any config entity whose dependencies include a bundle cannot be calculated while that bundle
is still in the change set. Types that read no other config —
user_role, node_type, field_storage_config — can.

Two implementations fail in a second, worse way, by returning too few dependencies instead of
throwing:

  • FieldConfig::calculateDependencies() also calls
    getFieldStorageDefinition(), which throws when the field storage is in the same
    set.
  • EntityDisplayBase::calculateDependencies() intersects components with
    getFieldDefinitions() and silently returns too few.
    EntityFormDisplay::getPluginCollections() also skips components whose field it
    cannot resolve, so widget module dependencies are lost too.
  • Field item implementations can do the same: EntityReferenceItem resolves
    target_bundles with loadMultiple() and skips the ones absent.

Returning too few is the dangerous case. A real dependency then looks unexpected, so a naive
constraint rejects valid imports. The common one is adding a content type, its fields and its form
display together. Throwing is safer: a caller can treat it as "cannot say" and check nothing.

Out of scope: whether an enforced dependency is correct

dependencies.enforced exists so that a module can declare a dependency core cannot
calculate. Core therefore cannot tell a legitimate enforced dependency from a wrong one: it cannot
know whether some shipped configuration once declared it. Only the owner of a config workflow can
decide that. A distribution that syncs customer-authored configuration knows an enforced dependency
can never be legitimate there, and can refuse it. Core has no such rule available.

This is worth noting because an enforced dependency is the one kind nothing ever corrects.
calculateDependencies() keeps the key on every save, and
ConfigEntityDependency::__construct() merges it into the dependency graph, so Drupal acts
on it.

Remaining tasks

  • Decide where the comparison lives: an entity-level constraint on ConfigEntityType,
    or a constraint on the config_dependencies schema type.
  • Decide what to do for the types that cannot be calculated before apply.
  • Decide whether this is opt-in. Existing sites may hold drift that would now fail.
  • Tests.

User interface changes

None.

API changes

A new validation constraint is added.

Data model changes

None.

Comments

wim leers created an issue. See original summary.

wim leers’s picture

wim leers’s picture

Issue summary: View changes