Problem/Motivation

According to \Drupal\Core\Url::createFromRequest:

  /**
   * @throws \Drupal\Core\Routing\MatchingRouteNotFoundException
   *   Thrown when the request cannot be matched.
   */
  public static function createFromRequest(Request $request) {

But really it throws Symfony\Component\Routing\Exception\ResourceNotFoundException, which is NOT handled with code like this:

    try {
      $url = Url::createFromRequest($request);
    } catch (MatchingRouteNotFoundException) {
      return;
    }

The class is used nowhere else.

Proposed resolution

Remove or deprecate the class, fix docs.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 interdiff_4-8.txt1.53 KBakram khan
#8 3350922-8.patch2.32 KBakram khan
#4 3350922.patch1.23 KBranjit1032002

Issue fork drupal-3350922

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

geek-merlin created an issue. See original summary.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Title: Remove MatchingRouteNotFoundException and docs » Remove unused MatchingRouteNotFoundException and wrong docs

Ranjit1032002 made their first commit to this issue’s fork.

ranjit1032002’s picture

ranjit1032002
Primary language English
Location India
commented
Status: Active » Needs review
StatusFileSize
new 3350922.patch1.23 KB

Created a patch for Removing unused MatchingRouteNotFoundException and wrong docs, please review.
Thank You.

Status: Needs review » Needs work

The last submitted patch, 4: 3350922.patch, failed testing. View results

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs work » Needs review

That was a sporadic test failure, re-test is green.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Needs work 
diff --git a/core/lib/Drupal/Core/Url.php b/core/lib/Drupal/Core/Url.php
index a37f75e0d1..91eb5b6698 100644
--- a/core/lib/Drupal/Core/Url.php
+++ b/core/lib/Drupal/Core/Url.php
@@ -481,9 +481,6 @@ protected static function fromRouteUri(array $uri_parts, array $options, $uri) {
    *   A Url object. Warning: the object is created even if the current user
    *   would get an access denied running the same request via the normal page
    *   flow.
-   *
-   * @throws \Drupal\Core\Routing\MatchingRouteNotFoundException
-   *   Thrown when the request cannot be matched.
    */

Let's replace this with what is really thrown, copied over from \Symfony\Component\Routing\Matcher\RequestMatcherInterface::matchRequest

    /**
     * @throws NoConfigurationException  If no routing configuration could be found
     * @throws ResourceNotFoundException If no matching resource could be found
     * @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed
     */

NW for that, otherwise RTBC.

akram khan’s picture

akram khan
Primary language English
Location Cuttack, Odisha
commented
StatusFileSize
new 3350922-8.patch2.32 KB
new interdiff_4-8.txt1.53 KB

added updated patch and address #7

akram khan’s picture

akram khan
Primary language English
Location Cuttack, Odisha
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Part of me is saying this should be deprecated vs just removing but searching

https://git.drupalcode.org/search?group_id=2&scope=blobs&search=Matching...

I couldn't find any usage other then core forks.

 // cspell:ignore abempty
-
 /**

Out of scope change added in #8 that should be fixed if committed. But won't hold for that 1 space.

The last submitted patch, 4: 3350922.patch, failed testing. View results

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

(Yers, 4 is red, but 8 is green...)

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Yes, I agree we need to deprecate the class before we can remove it - see the policy for how we deprecate concrete classes.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone made their first commit to this issue’s fork.

quietone opened merge request !8852

quietone’s picture

quietone commented
Title: Remove unused MatchingRouteNotFoundException and wrong docs » Remove unused MatchingRouteNotFoundException
Status: Needs work » Needs review
Issue tags: -Needs change record

Update the MR, created a CR.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Looks good, nothing more to add.

catch’s picture

catch
he/him
Primary language English
commented
Title: Remove unused MatchingRouteNotFoundException » Deprecate unused MatchingRouteNotFoundException
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

  • catch committed 96b418f5 on 11.x 
    Issue #3350922 by Ranjit1032002, Akram Khan, quietone, geek-merlin,...
catch’s picture

catch
he/him
Primary language English
commented

catch closed merge request !8852

Status: Fixed » Closed (fixed)

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