Issue #2426805 by dawehner, martin107: Modernize drupal_get_destination()

core/core.services.yml

@@ -587,6 +587,9 @@ services:
     arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@logger.channel.default', '@request_stack']
     calls:
       - [setContext, ['@?router.request_context']]
+  redirect.destination:
+    class: Drupal\Core\Routing\RedirectDestination
+    arguments: ['@request_stack', '@url_generator']
   unrouted_url_assembler:
     class: Drupal\Core\Utility\UnroutedUrlAssembler
     arguments: ['@request_stack', '@config.factory', '@path_processor_manager']

core/includes/common.inc

@@ -219,28 +219,14 @@ function drupal_get_html_head($render = TRUE) {
  *     request is returned.
  *
  * @see \Drupal\Core\EventSubscriber\RedirectResponseSubscriber::checkRedirectUrl()
+ *
  * @ingroup form_api
+ *
+ * @deprecated in Drupal 8.0.x-dev, will be removed before Drupal 9.0.0.
+ *   Use the redirect.destination service.
  */
 function drupal_get_destination() {
-  $destination = &drupal_static(__FUNCTION__);
-
-  if (isset($destination)) {
-    return $destination;
-  }
-
-  $query = \Drupal::request()->query;
-  if ($query->has('destination')) {
-    $destination = array('destination' => $query->get('destination'));
-  }
-  else {
-    $path = \Drupal::url('<current>');
-    $query = UrlHelper::buildQuery(UrlHelper::filterQueryParameters($query->all()));
-    if ($query != '') {
-      $path .= '?' . $query;
-    }
-    $destination = array('destination' => $path);
-  }
-  return $destination;
+  return \Drupal::destination()->getAsArray();
 }
 
 /**

core/lib/Drupal.php

@@ -667,4 +667,14 @@ public static function accessManager() {
     return static::getContainer()->get('access_manager');
   }
 
+  /**
+   * Returns the redirect destination helper.
+   *
+   * @return \Drupal\Core\Routing\RedirectDestinationInterface
+   *   The redirect destination helper.
+   */
+  public static function destination() {
+    return static::getContainer()->get('redirect.destination');
+  }
+
 }

