Problem/Motivation

Originally the \Drupal\Core\Config\ExportStorageManager was dispatching the transformation event only when needed. For that reason it was listening to events related to configuration changes. In #3077504: Add config_exclude functionality to core we added StorageRebuildNeededEvent because sometimes things might change that are not just dependent on the active configuration.
In the patches for #2991683: Move configuration transformation API in \Drupal\Core\Config namespace (in particular #43) we found out that all the moving parts for this cause some unnecessary trouble with the installer.

Proposed resolution

Less moving parts but dispatching of transformation event more often.
This makes the API more concise. Importing and exporting configuration is a relatively rare event so this does not have a negative impact on site performance.

Remaining tasks

create patch
review
commit

User interface changes

none

API changes

very recently added (in experimental module) event is removed again as it becomes unnecessary.

Data model changes

none

Release notes snippet

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bircher created an issue. See original summary.

bircher’s picture

Status: Active » Needs review
FileSize
13.07 KB

Attached a patch that removes complexity at the cost of maybe-redundant event dispatching.

alexpott’s picture

+1 for this change. Less moving parts. If we discover later that we need to add something like this then we can.

wouter.adem’s picture

  1. +++ b/core/modules/config_environment/src/Core/Config/ExportStorageManager.php
    @@ -21,15 +21,10 @@
      *
      * @internal
      */
    -class ExportStorageManager implements StorageManagerInterface, EventSubscriberInterface {
    +class ExportStorageManager implements StorageManagerInterface {

    Minor. Remove use statement for EventSubscriberInterface.

  2. -  public function __construct(StorageInterface $active, StateInterface $state, Connection $connection, EventDispatcherInterface $event_dispatcher) {
    +  public function __construct(StorageInterface $active, Connection $connection, EventDispatcherInterface $event_dispatcher) {

    Minor. Remove use statement for StateInterface.

  3. -   * @param \Drupal\Core\State\StateInterface $state
    -   *   The Drupal state.
        */
    -  public function __construct(StorageInterface $active_storage, Settings $settings, ConfigManagerInterface $manager, StateInterface $state) {

    Minor. Remove use statement for StateInterface

  4. -   * @param \Drupal\Core\Config\StorageRebuildNeededEvent $event
    -   *   The event to control the storage rebuild.
    -   */
    -  public function onExportStorageNeedsRebuild(StorageRebuildNeededEvent $event) {

    Minor. Remove use statement for StorageRebuildNeededEvent.

  5. -    // Save the config to trigger the rebuild.
    +    // Save the config to test transforming always the active config.
         $this->config('system.site')
           ->set('name', 'New name')
           ->set('slogan', 'New slogan')

    Minor. Comment is a bit unclear. I'd put just something like: Save the config to active storage so that the transformer can alter it.

Krzysztof Domański’s picture

-- edited --

Krzysztof Domański’s picture

Changes according to #4.

Krzysztof Domański’s picture

This can also be removed.

wouter.adem’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 3f4d403 and pushed to 8.8.x. Thanks!

diff --git a/core/modules/config_environment/src/Core/Config/ConfigEvents.php b/core/modules/config_environment/src/Core/Config/ConfigEvents.php
index c932984ba6..8a87c54a93 100644
--- a/core/modules/config_environment/src/Core/Config/ConfigEvents.php
+++ b/core/modules/config_environment/src/Core/Config/ConfigEvents.php
@@ -71,7 +71,6 @@ final class ConfigEvents {
    *
    * @see \Drupal\Core\Config\StorageTransformEvent
    * @see \Drupal\Core\Config\ConfigEvents::STORAGE_TRANSFORM_IMPORT
-   * @see \Drupal\Core\Config\ConfigEvents::STORAGE_EXPORT_REBUILD
    * @see \Drupal\config_environment\Core\Config\ExportStorageManager::getStorage
    *
    * @var string

Removed @see to removed constant on commit.

Krzysztof Domański’s picture

3f4d403 returns "Git Resource Not found" and I don't see this commit on https://git.drupalcode.org/project/drupal/commits/8.8.x.

Krzysztof Domański’s picture

Status: Fixed » Reviewed & tested by the community

Back to RTBC due to #10.

  • alexpott committed 3f4d403 on 8.8.x
    Issue #3083065 by Krzysztof Domański, bircher, wouter.adem, alexpott:...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Krzysztof Domański’s picture

Thanks!

Status: Fixed » Closed (fixed)

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