Problem/Motivation

Currently Config::resetOverriddenData() completely removes the overriddenData class's property instead of reset the property value.

  /**
   * Resets the current data, so overrides are re-applied.
   *
   * This method should be called after the original data or the overridden data
   * has been changed.
   *
   * @return \Drupal\Core\Config\Config
   *   The configuration object.
   */
  protected function resetOverriddenData() {
    unset($this->overriddenData);
    return $this;
  }

the instruction unset($this->overriddenData); remove the class's property when the correct behaviour should be set overriddenData to NULL.

This is not a big deal in se but
1) it is not consistent (and IMHO this can be marked as a bug).
2) it can cause warning with code that use reflection (but this of course is not a strong point). Libs like Kint or Doctrine Debug are affected by this issue.

Proposed resolution

Make Config::resetOverriddenData() behaves consistently and reset only the VALUE of the property, not remove the class's property itself

Remaining tasks

Issue Summary update
Answer questions in #8 and #16

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 2833979-2.patch457 byteswillzyx

Comments

willzyx created an issue. See original summary.

willzyx’s picture

willzyx commented
Status: Active » Needs review
StatusFileSize
new 2833979-2.patch457 bytes

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.

dawehner’s picture

dawehner
Primary language German
commented

Ha, nice and quick test. I'm curious whether this results in any change of the observable universe?

chi’s picture

chi commented

Seems like not unique case.
$ ag 'unset\(\$this->[a-z]*\)'

core/modules/system/src/Tests/Cache/GenericCacheBackendUnitTestBase.php
123:    unset($this->cachebackends);

core/modules/rest/src/Tests/RESTTestBase.php
434:    unset($this->response);

core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
308:    unset($this->iterator);

core/modules/migrate/src/Plugin/Migration.php
534:      unset($this->sourcePlugin);
538:      unset($this->destinationPlugin);

core/modules/simpletest/src/WebTestBase.php
855:      unset($this->curlHandle);

core/modules/locale/src/LocaleTranslation.php
139:    unset($this->translateEnglish);

core/modules/views/tests/src/Kernel/ModuleTest.php
94:    unset($this->lastErrorMessage);
102:    unset($this->lastErrorMessage);
110:    unset($this->lastErrorMessage);

core/modules/field/src/Entity/FieldStorageConfig.php
276:    unset($this->schema);

core/modules/tour/src/Entity/Tour.php
165:    unset($this->keyedRoutes);

core/modules/taxonomy/src/VocabularyListBuilder.php
80:      unset($this->weightKey);

core/lib/Drupal/Core/Template/TwigNodeVisitor.php
32:        unset($this->skipRenderVarFunction);

core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php
237:    unset($this->baseFieldDefinition);

core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php
94:    unset($this->target);
95:    unset($this->id);

core/lib/Drupal/Core/Config/Config.php
176:    unset($this->overriddenData);

core/lib/Drupal/Component/Diff/Engine/DiffEngine.php
44:    unset($this->seq);
46:    unset($this->lcs);

core/tests/Drupal/KernelTests/Core/Cache/GenericCacheBackendUnitTestBase.php
120:    unset($this->cachebackends);

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.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

Does #5 mean that we should fix this for the other occurances as well?

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.

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.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Priority: Normal » Minor
Status: Needs review » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

What's the impact here? Can we get that added to the issue summary

Feels minor at best

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.

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.

quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: +Needs issue summary update

Adding tag for IS update.

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.

acbramley’s picture

acbramley commented
Category: Bug report » Task

Based on the lack of feedback since #16 and given this is not unique to this class I can't see how this should be classified as a bug.

Swapping to a task, IMO we could/close this if no one can answer #16 within a couple of months?

quietone’s picture

quietone commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

It has been 3 years since a committer asked for more information and none has been supplied. @acbramley suggested waiting a few more months, and that was about 1 year ago and still no comment. Therefore, I am closing this.

If there is work to do here, then either re-open the issue or open a new issue and reference this one. If the choice is to use this issue then add a comment change make sure to change the issue status to 'Active'.