core/lib/Drupal/Core/Routing/RedirectDestination.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\RedirectDestination.
+ */
+
+namespace Drupal\Core\Routing;
+
+use Drupal\Component\Utility\UrlHelper;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * Provides helpers for redirect destinations.
+ */
+class RedirectDestination implements RedirectDestinationInterface {
+
+  /**
+   * The request stack.
+   *
+   * @var \Symfony\Component\HttpFoundation\RequestStack
+   */
+  protected $requestStack;
+
+  /**
+   * The URL generator.
+   *
+   * @var \Drupal\Core\Routing\UrlGeneratorInterface
+   */
+  protected $urlGenerator;
+
+  /**
+   * The destination used by the current request.
+   *
+   * @var string
+   */
+  protected $destination;
+
+  /**
+   * Constructs a new RedirectDestination instance.
+   *
+   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
+   *   The request stack.
+   * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator
+   *   The URL generator.
+   */
+  public function __construct(RequestStack $request_stack, UrlGeneratorInterface $url_generator) {
+    $this->requestStack = $request_stack;
+    $this->urlGenerator = $url_generator;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getAsArray() {
+    return ['destination' => $this->get()];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function get() {
+    if (!isset($this->destination)) {
+      $query = $this->requestStack->getCurrentRequest()->query;
+      if ($query->has('destination')) {
+        $this->destination = $query->get('destination');
+      }
+      else {
+        $this->destination = $this->urlGenerator->generateFromRoute('<current>', [], ['query' => UrlHelper::buildQuery(UrlHelper::filterQueryParameters($query->all()))]);
+      }
+    }
+
+    return $this->destination;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function set($new_destination) {
+    $this->destination = $new_destination;
+    return $this;
+  }
+
+}

core/lib/Drupal/Core/Routing/RedirectDestinationInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\RedirectDestinationInterface.
+ */
+
+namespace Drupal\Core\Routing;
+
+/**
+ * Provides an interface for redirect destinations.
+ */
+interface RedirectDestinationInterface {
+
+  /**
+   * Prepares a 'destination' URL query parameter for use with \Drupal\Core\Url.
+   *
+   * Used to direct the user back to the referring page after completing a form.
+   * By default the current URL is returned. If a destination exists in the
+   * current request, that destination is returned. As such, a destination can
+   * persist across multiple pages.
+   *
+   * @return array
+   *   An associative array containing the key:
+   *   - destination: The value of the current request's 'destination' query
+   *     parameter, if present. This can be either a relative or absolute URL.
+   *     However, for security, redirection to external URLs is not performed.
+   *     If the query parameter isn't present, then the URL of the current
+   *     request is returned.
+   *
+   * @see \Drupal\Core\EventSubscriber\RedirectResponseSubscriber::checkRedirectUrl()
+   * @ingroup form_api
+   */
+  public function getAsArray();
+
+  /**
+   * Gets the destination as URL.
+   *
+   * @return string
+   */
+  public function get();
+
+  /**
+   * Sets the destination as URL.
+   *
+   * This method should be used really rarely, for example views uses it, in
+   * order to override all destination calls in all of its rendering.
+   *
+   * @param string $new_destination
+   *   The new destination.
+   *
+   * @return $this
+   */
+  public function set($new_destination);
+
+}

core/modules/menu_ui/src/Plugin/Menu/LocalAction/MenuLinkAdd.php

@@ -8,21 +8,63 @@
 namespace Drupal\menu_ui\Plugin\Menu\LocalAction;
 
 use Drupal\Core\Menu\LocalActionDefault;
+use Drupal\Core\Routing\RedirectDestination;
 use Drupal\Core\Routing\RouteMatchInterface;
-use Drupal\Core\Url;
+use Drupal\Core\Routing\RouteProviderInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Modifies the 'Add link' local action to add a destination.
  */
 class MenuLinkAdd extends LocalActionDefault {
 
+  /**
+   * The redirect destination.
+   *
+   * @var \Drupal\Core\Routing\RedirectDestination
+   */
+  private $redirectDestination;
+
+  /**
+   * Constructs a MenuLinkAdd object.
+   *
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param mixed $plugin_definition
+   *   The plugin implementation definition.
+   * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider
+   *   The route provider to load routes by name.
+   * @param \Drupal\Core\Routing\RedirectDestination $redirect_destination
+   *   The redirect destination.
+   */
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, RedirectDestination $redirect_destination) {
+    parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider);
+
+    $this->redirectDestination = $redirect_destination;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
+    return new static(
+      $configuration,
+      $plugin_id,
+      $plugin_definition,
+      $container->get('router.route_provider'),
+      $container->get('redirect.destination')
+    );
+  }
+
   /**
    * {@inheritdoc}
    */
   public function getOptions(RouteMatchInterface $route_match) {
     $options = parent::getOptions($route_match);
     // Append the current path as destination to the query string.
-    $options['query']['destination'] = drupal_get_destination()['destination'];
+    $options['query']['destination'] = $this->redirectDestination->get();
     return $options;
   }
 

core/modules/views/src/Controller/ViewAjaxController.php

@@ -14,6 +14,7 @@
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Path\CurrentPathStack;
 use Drupal\Core\Render\RendererInterface;
+use Drupal\Core\Routing\RedirectDestinationInterface;
 use Drupal\views\Ajax\ScrollTopCommand;
 use Drupal\views\Ajax\ViewAjaxResponse;
 use Drupal\views\ViewExecutableFactory;
@@ -55,6 +56,13 @@ class ViewAjaxController implements ContainerInjectionInterface {
    */
   protected $currentPath;
 
+  /**
+   * The redirect destination.
+   *
+   * @var \Drupal\Core\Routing\RedirectDestinationInterface
+   */
+  protected $redirectDestination;
+
   /**
    * Constructs a ViewAjaxController object.
    *
@@ -66,12 +74,15 @@ class ViewAjaxController implements ContainerInjectionInterface {
    *   The renderer.
    * @param \Drupal\Core\Path\CurrentPathStack $current_path
    *   The current path.
+   * @param \Drupal\Core\Routing\RedirectDestinationInterface $redirect_destination
+   *   The redirect destination.
    */
-  public function __construct(EntityStorageInterface $storage, ViewExecutableFactory $executable_factory, RendererInterface $renderer, CurrentPathStack $current_path) {
+  public function __construct(EntityStorageInterface $storage, ViewExecutableFactory $executable_factory, RendererInterface $renderer, CurrentPathStack $current_path, RedirectDestinationInterface $redirect_destination) {
     $this->storage = $storage;
     $this->executableFactory = $executable_factory;
     $this->renderer = $renderer;
     $this->currentPath = $current_path;
+    $this->redirectDestination = $redirect_destination;
   }
 
   /**
@@ -82,7 +93,8 @@ public static function create(ContainerInterface $container) {
       $container->get('entity.manager')->getStorage('view'),
       $container->get('views.executable'),
       $container->get('renderer'),
-      $container->get('path.current')
+      $container->get('path.current'),
+      $container->get('redirect.destination')
     );
   }
 
@@ -151,8 +163,7 @@ public function ajaxView(Request $request) {
         if ($query != '') {
           $origin_destination .= '?' . $query;
         }
-        $destination = &drupal_static('drupal_get_destination');
-        $destination = array('destination' => $origin_destination);
+        $this->redirectDestination->set($origin_destination);
 
         // Override the display's pager_element with the one actually used.
         if (isset($pager_element)) {

core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php

@@ -47,6 +47,20 @@ class ViewAjaxControllerTest extends UnitTestCase {
    */
   protected $currentPath;
 
+  /**
+   * The redirect destination.
+   *
+   * @var \Drupal\Core\Routing\RedirectDestinationInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $redirectDestination;
+
+  /**
+   * The renderer.
+   *
+   * @var \Drupal\Core\Render\RendererInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $renderer;
+
   protected function setUp() {
     $this->viewStorage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface');
     $this->executableFactory = $this->getMockBuilder('Drupal\views\ViewExecutableFactory')
@@ -62,8 +76,9 @@ protected function setUp() {
     $this->currentPath = $this->getMockBuilder('Drupal\Core\Path\CurrentPathStack')
       ->disableOriginalConstructor()
       ->getMock();
+    $this->redirectDestination = $this->getMock('\Drupal\Core\Routing\RedirectDestinationInterface');
 
-    $this->viewAjaxController = new ViewAjaxController($this->viewStorage, $this->executableFactory, $this->renderer, $this->currentPath);
+    $this->viewAjaxController = new ViewAjaxController($this->viewStorage, $this->executableFactory, $this->renderer, $this->currentPath, $this->redirectDestination);
   }
 
   /**

core/tests/Drupal/Tests/Core/Routing/RedirectDestinationTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Routing\RedirectDestinationTest.
+ */
+
+namespace Drupal\Tests\Core\Routing;
+
+use Drupal\Core\Routing\RedirectDestination;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Routing\RedirectDestination
+ * @group Routing
+ */
+class RedirectDestinationTest extends UnitTestCase {
+
+  /**
+   * The request stack.
+   *
+   * @var \Symfony\Component\HttpFoundation\RequestStack
+   */
+  protected $requestStack;
+
+  /**
+   * The mocked URL generator.
+   *
+   * @var \Drupal\Core\Routing\UrlGeneratorInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $urlGenerator;
+
+  /**
+   * The tested redirect destination.
+   *
+   * @var \Drupal\Core\Routing\RedirectDestination
+   */
+  protected $redirectDestination;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->requestStack = new RequestStack();
+    $this->urlGenerator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface');
+    $this->redirectDestination = new RedirectDestination($this->requestStack, $this->urlGenerator);
+  }
+
+  protected function setupUrlGenerator() {
+    $this->urlGenerator->expects($this->any())
+      ->method('generateFromRoute')
+      ->willReturnCallback(function($route, $parameters, $options) {
+        $query_string = '';
+        if (!empty($options['query'])) {
+          $query_string = '?' . $options['query'];
+        }
+
+        return '/current-path' . $query_string;
+      });
+  }
+
+  /**
+   * @dataProvider providerGet
+   *
+   * @covers ::get
+   */
+  public function testGet(Request $request, $expected_destination) {
+    $this->requestStack->push($request);
+    $this->setupUrlGenerator();
+
+    // Call in twice in order to ensure it returns the same the next time.
+    $this->assertEquals($expected_destination, $this->redirectDestination->get());
+    $this->assertEquals($expected_destination, $this->redirectDestination->get());
+  }
+
+  /**
+   * @dataProvider providerGet
+   *
+   * @covers ::getAsArray
+   */
+  public function testGetAsArray(Request $request, $expected_destination) {
+    $this->requestStack->push($request);
+    $this->setupUrlGenerator();
+
+    // Call in twice in order to ensure it returns the same the next time.
+    $this->assertEquals(['destination' => $expected_destination], $this->redirectDestination->getAsArray());
+    $this->assertEquals(['destination' => $expected_destination], $this->redirectDestination->getAsArray());
+  }
+
+  public function providerGet() {
+    $data = [];
+
+    $request = Request::create('/');
+    $request->query->set('destination', '/example');
+    // A request with a destination query.
+    $data[] = [$request, '/example'];
+
+      // A request without a destination query,
+    $request = Request::create('/');
+    $data[] = [$request, '/current-path'];
+
+    // A request without destination query, but other query attributes.
+    $request = Request::create('/');
+    $request->query->set('other', 'value');
+    $data[] = [$request, '/current-path?other=value'];
+
+    return $data;
+  }
+
+  /**
+   * @covers ::set
+   * @covers ::get
+   */
+  public function testSetBeforeGetCall() {
+    $this->redirectDestination->set('/example');
+    $this->assertEquals('/example', $this->redirectDestination->get());
+  }
+
+  /**
+   * @covers ::set
+   * @covers ::get
+   */
+  public function testSetAfterGetCall() {
+    $request = Request::create('/');
+    $request->query->set('destination', '/other-example');
+    $this->requestStack->push($request);
+    $this->setupUrlGenerator();
+
+    $this->redirectDestination->set('/example');
+    $this->assertEquals('/example', $this->redirectDestination->get());
+  }
+
+}