Add a new method to the DrupalConfig class that allows you to revert to the default provided by the module. This basically replaces variable_del from Drupal 7 where variable_del was being used to revert back to the default provided by a variable_get.

Sample usage:

  $config = config('aggregator.admin');
  $config->set('aggregator_fetcher', 'test');
  $config->save();
  $config->revert('aggregator_fetcher');
  $config->save();

This patch requires that a config object / file is named module_name.config_space.xml so that we can work out which module is providing the defaults.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

Patch now has tests and fixes a php warning it was causing.

This approach might not correct as it introduces a dependency on the name of the config file and module. The better option is probably to scan the directory for the file.

sun’s picture

Issue tags: +Configuration system, +VDC
vlad.ilyich’s picture

possibly stupid comment - don't we need to be mindful of the fact that not all default configuration at module install time is not convered by what's in the file? any module that checks the state of say, installed views, or content types, or roles, or image styles, or ..., and provides a different install time default based on that?

checking files to revert to a default will surely cover some cases. perhaps we need to fire some code here?

tim.plunkett’s picture

Status: Active » Postponed

I'd think this would be better served using whatever comes out of #1515312: Add snapshots of last loaded config for tracking whether config has changed

sun’s picture

I may be mistaken, but I think that the Config class is the wrong place to add a revert method.

We already discussed the sense and nonsense of a revert functionality in:
#1398040: Detect if default configuration of a module has been changed, and allow to restore to the original

