The todo from:

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21PathProce...

and was added here:

#2335661: Outbound path & route processors must specify cacheability metadata

The problem that I cannot find any mentions about UrlGenerator::fromPath(), maybe it's a mistake and actually it should be UrlGeneratorInterface::generateFromPath(). In any casse any of them not exist in core, so it's time to resolve the todo.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

init90 created an issue. See original summary.

init90’s picture

Status: Active » Needs review
FileSize
1.37 KB

Let's try

init90’s picture

Issue summary: View changes

Ok, looks like we cannot simply remove this code. When I run tests with the next code change:

  public function processOutbound($path, &$options = [], Request $request = NULL, BubbleableMetadata $bubbleable_metadata = NULL) {
    // The special path '<front>' links to the default front page.
//    if ($path === '/<front>') {
//      $path = '/';
//    }
    return $path;
  }

I've got the next test error:

There was 1 failure:

1) Drupal\Tests\Core\PathProcessor\PathProcessorFrontTest::testProcessOutbound with data set #0 ('/<front>', '/')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/'
+'/<front>'

/var/www/docroot/core/tests/Drupal/Tests/Core/PathProcessor/PathProcessorFrontTest.php:80

So it mean that processOutbound still needed. Maybe, in that case, we can simply remove that todo?

govind.maloo’s picture

Status: Needs review » Needs work
Sivaji_Ganesh_Jojodae’s picture

Status: Needs work » Needs review
FileSize
585 bytes

The attached patch just removes the @todo. Needs Review.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Status: Needs review » Needs work

Found this @todo again today.

As we are removing the processOutbound() method, we need to remove the explicit test for it as well. If there was something else relying on this behaviour I would hope it would be tested as part of something else, but that doesn't seem to be the case.

longwave’s picture

Also the patch in #2 needs to remove the service tag, as in the patch in #2057577-4: Remove PathProcecssorFront::processOutbound once getPathFromRoute is not available anymore

ankithashetty’s picture

Status: Needs work » Needs review
FileSize
2.94 KB
1.35 KB

Addressed the changes specified in #9 and #10, thanks!

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! This hasn't broken anything else according to tests, and the @todo says this should be removed when UrlGenerator::fromPath() was removed - which probably meant UrlGeneratorInterface::generateFromPath(), which was removed in #2575869: Remove all remaining usage of deprecated UrlGeneratorInterface::generateFromPath() and delete it - it seems this is OK to remove now.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 1d93843 and pushed to 9.3.x. Thanks!

alexpott’s picture

Title: Resolve @todo - remove ::processOutbound() when we remove UrlGenerator::fromPath() » Resolve @todo - remove ::processOutbound() when we remove UrlGenerator::generateFromPath()

  • alexpott committed 1d93843 on 9.3.x
    Issue #3110580 by ankithashetty, init90, Sivaji_Ganesh_Jojodae, longwave...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.