From a351d726a6c031b2851c2b125bccb7302feddf46 Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Sun, 24 Dec 2017 10:03:14 +1000 Subject: [PATCH] Issue #2931598 by kim.pepper, markcarver, almaudoh, Sam152: Messenger methods drop repeat flag --- .../Drupal/Core/Messenger/LegacyMessenger.php | 11 +- core/lib/Drupal/Core/Messenger/Messenger.php | 6 +- .../Core/Messenger/MessengerLegacyTest.php | 108 ++++++++++ .../Core/Messenger/MessengerTest.php | 197 ++++++++++++------ 4 files changed, 257 insertions(+), 65 deletions(-) create mode 100644 core/tests/Drupal/KernelTests/Core/Messenger/MessengerLegacyTest.php diff --git a/core/lib/Drupal/Core/Messenger/LegacyMessenger.php b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php index bdc66ffe39bc..86034258315e 100644 --- a/core/lib/Drupal/Core/Messenger/LegacyMessenger.php +++ b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php @@ -34,7 +34,7 @@ class LegacyMessenger implements MessengerInterface { * {@inheritdoc} */ public function addError($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_ERROR); + return $this->addMessage($message, static::TYPE_ERROR, $repeat); } /** @@ -67,14 +67,14 @@ public function addMessage($message, $type = self::TYPE_STATUS, $repeat = FALSE) * {@inheritdoc} */ public function addStatus($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_STATUS); + return $this->addMessage($message, static::TYPE_STATUS, $repeat); } /** * {@inheritdoc} */ public function addWarning($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_WARNING); + return $this->addMessage($message, static::TYPE_WARNING, $repeat); } /** @@ -100,13 +100,16 @@ protected function getMessengerService() { if (\Drupal::hasService('messenger')) { // Note: because the container has the potential to be rebuilt during // requests, this service cannot be directly stored on this class. + /** @var \Drupal\Core\Messenger\MessengerInterface $messenger */ $messenger = \Drupal::service('messenger'); // Transfer any messages into the service. if (isset($this->messages)) { foreach ($this->messages as $type => $messages) { foreach ($messages as $message) { - $messenger->addMessage($message, $type); + // Force repeat to TRUE since this is merging existing messages to + // the Messenger service and would have already checked this prior. + $messenger->addMessage($message, $type, TRUE); } } unset($this->messages); diff --git a/core/lib/Drupal/Core/Messenger/Messenger.php b/core/lib/Drupal/Core/Messenger/Messenger.php index b8b6c3ddf369..c0949438cc56 100644 --- a/core/lib/Drupal/Core/Messenger/Messenger.php +++ b/core/lib/Drupal/Core/Messenger/Messenger.php @@ -43,7 +43,7 @@ public function __construct(FlashBagInterface $flash_bag, KillSwitch $killSwitch * {@inheritdoc} */ public function addError($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_ERROR); + return $this->addMessage($message, static::TYPE_ERROR, $repeat); } /** @@ -70,14 +70,14 @@ public function addMessage($message, $type = self::TYPE_STATUS, $repeat = FALSE) * {@inheritdoc} */ public function addStatus($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_STATUS); + return $this->addMessage($message, static::TYPE_STATUS, $repeat); } /** * {@inheritdoc} */ public function addWarning($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_WARNING); + return $this->addMessage($message, static::TYPE_WARNING, $repeat); } /** diff --git a/core/tests/Drupal/KernelTests/Core/Messenger/MessengerLegacyTest.php b/core/tests/Drupal/KernelTests/Core/Messenger/MessengerLegacyTest.php new file mode 100644 index 000000000000..29334647db17 --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Messenger/MessengerLegacyTest.php @@ -0,0 +1,108 @@ +<?php + +namespace Drupal\KernelTests\Core\Messenger; + +use Drupal\Core\Messenger\LegacyMessenger; +use Drupal\Core\Messenger\Messenger; +use Drupal\Core\Messenger\MessengerInterface; +use Drupal\KernelTests\KernelTestBase; + +/** + * @group Messenger + * @coversDefaultClass \Drupal\Core\Messenger\LegacyMessenger + * + * Note: The Symphony PHPUnit Bridge automatically treats any test class that + * starts with "Legacy" as a deprecation. To subvert that, reverse it here. + * + * @see http://symfony.com/blog/new-in-symfony-2-7-phpunit-bridge + * @see https://www.drupal.org/node/2931598#comment-12395743 + */ +class MessengerLegacyTest extends KernelTestBase { + + /** + * Retrieves the Messenger service from LegacyMessenger. + * + * @param \Drupal\Core\Messenger\LegacyMessenger $legacy_messenger + * The legacy messenger. + * + * @return \Drupal\Core\Messenger\MessengerInterface|null + * A messenger implementation. + */ + protected function getMessengerService(LegacyMessenger $legacy_messenger) { + $method = new \ReflectionMethod($legacy_messenger, 'getMessengerService'); + $method->setAccessible(TRUE); + return $method->invoke($legacy_messenger); + } + + /** + * @covers \Drupal::messenger + * @covers ::getMessengerService + * @covers ::all + * @covers ::addMessage + * @covers ::addError + * @covers ::addStatus + * @covers ::addWarning + */ + public function testMessages() { + // Save the current container for later use. + $container = \Drupal::getContainer(); + + // Unset the container to mimic not having one. + \Drupal::unsetContainer(); + + /** @var \Drupal\Core\Messenger\LegacyMessenger $messenger */ + // Verify that the Messenger service doesn't exists. + $messenger = \Drupal::messenger(); + $this->assertNull($this->getMessengerService($messenger)); + + // Add messages. + $messenger->addMessage('Foobar', 'custom'); + $messenger->addMessage('Foobar', 'custom', TRUE); + $messenger->addError('Foo'); + $messenger->addError('Foo', TRUE); + + // Verify that retrieving another instance and adding more messages works. + $messenger = \Drupal::messenger(); + $messenger->addStatus('Bar'); + $messenger->addStatus('Bar', TRUE); + $messenger->addWarning('Fiz'); + $messenger->addWarning('Fiz', TRUE); + + // Restore the container. + \Drupal::setContainer($container); + + // Verify that the Messenger service exists. + $messenger = \Drupal::messenger(); + $this->assertInstanceOf(Messenger::class, $this->getMessengerService($messenger)); + + // Add more messages. + $messenger->addMessage('Platypus', 'custom'); + $messenger->addMessage('Platypus', 'custom', TRUE); + $messenger->addError('Rhinoceros'); + $messenger->addError('Rhinoceros', TRUE); + $messenger->addStatus('Giraffe'); + $messenger->addStatus('Giraffe', TRUE); + $messenger->addWarning('Cheetah'); + $messenger->addWarning('Cheetah', TRUE); + + // Verify all messages added via LegacyMessenger are accounted for. + $messages = $messenger->all(); + $this->assertContains('Foobar', $messages['custom']); + $this->assertContains('Foo', $messages[MessengerInterface::TYPE_ERROR]); + $this->assertContains('Bar', $messages[MessengerInterface::TYPE_STATUS]); + $this->assertContains('Fiz', $messages[MessengerInterface::TYPE_WARNING]); + + // Verify all messages added via Messenger service are accounted for. + $this->assertContains('Platypus', $messages['custom']); + $this->assertContains('Rhinoceros', $messages[MessengerInterface::TYPE_ERROR]); + $this->assertContains('Giraffe', $messages[MessengerInterface::TYPE_STATUS]); + $this->assertContains('Cheetah', $messages[MessengerInterface::TYPE_WARNING]); + + // Verify repeat counts. + $this->assertCount(4, $messages['custom']); + $this->assertCount(4, $messages[MessengerInterface::TYPE_STATUS]); + $this->assertCount(4, $messages[MessengerInterface::TYPE_WARNING]); + $this->assertCount(4, $messages[MessengerInterface::TYPE_ERROR]); + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/Messenger/MessengerTest.php b/core/tests/Drupal/KernelTests/Core/Messenger/MessengerTest.php index 362069756eae..32c4743d93d4 100644 --- a/core/tests/Drupal/KernelTests/Core/Messenger/MessengerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Messenger/MessengerTest.php @@ -2,83 +2,164 @@ namespace Drupal\KernelTests\Core\Messenger; -use Drupal\Core\Messenger\LegacyMessenger; -use Drupal\Core\Messenger\Messenger; use Drupal\Core\Messenger\MessengerInterface; +use Drupal\Core\Render\Markup; +use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\KernelTests\KernelTestBase; /** * @group Messenger - * @coversDefaultClass \Drupal\Core\Messenger\LegacyMessenger + * @coversDefaultClass \Drupal\Core\Messenger\Messenger */ class MessengerTest extends KernelTestBase { /** - * Retrieves the Messenger service from LegacyMessenger. + * The messenger under test. * - * @param \Drupal\Core\Messenger\LegacyMessenger $legacy_messenger - * - * @return \Drupal\Core\Messenger\MessengerInterface|null + * @var \Drupal\Core\Messenger\MessengerInterface + */ + protected $messenger; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->messenger = \Drupal::service('messenger'); + } + + /** + * @covers ::addStatus + * @covers ::deleteByType + * @covers ::messagesByType */ - protected function getMessengerService(LegacyMessenger $legacy_messenger) { - $method = new \ReflectionMethod($legacy_messenger, 'getMessengerService'); - $method->setAccessible(TRUE); - return $method->invoke($legacy_messenger); + public function testRemoveSingleMessage() { + + // Set two messages. + $this->messenger->addStatus('First message (removed).'); + $this->messenger->addStatus(t('Second message with <em>markup!</em> (not removed).')); + $messages = $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS); + // Remove the first. + unset($messages[0]); + + // Re-add the second. + foreach ($messages as $message) { + $this->messenger->addStatus($message); + } + + // Check we only have the second one. + $this->assertCount(1, $this->messenger->messagesByType(MessengerInterface::TYPE_STATUS)); + $this->assertContains('Second message with <em>markup!</em> (not removed).', $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS)); + } /** - * @covers \Drupal::messenger - * @covers ::getMessengerService + * Tests we don't add duplicates. + * * @covers ::all - * @covers ::addMessage + * @covers ::addStatus + * @covers ::addWarning * @covers ::addError + * @covers ::deleteByType + * @covers ::deleteAll + */ + public function testAddNoDuplicates() { + + $this->messenger->addStatus('Non Duplicated status message'); + $this->messenger->addStatus('Non Duplicated status message'); + + $this->assertCount(1, $this->messenger->messagesByType(MessengerInterface::TYPE_STATUS)); + + $this->messenger->addWarning('Non Duplicated warning message'); + $this->messenger->addWarning('Non Duplicated warning message'); + + $this->assertCount(1, $this->messenger->messagesByType(MessengerInterface::TYPE_WARNING)); + + $this->messenger->addError('Non Duplicated error message'); + $this->messenger->addError('Non Duplicated error message'); + + $messages = $this->messenger->messagesByType(MessengerInterface::TYPE_ERROR); + $this->assertCount(1, $messages); + + // Check getting all messages. + $messages = $this->messenger->all(); + $this->assertCount(3, $messages); + $this->assertArrayHasKey(MessengerInterface::TYPE_STATUS, $messages); + $this->assertArrayHasKey(MessengerInterface::TYPE_WARNING, $messages); + $this->assertArrayHasKey(MessengerInterface::TYPE_ERROR, $messages); + + // Check deletion. + $this->messenger->deleteAll(); + $this->assertCount(0, $this->messenger->messagesByType(MessengerInterface::TYPE_STATUS)); + $this->assertCount(0, $this->messenger->messagesByType(MessengerInterface::TYPE_WARNING)); + $this->assertCount(0, $this->messenger->messagesByType(MessengerInterface::TYPE_ERROR)); + + } + + /** + * Tests we do add duplicates with repeat flag. + * * @covers ::addStatus * @covers ::addWarning + * @covers ::addError + * @covers ::deleteByType */ - public function testMessages() { - // Save the current container for later use. - $container = \Drupal::getContainer(); - - // Unset the container to mimic not having one. - \Drupal::unsetContainer(); - - /** @var \Drupal\Core\Messenger\LegacyMessenger $messenger */ - // Verify that the Messenger service doesn't exists. - $messenger = \Drupal::messenger(); - $this->assertNull($this->getMessengerService($messenger)); - - // Add messages. - $messenger->addMessage('Foobar'); - $messenger->addError('Foo'); - - // Verify that retrieving another instance and adding more messages works. - $messenger = \Drupal::messenger(); - $messenger->addStatus('Bar'); - $messenger->addWarning('Fiz'); - - // Restore the container. - \Drupal::setContainer($container); - - // Verify that the Messenger service exists. - $messenger = \Drupal::messenger(); - $this->assertInstanceOf(Messenger::class, $this->getMessengerService($messenger)); - - // Add more messages. - $messenger->addMessage('Platypus'); - $messenger->addError('Rhinoceros'); - $messenger->addStatus('Giraffe'); - $messenger->addWarning('Cheetah'); - - // Verify that all the messages are present and accounted for. - $messages = $messenger->all(); - $this->assertContains('Foobar', $messages[MessengerInterface::TYPE_STATUS]); - $this->assertContains('Foo', $messages[MessengerInterface::TYPE_ERROR]); - $this->assertContains('Bar', $messages[MessengerInterface::TYPE_STATUS]); - $this->assertContains('Fiz', $messages[MessengerInterface::TYPE_WARNING]); - $this->assertContains('Platypus', $messages[MessengerInterface::TYPE_STATUS]); - $this->assertContains('Rhinoceros', $messages[MessengerInterface::TYPE_ERROR]); - $this->assertContains('Giraffe', $messages[MessengerInterface::TYPE_STATUS]); - $this->assertContains('Cheetah', $messages[MessengerInterface::TYPE_WARNING]); + public function testAddWithDuplicates() { + + $this->messenger->addStatus('Duplicated status message', TRUE); + $this->messenger->addStatus('Duplicated status message', TRUE); + + $this->assertCount(2, $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS)); + + $this->messenger->addWarning('Duplicated warning message', TRUE); + $this->messenger->addWarning('Duplicated warning message', TRUE); + + $this->assertCount(2, $this->messenger->deleteByType(MessengerInterface::TYPE_WARNING)); + + $this->messenger->addError('Duplicated error message', TRUE); + $this->messenger->addError('Duplicated error message', TRUE); + + $this->assertCount(2, $this->messenger->deleteByType(MessengerInterface::TYPE_ERROR)); + + } + + /** + * Test adding markup. + * + * @covers ::addStatus + * @covers ::deleteByType + * @covers ::messagesByType + */ + public function testAddMarkup() { + + // Add a Markup message. + $this->messenger->addStatus(Markup::create('Markup with <em>markup!</em>')); + // Test duplicate Markup messages. + $this->messenger->addStatus(Markup::create('Markup with <em>markup!</em>')); + + $this->assertCount(1, $this->messenger->messagesByType(MessengerInterface::TYPE_STATUS)); + + // Ensure that multiple Markup messages work. + $this->messenger->addStatus(Markup::create('Markup2 with <em>markup!</em>')); + + $this->assertCount(2, $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS)); + + // Test mixing of types. + $this->messenger->addStatus(Markup::create('Non duplicate Markup / string.')); + $this->messenger->addStatus('Non duplicate Markup / string.'); + $this->messenger->addStatus(Markup::create('Duplicate Markup / string.'), TRUE); + $this->messenger->addStatus('Duplicate Markup / string.', TRUE); + + $this->assertCount(3, $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS)); + + $this->messenger->deleteAll(); + + // Check translatable string is converted to Markup. + $this->messenger->addStatus(new TranslatableMarkup('Translatable message')); + $messages = $this->messenger->deleteByType(MessengerInterface::TYPE_STATUS); + + $this->assertInstanceOf(Markup::class, $messages[0]); + } } -- GitLab