Issue #2293205 by Berdir | tim.plunkett: Consider removing usage of DataDefinition in ContextHandler.

core/lib/Drupal/Core/Plugin/Context/ContextHandler.php

@@ -11,32 +11,12 @@ use Drupal\Component\Plugin\ConfigurablePluginInterface;
 use Drupal\Component\Plugin\ContextAwarePluginInterface;
 use Drupal\Component\Plugin\Exception\ContextException;
 use Drupal\Component\Utility\String;
-use Drupal\Core\TypedData\DataDefinitionInterface;
-use Drupal\Core\TypedData\DataDefinition;
-use Drupal\Core\TypedData\TypedDataManager;
 
 /**
  * Provides methods to handle sets of contexts.
  */
 class ContextHandler implements ContextHandlerInterface {
 
-  /**
-   * The typed data manager.
-   *
-   * @var \Drupal\Core\TypedData\TypedDataManager
-   */
-  protected $typedDataManager;
-
-  /**
-   * Constructs a new ContextHandler.
-   *
-   * @param \Drupal\Core\TypedData\TypedDataManager $typed_data
-   *   The typed data manager.
-   */
-  public function __construct(TypedDataManager $typed_data) {
-    $this->typedDataManager = $typed_data;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -47,37 +27,8 @@ class ContextHandler implements ContextHandlerInterface {
         return TRUE;
       }
 
-      // Build an array of requirements out of the contexts specified by the
-      // plugin definition.
-      $requirements = array();
-      /** @var $plugin_context \Drupal\Core\Plugin\Context\ContextDefinitionInterface */
-      foreach ($plugin_definition['context'] as $context_id => $plugin_context) {
-        $definition = $this->typedDataManager->getDefinition($plugin_context->getDataType());
-        $definition['type'] = $plugin_context->getDataType();
-
-        // If the plugin specifies additional constraints, add them to the
-        // constraints defined by the plugin type.
-        if ($plugin_constraints = $plugin_context->getConstraints()) {
-          // Ensure the array exists before adding in constraints.
-          if (!isset($definition['constraints'])) {
-            $definition['constraints'] = array();
-          }
-
-          $definition['constraints'] += $plugin_constraints;
-        }
-
-        // Assume the requirement is required if unspecified.
-        if (!isset($definition['required'])) {
-          $definition['required'] = TRUE;
-        }
-
-        // @todo Use context definition objects after
-        //   https://drupal.org/node/2281635.
-        $requirements[$context_id] = new DataDefinition($definition);
-      }
-
       // Check the set of contexts against the requirements.
-      return $this->checkRequirements($contexts, $requirements);
+      return $this->checkRequirements($contexts, $plugin_definition['context']);
     });
   }
 