So I think that, if at all, then a revert method belongs onto ConfigEntityBase. (which sorta ties into #1642062: Add TempStore for persistent, limited-term storage of non-cache data instead of reverting to default config)

jibran’s picture

Status: Postponed » Active

as per #4

gdd’s picture

I talked to timplunkett some time back when we were working on #1515312: Add snapshots of last loaded config for tracking whether config has changed. So one thing is that not everything can be reverted. For instance many field-related changes can not be undone (they change data irrevocably.) This means in order for revert to work at all, we need to add the concept of things that can and can't be reverted. We both agree with sun that this should be added on ConfigEntityBase, then the individual implementations can decide whether to use it or not. This would be similar in approach to #1826602: Allow all configuration entities to be enabled/disabled. I know he planned to work on this for Views so I'll check in with him to see what his status is.

tim.plunkett’s picture

Assigned: alexpott » dawehner

I'm pretty sure dawehner had code for this in a sandbox, I'm not sure how functional it was. But yes, what @heyrocker said is accurate.

dawehner’s picture

I know damian started on #1790398: Re introduce revert functionality for views using the config system but now I'm currently working on a generic functionality for the config entity, not specific for views.

dawehner’s picture

Just some basic outline what tstoeckler and I have in mind:

  • Add a revert method on the ConfigEntityInterface
  • Add a RevertableEntityStorageControllerInterface which allows to revert() and find out whether a certain entity is overridden().
    You don't want to couple that directly to the ConfigStorageController, as this is not what we want all the time.
  • To find out whether something is overridden, we can use config_get_changes, though in order to have this loaded lazy/make easy, we could store this information in the manifest file, during save, and use that information.

What do you think about that in general? Just posting random code.

ianthomas_uk’s picture

I have been advised that for configuration sets with variable names (e.g. \Drupal::config('menu.entity.node.'.$content_type')), defaults should be provided via hooks (in the case of the example by implementing menu_content_type_insert and menu_install).

If this is best practise, then any patch for this issue needs to take that into account.

pfrenssen’s picture

We now have StorageComparer which we can use to check if something is overridden.

Anonymous’s picture

Title: Add revert function to the DrupalConfig class » Add revert functionality to Config entities

there is no DrupalConfig anymore.

if this really is for Config, then i don't think we want it.

if it's for ConfigEntities, meh, i don't have strong opinions.

mtift’s picture

jhodgdon’s picture

Issue summary: View changes

Is this still on the table?

dawehner’s picture

Assigned: dawehner » Unassigned

I don't know to be honest. (unassign from myself)

Note: For some usecases of revert you can use config_devel, which reverts all the changes to the config files for you.

jhodgdon’s picture

See also https://www.drupal.org/sandbox/jhodgdon/2391835 - a sandbox module that has config revert functionality.

jhodgdon’s picture

Version: 8.0.x-dev » 8.1.x-dev
Category: Task » Feature request

I was looking at this issue and other Core issue to see whether my sandbox (see previous comment) should/could be added to Core to resolve them. But the others, and I think this one too, are really feature requests. So given where we are in the cycle, I'm afraid this is 8.1.x... "Add functionality" to me sounds like a feature request?

tkoleary’s picture

Added to config management usability meta #2642404

tkoleary’s picture

jhodgdon’s picture

By the way, if anyone wants to pursue this issue further... the (currently contrib) Config Update project has some code to manage diffs and revisions in it:
https://www.drupal.org/project/config_update

It is fairly well developed, and has an 8.x full release now. There are services for diffing and reverting in the base module, which is being used by several other contrib modules like Features that are related to config management. There is also a UI module in the project that has UI for reports, diffs, and reverts.

xjm’s picture

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mpotter’s picture

Sorry to bump an old issue, but in studying different modules that "revert" or "import" config, found that Core code in ConfigInstaller also does this and potentially just needs a simple refactor to provide a common method that can be used:

  • In config_update called from ConfigReverter::import() and ConfigReverter::revert()
  • In config_actions called from ConfigActionsId::doSave()
  • In core ConfigInstaller::createConfiguration()

Working on ConfigInstaller patch here: #3002026: Improve API for creating/updating configuration as part of CMI 2.0 initiative. Patch is still in progress, but comments welcome there.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

thenchev’s picture

Status: Active » Needs review
FileSize
5.63 KB

Time to wake up this beauty up after so long

The code is from the earlier mentioned Config Update module
This is mostly to get the ball rolling.

Is the core Config class the right place for this?
Do we consider the revert functionality to be important enough to be in the Config class or do we want to put it in a EXPERIMENTAl module first?
There is a dependency on Drush, this is probably something we don't want to include?

alexpott’s picture

@thenchev thanks for working on this.

I think the revert functionality should either live in it's own service in core.services or maybe in the ConfigManager. I think the Config class should remain unaware of whether it is dealing with a configuration entity or (so-called) simple configuration.

With respect to the drush - once this is part of core we should open an issue to add a revert command to Drush. And we also should have an issue to move move config_update to using the new core functionality.

thenchev’s picture

Status: Needs review » Needs work
FileSize
10.09 KB

Thanks @alexpott

Created a service for the revert functionality, added it also to the ConfigManager. Let me know if that works.

I still need to do some more testing and upload a patch with the test coverage so this is still WIP

thenchev’s picture

yogeshmpawar’s picture

Status: Needs work » Needs review
jonathan1055’s picture

Assigned: Unassigned » jonathan1055

Just checked with @thenchev on slack, and I'm going to fix the patch so that the tests run.

alexpott’s picture

Status: Needs review » Needs work
  1. +++ b/core/core.services.yml
    @@ -306,7 +306,7 @@ services:
       config.manager:
         class: Drupal\Core\Config\ConfigManager
    -    arguments: ['@entity_type.manager', '@config.factory', '@config.typed', '@string_translation', '@config.storage', '@event_dispatcher', '@entity.repository', '@extension.path.resolver']
    +    arguments: ['@entity_type.manager', '@config.factory', '@config.typed', '@string_translation', '@config.storage', '@event_dispatcher', '@entity.repository', '@config.reverter', '@extension.path.resolver']
    

    I don't think we should be adding revert functionality to the ConfigManager in this issue - could be a follow-up if we decide that's what we want.

  2. +++ b/core/core.services.yml
    @@ -354,6 +354,12 @@ services:
    +  config.storage.install:
    +    class: Drupal\Core\Config\ExtensionInstallStorage
    +    arguments: ['@config.storage', 'config/install', '', true, '%install_profile%']
    +  config.storage.optional_install:
    +    class: Drupal\Core\Config\ExtensionInstallStorage
    +    arguments: ['@config.storage', 'config/optional', '', true, '%install_profile%']
    

    I don't think these should be services. I thunk we should create the classes in the ConfigReverter constructor. Or something similar. The reason being is that this should never really be swapped.

  3. +++ b/core/lib/Drupal/Core/Config/ConfigManagerInterface.php
    @@ -5,7 +5,7 @@
    -interface ConfigManagerInterface {
    +interface ConfigManagerInterface extends ConfigReverterInterface {
    

    As above - let's not make this part of the config manager just yet - these no use-case atm.

  4. +++ b/core/lib/Drupal/Core/Config/ConfigReverter.php
    @@ -0,0 +1,143 @@
    +/**
    + * This service is used for reverting configuration to the original value.
    + */
    +class ConfigReverter implements ConfigReverterInterface {
    

    There is a unit test for this class in the config_update module - we should definitely be bringing it in as part of this change.

  5. +++ b/core/lib/Drupal/Core/Config/ConfigReverter.php
    @@ -0,0 +1,143 @@
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function revert($type, $name) {
    

    The arguments here are pretty interesting. I guess the arguments are from the config_update module but it feels funny to have to work out the type and the name before calling this. Also it's odd because we have special code to add it all back together again.

    I would have thought here that we should supply just the full config name and then call \Drupal\Core\Config\ConfigManagerInterface::getEntityTypeIdByName() to determine if it is a config entity or not and then do the write thing.

    This would allow us to remove ::getFullName() which I really don't think should be here.

  6. +++ b/core/lib/Drupal/Core/Config/ConfigReverter.php
    @@ -0,0 +1,143 @@
    +    if (!$value) {
    +      return FALSE;
    +    }
    +
    +    // Make sure the configuration exists currently in active storage.
    +    if (!$this->activeConfigStorage->read($full_name)) {
    +      return FALSE;
    +    }
    

    I think both the return FALSE here should throw an exception that can be caught by the caller. The caller really should only be providing configuration that can be reverted. Potentially the question of revertable-ness should be answered in another method on this service.

  7. +++ b/core/lib/Drupal/Core/Config/ConfigReverter.php
    @@ -0,0 +1,143 @@
    +    // Load the current config and replace the value, retaining the config
    +    // hash (which is part of the _core config key's value).
    +    if ($type == 'system.simple') {
    +      $config = $this->configFactory->getEditable($full_name);
    +      $core = $config->get('_core');
    +      $config
    +        ->setData($value)
    +        ->set('_core', $core)
    +        ->save();
    +    }
    +    else {
    +      $definition = $this->entityManager->getDefinition($type);
    +      $id_key = $definition->getKey('id');
    +      $id = $value[$id_key];
    +      $entity_storage = $this->entityManager->getStorage($type);
    +      $entity = $entity_storage->load($id);
    +      $core = $entity->get('_core');
    +      $entity = $entity_storage->updateFromStorageRecord($entity, $value);
    +      $entity->set('_core', $core);
    +      $entity->save();
    +    }
    

    The copy of the _core hash here gets to an interesting. It's not really revert to state at install - it's revert to the current state of the configuration provided by the module - so things might have changed. I think we should be re-calculating this in the same way that the ConfigInstaller does.

    And this in turn make me realise that we have further complexities to deal with. It's quite possible that dependencies of this configuration as it is provided by the module might have been deleted and therefore the configuration might not actually be valid for the current site. This adds a whole layer of complexity that is just not dealt with by the contrib code but something we'll need to handle if we're going to do this in core.

thenchev’s picture

Status: Needs work » Needs review
FileSize
6.04 KB
6.84 KB

1. Sounds good, this makes the patch a lot smaller.
2. Moving to the constructor (LocaleDefaultConfigStorage does the same thing)
3. Removed the Interface
4. WIP
5. WIP, still testing it
6. Makes sense, WIP while doing the refactor
7. Relevant issue from config_update module for handling the core hash => #2829506: Simple config doesn't retain config hash when reverted. Also you are right, if the config is completely removed from the module that provided it then there will be issues. Do we need to handle that case? I think we should throw an exception if there is nothing to revert to, even if removing it is the correct state of the config.

thenchev’s picture

ranjith_kumar_k_u’s picture

Status: Needs review » Needs work

The last submitted patch, 46: 1497268-46.patch, failed testing. View results

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

alexpott’s picture

nedjo’s picture

A basic problem with the current patch's approach is suggested in this open issue on Configuration Update Manager: #2852638: RevertInterface::revert() and ::import() do not handle config dependencies. See also this prior issue: #2423211: Review config dependency handling in import operation.

Configuration Update Manager is a great module and super useful for many purposes.

Still, the root problem is that Configuration Update Manager methods act on individual config entities, treating them as if they were atomic entities. Existing core config handling in ConfigImporter and ConfigInstaller, in contrast, handles configuration as sets of (often interrelated) entities, including dependency relationships.

In Configuration Synchronizer, we use ConfigImporter to accomplish config reverting. It's not clear there's a need in core for a new specialized API method to apply only to individual config items.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.