[HttpFoundation] Extract request matchers for better reusability by fabpot · Pull Request #47595 · symfony/symfony

fabpot · 2022-09-16T08:56:16Z

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	n/a
License	MIT
Doc PR

The RequestMatcher class hardcodes its matchers. This PR extracts those into their own classes so that we can compose a RequestMatcher and allow better reusability.

fabpot · 2022-09-16T08:57:58Z

I would love to simplify the RequestMatcher class by deprecating all the match* methods but I'm not sure it's worth it.

HeahDude

Should the class be renamed ChainRequestMatcher as of Sf 7.0 and/or add a @final?

src/Symfony/Component/HttpFoundation/RequestMatcher.php

src/Symfony/Component/HttpFoundation/RequestMatcher/AttributesRequestMatcher.php

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

HeahDude · 2022-09-17T07:22:40Z

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php


        $container
-            ->register($id, RequestMatcher::class)
+            ->register($id, ChainRequestMatcher::class)


Suggested change

->register($id, ChainRequestMatcher::class)

$container->register($id, ChainRequestMatcher::class)

?

I'd keep the current name: it doesn't have the same interface + needless BC break ahead when renaming.

src/Symfony/Component/HttpFoundation/RequestMatcher/PathRequestMatcher.php

src/Symfony/Component/HttpFoundation/RequestMatcher/IsJsonRequestMatcher.php

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

src/Symfony/Component/HttpFoundation/RequestMatcher.php

src/Symfony/Component/HttpFoundation/CHANGELOG.md

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php

src/Symfony/Component/HttpFoundation/RequestMatcher/AttributesRequestMatcher.php

src/Symfony/Component/HttpFoundation/RequestMatcher/IpsRequestMatcher.php

src/Symfony/Component/HttpFoundation/RequestMatcher/SchemeRequestMatcher.php

src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php

src/Symfony/Component/HttpFoundation/Tests/RequestMatcher/SchemeRequestMatcherTest.php

src/Symfony/Component/HttpFoundation/Tests/RequestMatcher/IpsRequestMatcherTest.php

src/Symfony/Component/HttpFoundation/RequestMatcher/IsJsonRequestMatcher.php

src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

src/Symfony/Component/HttpFoundation/Tests/RequestMatcher/IsJsonRequestMatcherTest.php

GromNaN · 2022-11-24T12:04:16Z

I had to build a factory to create an instance of RequestMatcherInterface from an array of config, compatible with older and newer Symfony versions. If anybody have a similar needs, the code is here: https://gist.github.com/GromNaN/3ab352cc6bc348b8405e98006fcc2754

… behaviors (GromNaN) This PR was squashed before being merged into the 7.0 branch. Discussion ---------- [HttpFoundation] Remove deprecated classes, method and behaviors | Q | A | ------------- | --- | Branch? | 7.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | n/a Clean `symfony/http-foundation` from all its legacy. - Remove `RequestMatcher` and `ExpressionRequestMatcher`, deprecated since #47595 - Remove `Request::getContentType()`, deprecated since #45034 - Throw a `UnexpectedValueException` or `BadRequestException` when `ParameterBag::filter()` or `InputBag::filter()` reads an invalid value and the flag `FILTER_NULL_ON_FAILURE` is not set. new behavior announced since #48525 - Throw a `InvalidArgumentException` when calling `Request::create()` with a malformed URI, deprecated since #49376 Commits ------- 665a775 [HttpFoundation] Remove deprecated classes, method and behaviors

This PR was squashed before being merged into the 7.0 branch. Discussion ---------- [Security] Update request matcher doc RequestMatcher class has been removed in 7.0 cf [code PR](symfony/symfony#47595) Changing link and rewording according to new behaviour Commits ------- f32a8cf [Security] Update request matcher doc

carsonbot added Status: Needs Review Feature HttpFoundation labels Sep 16, 2022

carsonbot added this to the 6.2 milestone Sep 16, 2022

fabpot force-pushed the request-matchers branch 2 times, most recently from b582774 to 1f853b2 Compare September 16, 2022 09:04

HeahDude reviewed Sep 16, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/RequestMatcher.php Outdated Show resolved Hide resolved

fabpot force-pushed the request-matchers branch from 1f853b2 to 8918499 Compare September 16, 2022 10:29

ghost reviewed Sep 16, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/RequestMatcher.php Outdated Show resolved Hide resolved

stloyd reviewed Sep 16, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/RequestMatcher.php Outdated Show resolved Hide resolved

stof reviewed Sep 16, 2022

View reviewed changes

fabpot force-pushed the request-matchers branch 2 times, most recently from bcb1557 to 33f97a3 Compare September 16, 2022 16:17

fabpot requested review from chalasr and wouterj as code owners September 16, 2022 18:23

fabpot force-pushed the request-matchers branch 4 times, most recently from cc7b315 to e23feac Compare September 17, 2022 05:48

HeahDude reviewed Sep 17, 2022

View reviewed changes

nicolas-grekas reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/RequestMatcher.php Show resolved Hide resolved

nicolas-grekas reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/CHANGELOG.md Show resolved Hide resolved

nicolas-grekas reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php Show resolved Hide resolved

stof reviewed Sep 17, 2022

View reviewed changes

fabpot force-pushed the request-matchers branch from e23feac to 79d5ad5 Compare September 17, 2022 11:36

stof reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php Outdated Show resolved Hide resolved

fabpot force-pushed the request-matchers branch 2 times, most recently from 0109fe9 to 961cd85 Compare September 17, 2022 11:59

fabpot force-pushed the request-matchers branch from 961cd85 to c698b4b Compare September 17, 2022 12:03

nicolas-grekas reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php Outdated Show resolved Hide resolved

chalasr reviewed Sep 17, 2022

View reviewed changes

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php Outdated Show resolved Hide resolved

fabpot force-pushed the request-matchers branch from c698b4b to b081a56 Compare September 17, 2022 16:03

stof requested changes Sep 17, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/Tests/RequestMatcher/IsJsonRequestMatcherTest.php Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels Sep 17, 2022

[HttpFoundation] Extract request matchers for better reusability

6cfd3b7

fabpot force-pushed the request-matchers branch from b081a56 to 6cfd3b7 Compare September 17, 2022 16:13

fabpot merged commit 77538c8 into symfony:6.2 Sep 18, 2022

fabpot deleted the request-matchers branch September 18, 2022 06:39

fabpot mentioned this pull request Sep 18, 2022

[HttpFoundation] Extract request matchers for better reusability symfony/symfony-docs#17280

Closed

xabbuh mentioned this pull request Oct 20, 2022

[SecurityBundle] Allow specifying attributes for RequestMatcher #45907

Merged

fabpot mentioned this pull request Oct 24, 2022

Release v6.2.0-BETA1 #47972

Merged

GromNaN mentioned this pull request Jun 29, 2023

[HttpFoundation] Remove deprecated classes, method and behaviors #50826

Merged

MrYamous mentioned this pull request Dec 20, 2023

[Security] Update request matcher doc symfony/symfony-docs#19322

Merged

mvhirsch mentioned this pull request Mar 4, 2024

Fixes namespace of ChainRequestMatcher rectorphp/rector-symfony#586

Merged

	->register($id, ChainRequestMatcher::class)
	$container->register($id, ChainRequestMatcher::class)

Uh oh!

Conversation

fabpot commented Sep 16, 2022

Uh oh!

fabpot commented Sep 16, 2022

Uh oh!

HeahDude left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeahDude Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GromNaN commented Nov 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants