Issue #2232275 by alexpott: System menu blocks need to be able to depend on their menu entities.

core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php

@@ -310,6 +310,14 @@ public function calculateDependencies() {
       foreach($plugin_bag as $instance) {
         $definition = $instance->getPluginDefinition();
         $this->addDependency('module', $definition['provider']);
+        // Plugins can declare additional dependencies in their definition.
+        if (isset($definition['config_dependencies'])) {
+          foreach ($definition['config_dependencies'] as $type => $dependencies) {
+            foreach ($dependencies as $dependency) {
+              $this->addDependency($type, $dependency);
+            }
+          }
+        }
       }
     }
     return $this->dependencies;

core/modules/system/lib/Drupal/system/Plugin/Derivative/SystemMenuBlock.php

@@ -52,6 +52,7 @@ public function getDerivativeDefinitions($base_plugin_definition) {
     foreach ($this->menuStorage->loadMultiple() as $menu => $entity) {
       $this->derivatives[$menu] = $base_plugin_definition;
       $this->derivatives[$menu]['admin_label'] = $entity->label();
+      $this->derivatives[$menu]['config_dependencies']['entity'] = array($entity->getConfigDependencyName());
     }
     return $this->derivatives;
   }

core/modules/system/lib/Drupal/system/Tests/Block/SystemMenuBlockTest.php

+<?php
+/**
+ * Contains \Drupal\system\Tests\Block\SystemMenuBlockTest
+ */
+
+namespace Drupal\system\Tests\Block;
+
+use Drupal\simpletest\DrupalUnitTestBase;
+
+/**
+ * Tests \Drupal\system\Plugin\Block\SystemMenuBlock
+ *
+ * @todo Expand test coverage to all SystemMenuBlock functionality, including
+ *   block_menu_delete().
+ *
+ * @see \Drupal\system\Plugin\Derivative\SystemMenuBlock
+ * @see \Drupal\system\Plugin\Block\SystemMenuBlock
+ */
+class SystemMenuBlockTest extends DrupalUnitTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('system', 'menu_link', 'block');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'System menu block test',
+      'description' => 'Tests \Drupal\system\Plugin\Block\SystemMenuBlock.',
+      'group' => 'System blocks',
+    );
+  }
+
+  /**
+   * Tests calculation of a system menu block's configuration dependencies.
+   */
+  public function testSystemMenuBlockConfigDependencies() {
+    // Add a new custom menu.
+    $menu_name = $this->randomName(16);
+    $label = $this->randomName(16);
+
+    $menu = entity_create('menu', array(
+      'id' => $menu_name,
+      'label' => $label,
+      'description' => 'Description text',
+    ));
+    $menu->save();
+
+    $block = entity_create('block', array(
+      'plugin' => 'system_menu_block:'. $menu->id(),
+      'region' => 'footer',
+      'id' => 'machinename',
+      'theme' => 'stark',
+    ));
+
+    $dependencies = $block->calculateDependencies();
+    $expected = array(
+      'entity' => array(
+        'system.menu.' . $menu->id()
+      ),
+      'module' => array(
+        'system'
+      ),
+      'theme' => array(
+        'stark'
+      ),
+    );
+    $this->assertIdentical($expected, $dependencies);
+  }
+}

core/profiles/minimal/config/block.block.stark_admin.yml

@@ -18,6 +18,8 @@ visibility:
   node_type:
     types: {  }
 dependencies:
+  entity:
+    - system.menu.admin
   module:
     - system
   theme:

core/profiles/minimal/config/block.block.stark_tools.yml

@@ -18,6 +18,8 @@ visibility:
   node_type:
     types: {  }
 dependencies:
+  entity:
+    - system.menu.tools
   module:
     - system
   theme:

core/profiles/standard/config/block.block.bartik_footer.yml

@@ -20,6 +20,8 @@ visibility:
       article: '0'
       page: '0'
 dependencies:
+  entity:
+    - system.menu.footer
   module:
     - system
   theme:

core/profiles/standard/config/block.block.bartik_tools.yml

@@ -20,6 +20,8 @@ visibility:
       article: '0'
       page: '0'
 dependencies:
+  entity:
+    - system.menu.tools
   module:
     - system
   theme:

core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityBaseUnitTest.php

@@ -11,6 +11,7 @@
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Component\Plugin\PluginBase;
 use Drupal\Core\Language\Language;
+use Drupal\Tests\Core\Plugin\Fixtures\TestConfigurablePlugin;
 use Drupal\Tests\UnitTestCase;
 
 /**
@@ -197,8 +198,10 @@ public function testAddDependency() {
 
   /**
    * @covers ::calculateDependencies
+   *
+   * @dataProvider providerCalculateDependenciesWithPluginBag
    */