@@ -96,17 +47,17 @@ class ContextHandler implements ContextHandlerInterface {
   /**
    * {@inheritdoc}
    */
-  public function getMatchingContexts(array $contexts, DataDefinitionInterface $definition) {
+  public function getMatchingContexts(array $contexts, ContextDefinitionInterface $definition) {
     return array_filter($contexts, function (ContextInterface $context) use ($definition) {
-      $context_definition = $context->getContextDefinition()->getDataDefinition();
+      $context_definition = $context->getContextDefinition();
 
-      // If the data types do not match, this context is invalid.
+      // If the data types do not match, this context is invalid unless the
-      if ($definition->getDataType() != $context_definition->getDataType()) {
+      // expected data type is any, which means all data types are supported.
+      if ($definition->getDataType() != 'any' && $definition->getDataType() != $context_definition->getDataType()) {
         return FALSE;
       }
 
       // If any constraint does not match, this context is invalid.
-      // @todo This is too restrictive, consider only relying on data types.
       foreach ($definition->getConstraints() as $constraint_name => $constraint) {
         if ($context_definition->getConstraint($constraint_name) != $constraint) {
           return FALSE;

core/lib/Drupal/Core/Plugin/Context/ContextHandlerInterface.php

@@ -8,7 +8,6 @@
 namespace Drupal\Core\Plugin\Context;
 
 use Drupal\Component\Plugin\ContextAwarePluginInterface;
-use Drupal\Core\TypedData\DataDefinitionInterface;
 
 /**
  * Provides an interface for handling sets of contexts.
@@ -53,13 +52,13 @@ interface ContextHandlerInterface {
    *
    * @param \Drupal\Component\Plugin\Context\ContextInterface[] $contexts
    *   An array of contexts.
-   * @param \Drupal\Core\TypedData\DataDefinitionInterface $definition
+   * @param \Drupal\Core\Plugin\Context\ContextDefinitionInterface $definition
    *   The definition to satisfy.
    *
    * @return \Drupal\Component\Plugin\Context\ContextInterface[]
    *   An array of matching contexts.
    */
-  public function getMatchingContexts(array $contexts, DataDefinitionInterface $definition);
+  public function getMatchingContexts(array $contexts, ContextDefinitionInterface $definition);
 
   /**
    * Prepares a plugin for evaluation.

core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php

@@ -11,8 +11,6 @@ use Drupal\Component\Plugin\ConfigurablePluginInterface;
 use Drupal\Component\Plugin\ContextAwarePluginInterface;
 use Drupal\Core\Plugin\Context\ContextDefinition;
 use Drupal\Core\Plugin\Context\ContextHandler;
-use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\TypedData\DataDefinition;
 use Drupal\Tests\UnitTestCase;
 
 /**
@@ -21,13 +19,6 @@ use Drupal\Tests\UnitTestCase;
  */
 class ContextHandlerTest extends UnitTestCase {
 
-  /**
-   * The typed data manager.
-   *
-   * @var \Drupal\Core\TypedData\TypedDataManager|\PHPUnit_Framework_MockObject_MockObject
-   */
-  protected $typedDataManager;
-
   /**
    * The context handler.
    *
@@ -43,28 +34,7 @@ class ContextHandlerTest extends UnitTestCase {
   protected function setUp() {
     parent::setUp();
 
-    $this->typedDataManager = $this->getMockBuilder('Drupal\Core\TypedData\TypedDataManager')
+    $this->contextHandler = new ContextHandler();
-      ->disableOriginalConstructor()
-      ->getMock();
-    $this->typedDataManager->expects($this->any())
-      ->method('getDefaultConstraints')
-      ->will($this->returnValue(array()));
-    $this->typedDataManager->expects($this->any())
-      ->method('createDataDefinition')
-      ->will($this->returnValueMap(array(
-        array('expected_data_type', new DataDefinition(array('type' => 'expected_data_type'))),
-        array('mismatched_data_type', new DataDefinition(array('type' => 'mismatched_data_type'))),
-        array('unexpected_data_type', new DataDefinition(array('type' => 'unexpected_data_type'))),
-        array('empty', new DataDefinition(array())),
-        array('foo', new DataDefinition(array('type' => 'foo'))),
-        array('fuzzy', new DataDefinition(array('type' => 'fuzzy'))),
-        array('specific', new DataDefinition(array('type' => 'foo'))),
-      )));
-    $this->contextHandler = new ContextHandler($this->typedDataManager);
-
-    $container = new ContainerBuilder();
-    $container->set('typed_data_manager', $this->typedDataManager);
-    \Drupal::setContainer($container);
   }
 
   /**
@@ -80,37 +50,19 @@ class ContextHandlerTest extends UnitTestCase {
    * Provides data for testCheckRequirements().
    */
   public function providerTestCheckRequirements() {
-    $requirement_optional = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
+    $requirement_optional = new ContextDefinition();
-    $requirement_optional->expects($this->atLeastOnce())
+    $requirement_optional->setRequired(FALSE);
-      ->method('isRequired')
-      ->will($this->returnValue(FALSE));
 
-    $requirement_any = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
+    $requirement_any = new ContextDefinition();
-    $requirement_any->expects($this->atLeastOnce())
+    $requirement_any->setRequired(TRUE);
-      ->method('isRequired')
-      ->will($this->returnValue(TRUE));
-    $requirement_any->expects($this->atLeastOnce())
-      ->method('getDataType')
-      ->will($this->returnValue('any'));
-    $requirement_any->expects($this->atLeastOnce())
-      ->method('getConstraints')
-      ->will($this->returnValue(array()));
 
     $context_any = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface');
     $context_any->expects($this->atLeastOnce())
       ->method('getContextDefinition')
       ->will($this->returnValue(new ContextDefinition('empty')));
 
-    $requirement_specific = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
+    $requirement_specific = new ContextDefinition('specific');
-    $requirement_specific->expects($this->atLeastOnce())
+    $requirement_specific->setConstraints(array('bar' => 'baz'));
-      ->method('isRequired')
-      ->will($this->returnValue(TRUE));
-    $requirement_specific->expects($this->atLeastOnce())
-      ->method('getDataType')
-      ->will($this->returnValue('foo'));
-    $requirement_specific->expects($this->atLeastOnce())
-      ->method('getConstraints')
-      ->will($this->returnValue(array('bar' => 'baz')));
 
     $context_constraint_mismatch = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface');
     $context_constraint_mismatch->expects($this->atLeastOnce())
@@ -157,26 +109,10 @@ class ContextHandlerTest extends UnitTestCase {
    * Provides data for testGetMatchingContexts().
    */
   public function providerTestGetMatchingContexts() {
-    $requirement_any = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
+    $requirement_any = new ContextDefinition();
-    $requirement_any->expects($this->atLeastOnce())
+
-      ->method('isRequired')
+    $requirement_specific = new ContextDefinition('specific');
-      ->will($this->returnValue(TRUE));
+    $requirement_specific->setConstraints(array('bar' => 'baz'));
-    $requirement_any->expects($this->atLeastOnce())
-      ->method('getDataType')
-      ->will($this->returnValue('any'));
-    $requirement_any->expects($this->atLeastOnce())
-      ->method('getConstraints')
-      ->will($this->returnValue(array()));
-    $requirement_specific = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface');
-    $requirement_specific->expects($this->atLeastOnce())
-      ->method('isRequired')
-      ->will($this->returnValue(TRUE));
-    $requirement_specific->expects($this->atLeastOnce())
-      ->method('getDataType')
-      ->will($this->returnValue('foo'));
-    $requirement_specific->expects($this->atLeastOnce())
-      ->method('getConstraints')
-      ->will($this->returnValue(array('bar' => 'baz')));
 
     $context_any = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface');
     $context_any->expects($this->atLeastOnce())
@@ -218,7 +154,7 @@ class ContextHandlerTest extends UnitTestCase {
    *
    * @dataProvider providerTestFilterPluginDefinitionsByContexts
    */
-  public function testFilterPluginDefinitionsByContexts($has_context, $definitions, $expected, $typed_data_definition = NULL) {
+  public function testFilterPluginDefinitionsByContexts($has_context, $definitions, $expected) {
     if ($has_context) {
       $context = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface');
       $expected_context_definition = (new ContextDefinition('expected_data_type'))->setConstraints(array('expected_constraint_name' => 'expected_constraint_value'));
@@ -231,12 +167,6 @@ class ContextHandlerTest extends UnitTestCase {
       $contexts = array();
     }
 
-    if ($typed_data_definition) {
-      $this->typedDataManager->expects($this->atLeastOnce())
-        ->method('getDefinition')
-        ->will($this->returnValueMap($typed_data_definition));
-    }
-
     $this->assertSame($expected, $this->contextHandler->filterPluginDefinitionsByContexts($contexts, $definitions));
   }
 
@@ -260,7 +190,7 @@ class ContextHandlerTest extends UnitTestCase {
 
     $plugins = array('expected_plugin' => array('context' => array('context1' => new ContextDefinition('expected_data_type'))));
     // Missing context, no plugins available.
-    $data[] = array(array(), $plugins, array());
+    $data[] = array(FALSE, $plugins, array());
     // Satisfied context, all plugins available.
     $data[] = array(TRUE, $plugins, $plugins);
 
@@ -269,42 +199,30 @@ class ContextHandlerTest extends UnitTestCase {
     // Mismatched constraints, no plugins available.
     $data[] = array(TRUE, $plugins, array());
 
+    $optional_mismatched_context_definition = clone $mismatched_context_definition;
+    $optional_mismatched_context_definition->setRequired(FALSE);
+    $plugins = array('expected_plugin' => array('context' => array('context1' => $optional_mismatched_context_definition)));
+    // Optional mismatched constraint, all plugins available.
+    $data[] = array(FALSE, $plugins, $plugins);
+
     $expected_context_definition = (new ContextDefinition('expected_data_type'))->setConstraints(array('expected_constraint_name' => 'expected_constraint_value'));
     $plugins = array('expected_plugin' => array('context' => array('context1' => $expected_context_definition)));
     // Satisfied context with constraint, all plugins available.
     $data[] = array(TRUE, $plugins, $plugins);
 
-    $typed_data = array(array('expected_data_type', TRUE, array('required' => FALSE)));
+    $optional_expected_context_definition = clone $expected_context_definition;
-    // Optional unsatisfied context from TypedData, all plugins available.
+    $optional_expected_context_definition->setRequired(FALSE);
-    $data[] = array(FALSE, $plugins, $plugins, $typed_data);
+    $plugins = array('expected_plugin' => array('context' => array('context1' => $optional_expected_context_definition)));
-
+    // Optional unsatisfied context, all plugins available.
-    $typed_data = array(array('expected_data_type', TRUE, array('required' => TRUE)));
+    $data[] = array(FALSE, $plugins, $plugins);
-    // Required unsatisfied context from TypedData, no plugins available.
-    $data[] = array(FALSE, $plugins, array(), $typed_data);
-
-    $typed_data = array(array('expected_data_type', TRUE, array('constraints' => array('mismatched_constraint_name' => 'mismatched_constraint_value'), 'required' => FALSE)));
-    // Optional mismatched constraint from TypedData, all plugins available.
-    $data[] = array(FALSE, $plugins, $plugins, $typed_data);
-
-    $typed_data = array(array('expected_data_type', TRUE, array('constraints' => array('mismatched_constraint_name' => 'mismatched_constraint_value'), 'required' => TRUE)));
-    // Required mismatched constraint from TypedData, no plugins available.
-    $data[] = array(FALSE, $plugins, array(), $typed_data);
-
-    $typed_data = array(array('expected_data_type', TRUE, array('constraints' => array('expected_constraint_name' => 'expected_constraint_value'))));
-    // Satisfied constraint from TypedData, all plugins available.
-    $data[] = array(TRUE, $plugins, $plugins, $typed_data);
 
     $unexpected_context_definition = (new ContextDefinition('unexpected_data_type'))->setConstraints(array('mismatched_constraint_name' => 'mismatched_constraint_value'));
     $plugins = array(
       'unexpected_plugin' => array('context' => array('context1' => $unexpected_context_definition)),
       'expected_plugin' => array('context' => array('context2' => new ContextDefinition('expected_data_type'))),
     );
-    $typed_data = array(
-      array('unexpected_data_type', TRUE, array()),
-      array('expected_data_type', TRUE, array('constraints' => array('expected_constraint_name' => 'expected_constraint_value'))),
-    );
     // Context only satisfies one plugin.
-    $data[] = array(TRUE, $plugins, array('expected_plugin' => $plugins['expected_plugin']), $typed_data);
+    $data[] = array(TRUE, $plugins, array('expected_plugin' => $plugins['expected_plugin']));
 
     return $data;
   }