Problem/Motivation

ConfigEntityBase provides three methods to help with dependency management:

  • calculateDependencies()
  • addDependency()
  • addDependencies()

In #2248151: Configurable plugins can not control instance-specific config dependencies we added a mechanism for configurable plugins to manage their own dependencies.
However, unlike entities that had these convenience methods, plugins were left to manipulate the data structures directly.

Furthermore, if other objects need to calculate the dependencies of any plugins they manage, they would need to copy the logic used in ConfigEntityBase::calculateDependencies()

Proposed resolution

Move these methods into two traits:
One for addDependency()/addDependencies() for the data structure manipulation
One with a method called calculatePluginDependencies() just for the plugin-specific parts.

Remaining tasks

N/A

User interface changes

N/A

API changes

New traits available. No actual API change.

CommentFileSizeAuthor
#1 plugin-dependency-2266859-1.patch9.03 KBtim.plunkett

Comments

tim.plunkett’s picture

Status: Active » Needs review
StatusFileSize
new9.03 KB
tim.plunkett’s picture

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
    @@ -22,6 +23,10 @@
    +  use PluginDependencyTrait {
    +    addDependency as addDependencyTrait;
    +  }
    
    @@ -364,44 +342,8 @@ protected function addDependency($type, $name) {
         if ($type == 'module' && ($name == $this->getEntityType()->getProvider() || $name == 'Core')) {
           return $this;
         }
    ...
    +    return $this->addDependencyTrait($type, $name);
    

    Because we want to do this entity-specific check before adding dependencies, we need to do this aliasing trick with traits.

    We have no standard way to document this pattern, but while it seems weird now, it will become more common as we become more familiar with traits.

  2. +++ b/core/modules/user/lib/Drupal/user/Plugin/Action/ChangeUserRoleBase.php
    @@ -80,12 +83,11 @@ public function submitConfigurationForm(array &$form, array &$form_state) {
    -      $dependencies['entity'][] = $prefix . $this->configuration['rid'];
    +      $this->addDependency('entity', $prefix . $this->configuration['rid']);
    

    When I added this code to user roles initially, I remember being very confused about the nesting of the dependencies. This method makes it that much easier

alexpott’s picture

When I added this code to user roles initially, I remember being very confused about the nesting of the dependencies. This method makes it that much easier

This makes me +1 to this approach. Same methods for plugins and config entities is really nice and consistent - and if we need to extend this to anything else in the future (content entities???) it'll be nice.

dawehner’s picture

Priority: Major » Normal
Status: Needs review » Reviewed & tested by the community

I wonder whether we should have a dedicated test for this trait, but beside from this this seems fine.

wim leers’s picture

Yes — much, much better :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit 9b58423 on 8.x by webchick:
    Issue #2266859 by tim.plunkett: Move the dependency calculation helper...
les lim’s picture

FYI, this patch made D8 incompatible with APC 3.1.9, which is the latest stable version and the one packaged with MAMP 2. I don't think that's anything we should code around, but made a note of it at https://drupal.org/requirements/php.

tim.plunkett’s picture

Issue tags: +Page Manager

Status: Fixed » Closed (fixed)

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