diff --git a/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php b/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php index 4c8feb0..208bf0e 100644 --- a/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php +++ b/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php @@ -50,7 +50,7 @@ public function getDefinition($plugin_id) { // If a plugin defined itself as a derivative, merge in possible // defaults from the derivative. if ($derivative_id && isset($plugin_definition)) { - $plugin_definition += $derivative_plugin_definition ?: array(); + $plugin_definition = $this->mergeDerivativeDefinition($plugin_definition, $derivative_plugin_definition); } else { $plugin_definition = $derivative_plugin_definition; @@ -90,7 +90,7 @@ protected function getDerivatives(array $base_plugin_definitions) { // Use this definition as defaults if a plugin already defined // itself as this derivative. if ($derivative_id && isset($base_plugin_definitions[$plugin_id])) { - $derivative_definition = $base_plugin_definitions[$plugin_id] + ($derivative_definition ?: array()); + $derivative_definition = $this->mergeDerivativeDefinition($base_plugin_definitions[$plugin_id], $derivative_definition); } $plugin_definitions[$plugin_id] = $derivative_definition; } @@ -200,6 +200,26 @@ protected function getDerivativeClass($base_definition) { } /** + * Merges a base and derivative definition, taking into account empty values. + * + * @param array $base_plugin_definition + * The base plugin definition. + * @param array $derivative_definition + * The derivative plugin definition. + * + * @return array + * The merged definition. + */ + protected function mergeDerivativeDefinition($base_plugin_definition, $derivative_definition) { + // Use this definition as defaults if a plugin already defined itself as + // this derivative, but filter out empty values first. + $filtered_base = array_filter($base_plugin_definition); + $derivative_definition = $filtered_base + ($derivative_definition ?: array()); + // Add back any empty keys that the derivative didn't have. + return $derivative_definition + $base_plugin_definition; + } + + /** * Passes through all unknown calls onto the decorated object. */ public function __call($method, $args) { diff --git a/core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php b/core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php index a46f42c..c22c201 100644 --- a/core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php +++ b/core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php @@ -17,6 +17,13 @@ class DerivativeDiscoveryDecoratorTest extends UnitTestCase { /** + * The mock main discovery object. + * + * @var \Drupal\Component\Plugin\Discovery\DiscoveryInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $discoveryMain; + + /** * {@inheritdoc} */ public static function getInfo() { @@ -28,6 +35,13 @@ public static function getInfo() { } /** + * {@inheritdoc} + */ + public function setUp() { + $this->discoveryMain = $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); + } + + /** * Tests the getDerivativeFetcher method. * * @see \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator::getDerivativeFetcher(). @@ -39,12 +53,11 @@ public function testGetDerivativeFetcher() { 'derivative' => '\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery', ); - $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); - $discovery_main->expects($this->any()) + $this->discoveryMain->expects($this->any()) ->method('getDefinitions') ->will($this->returnValue($definitions)); - $discovery = new DerivativeDiscoveryDecorator($discovery_main); + $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain); $definitions = $discovery->getDefinitions(); // Ensure that both test derivatives got added. @@ -66,12 +79,11 @@ public function testGetDerivativeFetcherWithAnnotationObjects() { 'derivative' => '\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscoveryWithObject', ); - $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); - $discovery_main->expects($this->any()) + $this->discoveryMain->expects($this->any()) ->method('getDefinitions') ->will($this->returnValue($definitions)); - $discovery = new DerivativeDiscoveryDecorator($discovery_main); + $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain); $definitions = $discovery->getDefinitions(); // Ensure that both test derivatives got added. @@ -99,12 +111,92 @@ public function testInvalidDerivativeFetcher() { 'id' => 'invalid_discovery', 'derivative' => '\Drupal\system\Tests\Plugin\DerivativeTest', ); - $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); - $discovery_main->expects($this->any()) + $this->discoveryMain->expects($this->any()) ->method('getDefinitions') ->will($this->returnValue($definitions)); - $discovery = new DerivativeDiscoveryDecorator($discovery_main); + $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain); $discovery->getDefinitions(); } + + /** + * Tests derivative definitions when a definition already exists. + */ + public function testExistingDerivative() { + $definitions = array(); + $definitions['non_container_aware_discovery'] = array( + 'id' => 'non_container_aware_discovery', + 'derivative' => '\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery', + 'string' => 'string', + 'empty_string' => 'not_empty', + 'array' => array('one', 'two'), + 'empty_array' => array('three'), + 'null_value' => TRUE, + ); + // This will clash with a derivative id. + // @see \Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery + $definitions['non_container_aware_discovery:test_discovery_1'] = array( + 'id' => 'non_container_aware_discovery:test_discovery_1', + 'string' => 'string', + 'empty_string' => '', + 'array' => array('one', 'two'), + 'empty_array' => array(), + 'null_value' => NULL, + ); + + $this->discoveryMain->expects($this->any()) + ->method('getDefinitions') + ->will($this->returnValue($definitions)); + + $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain); + $returned_definitions = $discovery->getDefinitions(); + + // If the definition was merged, there should only be two. + $this->assertCount(2, $returned_definitions); + + $expected = $definitions['non_container_aware_discovery']; + $expected['id'] = 'non_container_aware_discovery:test_discovery_1'; + $this->assertArrayEquals($expected, $returned_definitions['non_container_aware_discovery:test_discovery_1']); + } + + /** + * Tests a single definition when a derivative already exists. + */ + public function testSingleExistingDerivative() { + $base_definition = array( + 'id' => 'non_container_aware_discovery', + 'derivative' => '\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery', + 'string' => 'string', + 'empty_string' => 'not_empty', + 'array' => array('one', 'two'), + 'empty_array' => array('three'), + 'null_value' => TRUE, + ); + // This will clash with a derivative id. + // @see \Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery + $derivative_definition = array( + 'id' => 'non_container_aware_discovery:test_discovery_1', + 'string' => 'string', + 'empty_string' => '', + 'array' => array('one', 'two'), + 'empty_array' => array(), + 'null_value' => NULL, + ); + + $this->discoveryMain->expects($this->at(0)) + ->method('getDefinition') + ->with('non_container_aware_discovery:test_discovery_1') + ->will($this->returnValue($derivative_definition)); + $this->discoveryMain->expects($this->at(1)) + ->method('getDefinition') + ->with('non_container_aware_discovery') + ->will($this->returnValue($base_definition)); + + $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain); + + $expected = $base_definition; + $expected['id'] = 'non_container_aware_discovery:test_discovery_1'; + $this->assertArrayEquals($expected, $discovery->getDefinition('non_container_aware_discovery:test_discovery_1')); + } + } diff --git a/core/tests/Drupal/Tests/UnitTestCase.php b/core/tests/Drupal/Tests/UnitTestCase.php index f64dca0..898f40e 100644 --- a/core/tests/Drupal/Tests/UnitTestCase.php +++ b/core/tests/Drupal/Tests/UnitTestCase.php @@ -83,6 +83,18 @@ protected function getRandomGenerator() { return $this->randomGenerator; } + /** + * Asserts if two arrays are equal by sorting them first. + * + * @param array $expected + * @param array $actual + * @param string $message + */ + protected function assertArrayEquals(array $expected, array $actual, $message = NULL) { + ksort($expected); + ksort($actual); + $this->assertEquals($expected, $actual, $message); + } /** * Returns a stub config factory that behaves according to the passed in array.