-
Notifications
You must be signed in to change notification settings - Fork 39
Add request matcher #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/** | ||
* Decides whether the rule(s) implemented by the strategy matches the supplied request. | ||
* | ||
* Equivalent of https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/RequestMatcherInterface.php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PSR-7 implementation of Symfony's RequestMatcher
@see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/RequestMatcherInterface.php
Other possible matchers to add:
|
LGTM 👍 |
17e8b80
to
e792553
Compare
Other matchers for another PR when we need it :) |
i like the symfony approach to this better, actually: https://github.com/symfony/symfony/blob/582f4753a343f230fbe18b4e9a0747d48351ddfb/src/Symfony/Component/HttpFoundation/RequestMatcher.php if we have a separate matcher for each part of the request, we build huge object trees. |
I don't see a reason why the regex one could not live together with the one you linked. The message library is not dedicated to PHP-HTTP packages (but rather to PSR-7 as a utility package), so IMO we can have it in here. There aren't any heavy dependencies. (BTW: We should ask the FIG if they want a http-message-utils repo, similarly to cache-utils.) But I am not against separating it to be standalone. (One reason against it: makes it harder to use it INSIDE message. For example I would use it the Matching authentication) |
agreed, lets keep it inside message then. good idea to ask fig - if we
contribute parts of this to their world we can then depend on those
parts from message without worries and still rely on it.
my unhappiness with the RegexRequestMatcher is that it is only matching
one thing, instead of ip, host, path, method and so on like the symfony
one. if we add the full matcher, the regex matcher becomes redundant.
the name is a bit unclear too, its a RegexUriRequestMatcher, not regex
for host, path or method...
|
It's just a first implementation to have a simple example, as i'm only interested in the interface for the moment, will certainly write other implementations at a latter time :) It can be renamed if you want, also didn't want a too long name, but yeah RegexUriRequestMatcher may be cleaner. |
See php-http/client-common#13