Problem/Motivation

We like how configuration dependency system works. We like onDependencyRemoval(), but we miss so much an onDependencyUpdating().

Proposed resolution

Allow configuration entities to react on their dependencies update:

  • Add a new listener onDependencyUpdating() on config entities.
  • Assure BC for Drupal 8.
  • One use-case conversion: In editor.module, convert editor_filter_format_presave() (hook_ENTITY_TYPE_presave()) in Editor::onDependencyUpdating(). This hook implementation has already tests.

Remaining tasks

  • Fill an issue for D9 to remove the BC layer.
  • Discuss the opportunity to add more fine grained events: onEnable, onDisable, onIdRename, etc.

User interface changes

None.

API changes

New interfaces and methods:

ConfigManagerDependencyFinderInterface implemented by ConfigManager

  • getConfigEntitiesToChange()

ConfigEntityDependencyResolverInterface implemented by ConfigEntityBase

  • onDependencyUpdating()
  • setDependencyResolved()
  • isDependencyResolved()

Data model changes

None.

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
22.55 KB

A first PoC patch. the test is already there, in the Editor module.

Status: Needs review » Needs work

The last submitted patch, 2: 2647576-2.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
4.13 KB
24.92 KB

Fixed unit tests.

Status: Needs review » Needs work

The last submitted patch, 4: 2647576-4.patch, failed testing.

claudiu.cristea’s picture

Title: Resolve config dependency updates » React on config dependency updating - aka onDependencyUpdating()
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
11.78 KB
27.32 KB

Closer...

claudiu.cristea’s picture

Title: React on config dependency updating - aka onDependencyUpdating() » onDependencyUpdating() - React on dependency updating
Issue summary: View changes

IS, title.

claudiu.cristea’s picture

Issue summary: View changes
claudiu.cristea’s picture

Issue tags: -Needs tests
FileSize
32.68 KB
10.57 KB

Added tests, fixed docs.

claudiu.cristea’s picture

Issue summary: View changes

IS.

claudiu.cristea’s picture

More docs fixes.

claudiu.cristea’s picture

Improved the kernel test to test cascading updates. Other doc fixes.

Status: Needs review » Needs work

The last submitted patch, 12: ondependencyupdating-2647576-12.patch, failed testing.

claudiu.cristea’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
36.22 KB
799 bytes

Fixed the unit test.

claudiu.cristea’s picture

Other docs fixes.

dawehner’s picture

Too tired to do a proper review now, sorry.

It would be nice if the issue summary could describe the usecase here ...

+++ b/core/lib/Drupal/Core/Config/ConfigManager.php
@@ -297,39 +298,44 @@ public function findConfigEntityDependentsAsEntities($type, array $names, Config
-      /** @var \Drupal\Core\Config\Entity\ConfigEntityInterface $dependent */
+      /** @var \Drupal\Core\Config\Entity\ConfigEntityBase $dependent */

Mh, is that really an assumption we can make? What happens if people have different config entities? IMHO we need instanceof checks

claudiu.cristea’s picture

Assigned: Unassigned » alexpott

@alexpott, while you wrote the dependency management I would like to hear an opinion on this patch. The tests are passing nice and there's no BC break.

Status: Needs review » Needs work

The last submitted patch, 15: ondependencyupdating-2647576-15.patch, failed testing.

claudiu.cristea’s picture

Issue tags: +Needs reroll
kostyashupenko’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
36.53 KB

Status: Needs review » Needs work

The last submitted patch, 20: ondependencyupdating-2647576-20.patch, failed testing.

claudiu.cristea’s picture

kostyashupenko’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
36.86 KB

Reroll of #15. Needs review, because a lot new changes for the files in this patch, that came from 12 jan 2016. There were a lot of conflicts, while I was doing this reroll

Status: Needs review » Needs work

The last submitted patch, 23: ondependencyupdating-2647576-23.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 23: ondependencyupdating-2647576-23.patch, failed testing.

claudiu.cristea’s picture

Status: Needs review » Needs work

The last submitted patch, 27: 2647576-27.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
34.53 KB

Sorry, the patch was incomplete.

Status: Needs review » Needs work

The last submitted patch, 29: ondependencyupdating-2647576-29.patch, failed testing.

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.