-  public function testCalculateDependenciesWithPluginBag() {
+  public function testCalculateDependenciesWithPluginBag($definition, $expected_dependencies) {
     $values = array();
     $this->entity = $this->getMockBuilder('\Drupal\Tests\Core\Config\Entity\Fixtures\ConfigEntityBaseWithPluginBag')
       ->setConstructorArgs(array($values, $this->entityTypeId))
@@ -207,7 +210,7 @@ public function testCalculateDependenciesWithPluginBag() {
 
     // Create a configurable plugin that would add a dependency.
     $instance_id = $this->randomName();
-    $instance = new TestConfigurablePlugin(array(), $instance_id, array('provider' => 'test'));
+    $instance = new TestConfigurablePlugin(array(), $instance_id, $definition);
 
     // Create a plugin bag to contain the instance.
     $pluginBag = $this->getMockBuilder('\Drupal\Core\Plugin\DefaultPluginBag')
@@ -222,45 +225,50 @@ public function testCalculateDependenciesWithPluginBag() {
 
     // Return the mocked plugin bag.
     $this->entity->expects($this->once())
-                 ->method('getPluginBag')
-                 ->will($this->returnValue($pluginBag));
+      ->method('getPluginBag')
+      ->will($this->returnValue($pluginBag));
 
-    $dependencies = $this->entity->calculateDependencies();
-    $this->assertContains('test', $dependencies['module']);
+    $this->assertEquals($expected_dependencies, $this->entity->calculateDependencies());
   }
 
   /**
-   * @covers ::calculateDependencies
+   * Data provider for testCalculateDependenciesWithPluginBag.
+   *
+   * @return array
    */
-  public function testCalculateDependenciesWithPluginBagSameProviderAsEntityType() {
-    $values = array();
-    $this->entity = $this->getMockBuilder('\Drupal\Tests\Core\Config\Entity\Fixtures\ConfigEntityBaseWithPluginBag')
-                         ->setConstructorArgs(array($values, $this->entityTypeId))
-                         ->setMethods(array('getPluginBag'))
-                         ->getMock();
-
-    // Create a configurable plugin that will not add a dependency since it is
-    // provider matches the provider of the entity type.
-    $instance_id = $this->randomName();
-    $instance = new TestConfigurablePlugin(array(), $instance_id, array('provider' => $this->provider));
-
-    // Create a plugin bag to contain the instance.
-    $pluginBag = $this->getMockBuilder('\Drupal\Core\Plugin\DefaultPluginBag')
-                      ->disableOriginalConstructor()
-                      ->setMethods(array('get'))
-                      ->getMock();
-    $pluginBag->expects($this->atLeastOnce())
-              ->method('get')
-              ->with($instance_id)
-              ->will($this->returnValue($instance));
-    $pluginBag->addInstanceId($instance_id);
+  public function providerCalculateDependenciesWithPluginBag() {
+    // Start with 'a' so that order of the dependency array is fixed.
+    $instance_dependency_1 = 'a' . $this->randomName(10);
+    $instance_dependency_2 = 'a' . $this->randomName(11);
 
-    // Return the mocked plugin bag.
-    $this->entity->expects($this->once())
-                 ->method('getPluginBag')
-                 ->will($this->returnValue($pluginBag));
-
-    $this->assertEmpty($this->entity->calculateDependencies());
+    return array(
+      // Tests that the plugin provider is a module dependency.
+      array(
+        array('provider' => 'test'),
+        array('module' => array('test')),
+      ),
+      // Tests that a plugin that is provided by the same module as the config
+      // entity is not added to the dependencies array.
+      array(
+        array('provider' => $this->provider),
+        array('module' => array(NULL)),
+      ),
+      // Tests that a config entity that has a plugin which provides config
+      // dependencies in its definition has them.
+      array(
+        array(
+          'provider' => 'test',
+          'config_dependencies' => array(
+            'entity' => array($instance_dependency_1),
+            'module' => array($instance_dependency_2),
+          )
+        ),
+        array(
+          'entity' => array($instance_dependency_1),
+          'module' => array($instance_dependency_2, 'test')
+        )
+      )
+    );
   }
 
   /**
@@ -418,27 +426,3 @@ public function testToArray() {
     }
   }
 }
-
-class TestConfigurablePlugin extends PluginBase implements ConfigurablePluginInterface {
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getConfiguration() {
-    return $this->configuration;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setConfiguration(array $configuration) {
-    $this->configuration = $configuration;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function defaultConfiguration() {
-    return array();
-  }
-}