API migration: No hook_redirect_alter() in 8.x

Can you maybe explain a bit more what your use case for this is? What do you think that hook should allow to do?

I could imagine that a standard response event subscriber might also work, but you for example wouldn't have access to the redirect entity and it might not be trivial to identify whether a redirect is actually coming from redirect.module, possibly by checking some headers.

A possible option would be to either have a custom redirect class that also contains the redirect entity and/or have a custom event, we probably shouldn't add "new" hooks.

I'd like to be able to strip or alter certain query parameters during the redirect execution, so that the destination does not have for instance "id=5" in it (I want this for certain redirects - otherwise use the global "preserve params" option).

I can use this, at least temporarily, to avoid what's happening in #2984272: Dots in query parameter names converted to underscores.

(removed, ignore this)

Maybe it's my Docker environment, will re-test that tomorrow.

I think it was some file sync error with Docker. Secondly I was testing the wrong hook (D7 version of it instead of new one).

I've tested hook_redirect_response_alter() and I've got the dump of $redirect, which is correct.

I've managed successfully implement hook_redirect_response_alter using the #17 patch in order to implement token support and it works.

More information at: #1331582-12: Use tokens for redirects

1. +++ b/redirect.api.php
   @@ -53,6 +53,31 @@
   +  // Set a drupal message.
   +  if (!$redirect->getRedirectUrl()->isExternal()) {
   +    drupal_set_message(t('You are not being directed off-site.'), 'status');
   +  }
   

   Sorry, but this is referencing deprecated code, should use \Drupal::messenger().

2. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
   @@ -167,32 +145,10 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
   -   * @param \Drupal\Core\Url $url
   -   *   The Url where we want to redirect.
   -   */
   -  protected function setResponse(GetResponseEvent $event, Url $url) {
   -    $request = $event->getRequest();
   -    $this->context->fromRequest($request);
    
   -    parse_str($request->getQueryString(), $query);
   -    $url->setOption('query', $query);
   -    $url->setAbsolute(TRUE);
   +      // Invoke hook_redirect_response_alter().
   +      $this->moduleHandler->alter('redirect_response', $response, $redirect);
    
   -    // We can only check access for routed URLs.
   -    if (!$url->isRouted() || $this->checker->canRedirect($request, $url->getRouteName())) {
   -      // Add the 'rendered' cache tag, so that we can invalidate all responses
   -      // when settings are changed.
   -      $response = new TrustedRedirectResponse($url->toString(), 301);
   -      $response->addCacheableDependency(CacheableMetadata::createFromRenderArray([])->addCacheTags(['rendered']));
          $event->setResponse($response);
        }
   

   This removes a lot of unrelated code, I'd rather keep this issue to a minimum and clean it up separately, easier if someone ever needs to debug that and I need to check whether this code really can be removed.

I'm also wondering if the canonical redirect handling would also need an alter hook. And still not 100% convinced the hook is the right way to go. Just thinking out loud, an option might be to have a custom redirect class that has the redirect entity included, so that people can implement response events instead and check for that type of response.

This patch provides a very quick way of implementing redirect logic. It is very useful on my project that uses the domain access module and I'd love to see this patch make it to a release.

One thing that I would like to understand better is the following mention by dpagini:

In this case, maybe I want to even cancel the redirect altogether and just serve a page not found.

I have a case where I want for the redirect to fire only under a specific set of conditions; otherwise, I want to serve the page without redirecting. How can I cancel the redirect and serve the original page?

Re-rolled patch #17 to apply to the latest 1.4.0 release.

I tested the patch #25 and the redirect.api.php file was updated and the hook_redirect_response_alter is working when you create it.

#25 no longer applies against 1.5.0

Updating with a re-roll of #25 to latest changes in 8.x-1.x. This should fix the patch issue per #28

This addresses the concerns in #23. It removes the changes that refactor out unnecessary class properties and dependency injection. This is because it involves both dependency deprecation which is getting addressed in https://www.drupal.org/project/redirect/issues/3102143, but also unused dependencies, which while it is cruft, doesn't need to be addressed in this feature addition.

+++ b/tests/modules/redirect_test/redirect_test.info.yml
@@ -0,0 +1,7 @@
+package: Testing
+core: 8.x

this needs to use the new compatibility key like other modules to be D9 compatible.

Sorry for the nitpick and slow response.

Not at all. Here is a patch to updateredirect_test.info.yml's core_version_requirement to match the main redirect.info.yml

Patch applied cleanly for me (on version 1.4). Many thanks!

Added a "$defaultTheme" to "tests/src/Functional/RedirectHooksTest.php" in order to be D9 compatible.

I created a MR for this issue.

I changed the parameter in the documentation file redirect.api.php... I also changed it in the tests.

Replaced RedirectResponse with TrustedRedirectResponse.

Reviewing this one

I've successfully implemented MR!15. This correctly adress the issue. Moving to RTBC.

Merged, thanks.

As part of my re-roll of https://www.drupal.org/project/redirect/issues/2957751#comment-15248419 I wrote the following commit:

https://git.drupalcode.org/issue/redirect-2957751/-/commit/2de86d1ef5ce2...

At the time that had seemed to me like a bug fix for this issue, so I'm leaving a comment about it here so that the appropriate people can review/confirm whether this change should be applied, as it looks like it was left out of the subsequent re-roll that was ultimately committed for that other issue.