Problem/Motivation

Drupal 11.2 has been released. It introduced some interesting changes to speed up site and module installations that impact Config Provider:

The changes to core's ConfigInstallerInterface cause fatal errors on sites upgraded to 11.2.x with Config Provider installed:

PHP Fatal error: Declaration of Drupal\config_provider\ProxyClass\ConfigProviderConfigInstaller::installDefaultConfig($type, $name) must be compatible with Drupal\Core\Config\ConfigInstallerInterface::installDefaultConfig($type, $name, Drupal\Core\Config\DefaultConfigMode $mode = Drupal\Core\Config\DefaultConfigMode::All) in /modules/contrib/config_provider/src/ProxyClass/ConfigProviderConfigInstaller.php on line 73

Steps to reproduce

Try upgrading a site with Config Provider installed to Drupal 11.2.x.

Proposed resolution

Looks like we need to add a compatibility layer similar to what was added to the Config Selector module in #3492937: Add compatibility layer for Drupal 11.2 to add Drupal >= 11.2 compatibility without breaking Drupal < 11.2 compatibility.

Remaining tasks

Figure out whether to close this as a duplicate of #2800839: Address conflict with Features over config.installer class or address separately.

User interface changes

API changes

Data model changes

Command icon 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

joegraduate created an issue. See original summary.

joegraduate’s picture

Issue summary: View changes

joegraduate’s picture

Status: Active » Needs review

Updated the module to decorate the core config.installer service rather than alter it in MR !20. This made it easier to employ the dynamic compatibility traits strategy employed by the config_selector module and made it possible to remove the proxy class. The proxy class was needed before because this module was extending the core ConfigInstaller class (which core declares as a lazy service in core.services.yml). It is no longer needed with the MR.

joegraduate’s picture

Tested MR !20 with an existing project that contains a custom ConfigProvider plugin and found that the decorator approach used in that MR doesn't seem to work (the custom ConfigProvider plugin I was testing did not work as expected).

Created alternate MR !21 that adds a separate backwards-compatible ConfigProviderConfigInstaller class, updates the existing class to be 11.2 compatible and updates the existing service provider to use the appropriate ConfigProviderConfigInstaller class depending on the installed core version. The custom ConfigProvider in our existing project did still work as expected when tested with this new MR.

joegraduate changed the visibility of the branch 3532693-dynamic-alter to hidden.

joegraduate’s picture

Discussed the 2 different MR approaches extensively with fellow maintainer @tadean.

I've updated MR !20 with the approach that we both feel the most comfortable with which registers our ConfigProviderConfigInstaller class as a decorator for the core config.installer service via the services.yml file instead of altering (replacing) the core service via a ServiceProvider class. This approach vastly simplifies the MR because it:

  • removes the need for the existing ServiceProvider and Proxy classes,
  • removes the need for the compatibility traits previously added in MR !20
  • requires no changes to the existing ConfigProviderConfigInstaller service class

I've also hidden the other MR branch.

While discussing this issue and the potential fixes with @tadean, we realized that the changes proposed in @trackleft2's MRs to address #2800839: Address conflict with Features over config.installer class are very similar. We might want to consider closing this issue as a duplicate of that one and updating its IS with the 11.2 incompatibility details.

joegraduate’s picture

@tadean also noted that the removal of the ProxyClass and changing the lazy behavior of the config.installer service probably constitutes a performance regression (probably especially for sites using Drupal core < 11.2?).

It does seem like it would be desirable to keep the lazy behavior / ProxyClass if possible but the changes to the ConfigInstallerInterface introduced in 11.2 (the reason for this issue) make that a much more complicated task (because 2 different versions of the ProxyClass are needed to provide compatibility with Drupal >=11.2 and Drupal <11.2).

joegraduate’s picture

Title: Drupal 11.2 compatibility issues » Update config_provider to be compatible with ConfigInstallerInterface changes in Drupal >=11.2
tadean’s picture

@joegraduate here's a patch versus MR !20 with a potential approach that keeps the lazy loading, but might be able to avoid the need for a backward compatibility Proxy class by inheriting from the \Drupal\Core\ProxyClass\Config\ConfigInstaller.

Some considerations: this would be less code to maintain than the two-proxy approach (in MR !21), but perhaps there are reasons to avoid inheriting from the \Drupal\Core\ProxyClass\Config\ConfigInstaller? (Though, it is not marked internal). The approach in this patch makes no claims about the API of the Proxy by saying it's whatever the Core Proxy uses, essentially.

Since all interface calls are deferred through the public functions in the lazy loading service via lazyLoadItself(), perhaps \Drupal\Core\ProxyClass\Config\ConfigInstaller already does everything it needs to, we just need it be available under a different class name.

anicho01’s picture

Hey -

Do you all know when this will be released?

tadean’s picture

@anicho01 sorry for the delay on getting this incorporated into a release - we're hoping to very soon.

tadean’s picture

Status: Needs review » Reviewed & tested by the community

Discussed the two potential MRs with @joegraduate and we agreed that the approach in MR !20 is less likely to create problems for other modules that need to extend the core service. The performance impact of not pursing the proxy class approach appears to be relatively small compared to the overhead of introducing two versions of the proxy class to support the two different core APIs.

@joegraduate proposed the idea of a followup improvement to pass along the inner argument to the decorated service for reuse, with this MR !20 in this issue being merged for 11.2 compatibility.

tadean changed the visibility of the branch 3532693-drupal-11.2-compatibility-keep-proxy to hidden.

  • tadean committed d1d8bc63 on 3.0.x authored by joegraduate
    Issue #3532693 by joegraduate, tadean: Update config_provider to be...
joegraduate’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @tadean!

Status: Fixed » Closed (fixed)

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

mprell’s picture

We're experiencing PreExistingConfigException during profile installation after upgrading to alpha3.

This appears related to the decorator change merged here. The followup mentioned in Comment #16 about "passing along the inner argument to the decorated service for reuse" seems critical.

I tested the patch from #2800839 which injects @config_provider.config.installer.inner and stores it in $this->configInstaller, but the getConfigToCreate() method still calls parent:: instead of delegating to the inner service, and I think this bypasses any other decorators in the chain (like Features).

Pinning to alpha2 resolves the issue but breaks Drupal 11.2 compatibility. Happy to help test any proposed fix.