From 0ba3fe49d0c0eee94599d695935999b7970e558a Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Mon, 4 Dec 2017 14:14:53 -0600 Subject: [PATCH] Issue #2760167 by kim.pepper, markcarver, jibran, dawehner, znerol, Wim Leers: Add \Drupal\Core\Messenger\Messenger --- core/core.services.yml | 4 +- core/includes/bootstrap.inc | 23 +- core/lib/Drupal.php | 25 +- core/lib/Drupal/Core/Messenger/LegacyMessenger.php | 251 +++++++++------------ core/lib/Drupal/Core/Messenger/Messenger.php | 112 +++++++++ .../Drupal/Core/Messenger/MessengerInterface.php | 9 +- .../src/Controller/SystemTestController.php | 24 +- .../Core/Common/DrupalSetMessageTest.php | 6 - .../Core/Messenger/LegacyMessengerTest.php | 70 ++++++ .../Drupal/Tests/Listeners/DeprecationListener.php | 3 + 10 files changed, 362 insertions(+), 165 deletions(-) create mode 100644 core/lib/Drupal/Core/Messenger/Messenger.php create mode 100644 core/tests/Drupal/KernelTests/Core/Messenger/LegacyMessengerTest.php diff --git a/core/core.services.yml b/core/core.services.yml index 459503e44c..49b27089b5 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1646,5 +1646,5 @@ services: tags: - { name: event_subscriber } messenger: - class: Drupal\Core\Messenger\LegacyMessenger - arguments: ['@page_cache_kill_switch'] + class: Drupal\Core\Messenger\Messenger + arguments: ['@session.flash_bag', '@page_cache_kill_switch'] diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index d05298d4bf..ae2f8915c3 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -466,11 +466,17 @@ function watchdog_exception($type, Exception $exception, $message = NULL, $varia * * @see drupal_get_messages() * @see status-messages.html.twig + * @see https://www.drupal.org/node/2774931 + * + * @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. + * Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. */ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE) { - /* @var \Drupal\Core\Messenger\MessengerInterface $messenger */ - $messenger = \Drupal::service('messenger'); - $messenger->addMessage($message, $type, $repeat); + @trigger_error('drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931', E_USER_DEPRECATED); + $messenger = \Drupal::messenger(); + if (isset($message)) { + $messenger->addMessage($message, $type, $repeat); + } return $messenger->all(); } @@ -498,11 +504,16 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE) * * @see drupal_set_message() * @see status-messages.html.twig + * @see https://www.drupal.org/node/2774931 + * + * @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. + * Use \Drupal\Core\Messenger\MessengerInterface::all() or + * \Drupal\Core\Messenger\MessengerInterface::messagesByType() instead. */ function drupal_get_messages($type = NULL, $clear_queue = TRUE) { - /** @var \Drupal\Core\Messenger\MessengerInterface $messenger */ - $messenger = \Drupal::hasService('messenger') ? \Drupal::service('messenger') : NULL; - if ($messenger && ($messages = $messenger->all())) { + @trigger_error('drupal_get_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::all() or \Drupal\Core\Messenger\MessengerInterface::messagesByType() instead. See https://www.drupal.org/node/2774931', E_USER_DEPRECATED); + $messenger = \Drupal::messenger(); + if ($messages = $messenger->all()) { if ($type) { if ($clear_queue) { $messenger->deleteByType($type); diff --git a/core/lib/Drupal.php b/core/lib/Drupal.php index 07dc12f1f3..e95e80f558 100644 --- a/core/lib/Drupal.php +++ b/core/lib/Drupal.php @@ -6,8 +6,9 @@ */ use Drupal\Core\DependencyInjection\ContainerNotInitializedException; -use Symfony\Component\DependencyInjection\ContainerInterface; +use Drupal\Core\Messenger\LegacyMessenger; use Drupal\Core\Url; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Static Service Container wrapper. @@ -100,6 +101,13 @@ class Drupal { */ protected static $container; + /** + * The currently active container object, or NULL if not initialized yet. + * + * @var \Drupal\Core\Messenger\LegacyMessenger|null + */ + protected static $legacyMessenger; + /** * Sets a new global container. * @@ -757,4 +765,19 @@ public static function time() { return static::getContainer()->get('datetime.time'); } + /** + * Returns the messenger. + * + * @return \Drupal\Core\Messenger\MessengerInterface + * The messenger. + */ + public static function messenger() { + // @todo Replace with service once LegacyMessenger is removed in 9.0.0. + // @see https://www.drupal.org/node/2928994 + if (!isset(static::$legacyMessenger)) { + static::$legacyMessenger = new LegacyMessenger(); + } + return static::$legacyMessenger; + } + } diff --git a/core/lib/Drupal/Core/Messenger/LegacyMessenger.php b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php index 8c9751f3d8..5e213e9235 100644 --- a/core/lib/Drupal/Core/Messenger/LegacyMessenger.php +++ b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php @@ -3,38 +3,64 @@ namespace Drupal\Core\Messenger; use Drupal\Component\Render\MarkupInterface; -use Drupal\Core\PageCache\ResponsePolicy\KillSwitch; use Drupal\Core\Render\Markup; /** - * A legacy implementation of the messenger interface. + * Provides a LegacyMessenger implementation. * - * @internal + * This implementation is for handling messages in a backwards compatible way + * using core's previous $_SESSION storage method. + * + * You should not instantiate a new instance of this class directly. Instead, + * you should inject the "messenger" service into your own services or use + * \Drupal::messenger() in procedural functions. + * + * @see https://www.drupal.org/node/2774931 + * @see https://www.drupal.org/node/2928994 + * + * @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. + * Use \Drupal\Core\Messenger\Messenger instead. */ class LegacyMessenger implements MessengerInterface { /** - * The page cache kill switch. + * The messages. * - * @var \Drupal\Core\PageCache\ResponsePolicy\KillSwitch + * @var array */ - protected $killSwitch; + protected $messages; /** - * LegacyMessenger constructor. - * - * @param \Drupal\Core\PageCache\ResponsePolicy\KillSwitch $killSwitch - * (optional) The page cache kill switch. + * {@inheritdoc} */ - public function __construct(KillSwitch $killSwitch) { - $this->killSwitch = $killSwitch; + public function addError($message, $repeat = FALSE) { + return $this->addMessage($message, static::TYPE_ERROR); } /** * {@inheritdoc} */ public function addMessage($message, $type = self::TYPE_STATUS, $repeat = FALSE) { - $this->setMessage($message, $type, $repeat); + // Proxy to the Messenger service, if it exists. + if ($messenger = $this->getMessenger()) { + return $messenger->addMessage($message, $type, $repeat); + } + + if (!isset($this->messages[$type])) { + $this->messages[$type] = []; + } + + if (!($message instanceof Markup) && $message instanceof MarkupInterface) { + $message = Markup::create((string) $message); + } + + // Do not use strict type checking so that equivalent string and + // MarkupInterface objects are detected. + if ($repeat || !in_array($message, $this->messages[$type])) { + $this->messages[$type][] = $message; + } + + return $this; } /** @@ -44,13 +70,6 @@ public function addStatus($message, $repeat = FALSE) { return $this->addMessage($message, static::TYPE_STATUS); } - /** - * {@inheritdoc} - */ - public function addError($message, $repeat = FALSE) { - return $this->addMessage($message, static::TYPE_ERROR); - } - /** * {@inheritdoc} */ @@ -62,151 +81,95 @@ public function addWarning($message, $repeat = FALSE) { * {@inheritdoc} */ public function all() { - return $this->getMessages(NULL, FALSE); - } + // Proxy to the Messenger service, if it exists. + if ($messenger = $this->getMessenger()) { + return $messenger->all(); + } - /** - * {@inheritdoc} - */ - public function messagesByType($type) { - return $this->getMessages($type, FALSE); + return $this->messages; } /** - * {@inheritdoc} + * Returns the Messenger service. + * + * @return \Drupal\Core\Messenger\MessengerInterface|null + * The Messenger service. */ - public function deleteAll() { - return $this->getMessages(NULL, TRUE); + public function getMessenger() { + // Use the Messenger service, if it exists. + 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. + $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); + } + } + $this->messages = []; + } + + return $messenger; + } + + // Otherwise, trigger an error. + @trigger_error('Adding or retrieving messages prior to the container being initialized was deprecated in Drupal 8.5.0 and this functionality will be removed before Drupal 9.0.0. Please report this usage at https://www.drupal.org/node/2928994.', E_USER_DEPRECATED); + + // Prematurely creating $_SESSION['messages'] in this class' constructor + // causes issues when the container attempts to initialize its own session + // later down the road. This can only be done after it has been determined + // the normal Messenger service is not available (i.e. no container) which + // is reasonable to assume that a new instance of this class will be + // created once the container becomes available. + if (!isset($_SESSION['messages'])) { + $_SESSION['messages'] = []; + } + + // Reference the previous method core used to store messages. + $this->messages = &$_SESSION['messages']; } /** * {@inheritdoc} */ - public function deleteByType($type) { - return $this->getMessages($type, TRUE); + public function messagesByType($type) { + // Proxy to the Messenger service, if it exists. + if ($messenger = $this->getMessenger()) { + return $messenger->messagesByType($type); + } + + return $this->messages[$type]; } /** - * Sets a message to display to the user. - * - * Messages are stored in a session variable and displayed in the page template - * via the $messages theme variable. - * - * Example usage: - * @code - * drupal_set_message(t('An error occurred and processing did not complete.'), 'error'); - * @endcode - * - * @param string|\Drupal\Component\Render\MarkupInterface $message - * (optional) The translated message to be displayed to the user. For - * consistency with other messages, it should begin with a capital letter and - * end with a period. - * @param string $type - * (optional) The message's type. Defaults to 'status'. These values are - * supported: - * - 'status' - * - 'warning' - * - 'error' - * @param bool $repeat - * (optional) If this is FALSE and the message is already set, then the - * message won't be repeated. Defaults to FALSE. - * - * @return array|null - * A multidimensional array with keys corresponding to the set message types. - * The indexed array values of each contain the set messages for that type, - * and each message is an associative array with the following format: - * - safe: Boolean indicating whether the message string has been marked as - * safe. Non-safe strings will be escaped automatically. - * - message: The message string. - * So, the following is an example of the full return array structure: - * @code - * array( - * 'status' => array( - * array( - * 'safe' => TRUE, - * 'message' => 'A safe markup string.', - * ), - * array( - * 'safe' => FALSE, - * 'message' => "$arbitrary_user_input to escape.", - * ), - * ), - * ); - * @endcode - * If there are no messages set, the function returns NULL. - * - * @internal + * {@inheritdoc} */ - private function setMessage($message = NULL, $type = 'status', $repeat = FALSE) { - if (isset($message)) { - if (!isset($_SESSION['messages'][$type])) { - $_SESSION['messages'][$type] = []; - } - - // Convert strings which are safe to the simplest Markup objects. - if (!($message instanceof Markup) && $message instanceof MarkupInterface) { - $message = Markup::create((string) $message); - } - - // Do not use strict type checking so that equivalent string and - // MarkupInterface objects are detected. - if ($repeat || !in_array($message, $_SESSION['messages'][$type])) { - $_SESSION['messages'][$type][] = $message; - } - - // Mark this page as being uncacheable. - $this->killSwitch->trigger(); + public function deleteAll() { + // Proxy to the Messenger service, if it exists. + if ($messenger = $this->getMessenger()) { + return $messenger->deleteAll(); } - // Messages not set when DB connection fails. - return isset($_SESSION['messages']) ? $_SESSION['messages'] : NULL; + $messages = $this->messages; + $this->messages = []; + return $messages; } /** - * Returns all messages that have been set with drupal_set_message(). - * - * @param string $type - * (optional) Limit the messages returned by type. Defaults to NULL, meaning - * all types. These values are supported: - * - NULL - * - 'status' - * - 'warning' - * - 'error' - * @param bool $clear_queue - * (optional) If this is TRUE, the queue will be cleared of messages of the - * type specified in the $type parameter. Otherwise the queue will be left - * intact. Defaults to TRUE. - * - * @return array - * An associative, nested array of messages grouped by message type, with - * the top-level keys as the message type. The messages returned are - * limited to the type specified in the $type parameter, if any. If there - * are no messages of the specified type, an empty array is returned. See - * drupal_set_message() for the array structure of individual messages. - * - * @see drupal_set_message() - * @see status-messages.html.twig - * - * @internal + * {@inheritdoc} */ - private function getMessages($type = NULL, $clear_queue = TRUE) { - if ($messages = $this->setMessage()) { - if ($type) { - if ($clear_queue) { - unset($_SESSION['messages'][$type]); - } - if (isset($messages[$type])) { - return [$type => $messages[$type]]; - } - } - else { - if ($clear_queue) { - unset($_SESSION['messages']); - } - return $messages; - } + public function deleteByType($type) { + // Proxy to the Messenger service, if it exists. + if ($messenger = $this->getMessenger()) { + return $messenger->messagesByType($type); } - return []; + + $messages = $this->messages[$type]; + $this->messages[$type] = []; + return $messages; } } diff --git a/core/lib/Drupal/Core/Messenger/Messenger.php b/core/lib/Drupal/Core/Messenger/Messenger.php new file mode 100644 index 0000000000..b8b6c3ddf3 --- /dev/null +++ b/core/lib/Drupal/Core/Messenger/Messenger.php @@ -0,0 +1,112 @@ +flashBag = $flash_bag; + $this->killSwitch = $killSwitch; + } + + /** + * {@inheritdoc} + */ + public function addError($message, $repeat = FALSE) { + return $this->addMessage($message, static::TYPE_ERROR); + } + + /** + * {@inheritdoc} + */ + public function addMessage($message, $type = self::TYPE_STATUS, $repeat = FALSE) { + if (!($message instanceof Markup) && $message instanceof MarkupInterface) { + $message = Markup::create((string) $message); + } + + // Do not use strict type checking so that equivalent string and + // MarkupInterface objects are detected. + if ($repeat || !in_array($message, $this->flashBag->peek($type))) { + $this->flashBag->add($type, $message); + } + + // Mark this page as being uncacheable. + $this->killSwitch->trigger(); + + return $this; + } + + /** + * {@inheritdoc} + */ + public function addStatus($message, $repeat = FALSE) { + return $this->addMessage($message, static::TYPE_STATUS); + } + + /** + * {@inheritdoc} + */ + public function addWarning($message, $repeat = FALSE) { + return $this->addMessage($message, static::TYPE_WARNING); + } + + /** + * {@inheritdoc} + */ + public function all() { + return $this->flashBag->peekAll(); + } + + /** + * {@inheritdoc} + */ + public function deleteAll() { + return $this->flashBag->clear(); + } + + /** + * {@inheritdoc} + */ + public function deleteByType($type) { + // Flash bag gets and clears flash messages from the stack. + return $this->flashBag->get($type); + } + + /** + * {@inheritdoc} + */ + public function messagesByType($type) { + return $this->flashBag->peek($type); + } + +} diff --git a/core/lib/Drupal/Core/Messenger/MessengerInterface.php b/core/lib/Drupal/Core/Messenger/MessengerInterface.php index 216835bcea..441ea6fc22 100644 --- a/core/lib/Drupal/Core/Messenger/MessengerInterface.php +++ b/core/lib/Drupal/Core/Messenger/MessengerInterface.php @@ -6,8 +6,6 @@ * Stores runtime messages sent out to individual users on the page. * * An example for these messages is for example: "Content X got saved". - * - * @internal */ interface MessengerInterface { @@ -109,11 +107,15 @@ public function all(); * or self::TYPE_ERROR. * * @return string[]|\Drupal\Component\Render\MarkupInterface[] + * The messages of given type. */ public function messagesByType($type); /** * Deletes all messages. + * + * @return string[]|\Drupal\Component\Render\MarkupInterface[] + * The deleted messages. */ public function deleteAll(); @@ -123,6 +125,9 @@ public function deleteAll(); * @param string $type * The messages' type. Either self::TYPE_STATUS, self::TYPE_WARNING, or * self::TYPE_ERROR. + * + * @return string[]|\Drupal\Component\Render\MarkupInterface[] + * The deleted messages of given type.. */ public function deleteByType($type); diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php index 37eb87585d..e350bc2eaf 100644 --- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php @@ -5,6 +5,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Render\Markup; use Drupal\Core\Session\AccountInterface; @@ -48,6 +49,13 @@ class SystemTestController extends ControllerBase { */ protected $renderer; + /** + * The messenger service. + * + * @var \Drupal\Core\Messenger\MessengerInterface + */ + protected $messenger; + /** * Constructs the SystemTestController. * @@ -59,12 +67,15 @@ class SystemTestController extends ControllerBase { * The current user. * @param \Drupal\Core\Render\RendererInterface $renderer * The renderer. + * @param \Drupal\Core\Messenger\MessengerInterface $messenger + * The messenger service. */ - public function __construct(LockBackendInterface $lock, LockBackendInterface $persistent_lock, AccountInterface $current_user, RendererInterface $renderer) { + public function __construct(LockBackendInterface $lock, LockBackendInterface $persistent_lock, AccountInterface $current_user, RendererInterface $renderer, MessengerInterface $messenger) { $this->lock = $lock; $this->persistentLock = $persistent_lock; $this->currentUser = $current_user; $this->renderer = $renderer; + $this->messenger = $messenger; } /** @@ -75,7 +86,8 @@ public static function create(ContainerInterface $container) { $container->get('lock'), $container->get('lock.persistent'), $container->get('current_user'), - $container->get('renderer') + $container->get('renderer'), + $container->get('messenger') ); } @@ -99,9 +111,13 @@ public function drupalSetMessageTest() { // Set two messages. drupal_set_message('First message (removed).'); drupal_set_message(t('Second message with markup! (not removed).')); - + $messages = $this->messenger->deleteByType('status'); // Remove the first. - unset($_SESSION['messages']['status'][0]); + unset($messages[0]); + + foreach ($messages as $message) { + $this->messenger->addStatus($message); + } // Duplicate message check. drupal_set_message('Non Duplicated message', 'status', FALSE); diff --git a/core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php b/core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php index 59470e6563..7a15fc3e04 100644 --- a/core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php +++ b/core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php @@ -20,10 +20,4 @@ public function testDrupalSetMessage() { $this->assertEquals('A message: bar', (string) $messages['status'][0]); } - protected function tearDown() { - // Clear session to prevent global leakage. - unset($_SESSION['messages']); - parent::tearDown(); - } - } diff --git a/core/tests/Drupal/KernelTests/Core/Messenger/LegacyMessengerTest.php b/core/tests/Drupal/KernelTests/Core/Messenger/LegacyMessengerTest.php new file mode 100644 index 0000000000..168098cc9a --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Messenger/LegacyMessengerTest.php @@ -0,0 +1,70 @@ +assertNull($messenger->getMessenger()); + + // 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, $messenger->getMessenger()); + + // 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]); + } + +} diff --git a/core/tests/Drupal/Tests/Listeners/DeprecationListener.php b/core/tests/Drupal/Tests/Listeners/DeprecationListener.php index 80b3d31c97..74a0ab6c64 100644 --- a/core/tests/Drupal/Tests/Listeners/DeprecationListener.php +++ b/core/tests/Drupal/Tests/Listeners/DeprecationListener.php @@ -113,6 +113,9 @@ public static function getSkippedDeprecations() { 'Automatically creating the first item for computed fields is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. Use \Drupal\Core\TypedData\ComputedItemListTrait instead.', '"\Drupal\Core\Entity\ContentEntityStorageBase::doLoadRevisionFieldItems()" is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. "\Drupal\Core\Entity\ContentEntityStorageBase::doLoadMultipleRevisionsFieldItems()" should be implemented instead. See https://www.drupal.org/node/2924915.', 'Passing a single revision ID to "\Drupal\Core\Entity\Sql\SqlContentEntityStorage::buildQuery()" is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. An array of revision IDs should be given instead. See https://www.drupal.org/node/2924915.', + 'drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931', + 'drupal_get_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::all() or \Drupal\Core\Messenger\MessengerInterface::messagesByType() instead. See https://www.drupal.org/node/2774931', + 'Adding or retrieving messages prior to the container being initialized was deprecated in Drupal 8.5.0 and this functionality will be removed before Drupal 9.0.0. Please report this usage at https://www.drupal.org/node/2928994.', ]; } -- 2.14.1