Closed Tobion closed 7 years ago
Performance impact of request transformation of was an argument in core meeting. So this would need to be measured.
Do we really need to change the method name in UrlGeneratorInterface ?
Regarding keeping RequestMatcherInterface, an interface using PSr-7 must be a different interface, not the current RequestMatcherInterface, otherwise it is a big BC break.
Regarding keeping RequestMatcherInterface, an interface using PSr-7 must be a different interface, not the current RequestMatcherInterface, otherwise it is a big BC break.
I think we need to discuss the goal of symfony 3 at next meeting. If the goal is that library authors can write code that is compatible with both symfony 2 and 3 at the same time, then we cannot do such a change indeed.
How about PsrRequestMatcherInterface::matchPsrRequest
? Looks ugly, but ok.
Do we really need to change the method name in UrlGeneratorInterface ?
No we do not need to as I explained. But then we keep the inconsistency generate
vs matchPsrRequest
.
I think we can close here, to me, PSR-7 support is a very low priority and the benefit would be really thin.
I agree with Nicolas. Let's close. Thanks!
@nicolas-grekas, @javiereguiluz In my case PSR-7 support would be useful for RoadRunner routing
The routing component currently has two methods to match the request:
UrlMatcherInterface
andRequestMatcherInterface
.This sucks for implementations as people need to implement both and it is bad from an architecture perspective. It was only created as a workaround anyway. See also https://github.com/symfony-cmf/Routing/pull/116#issuecomment-68352907
For version 3.0 we should get rid of this. With the rise of PSR-7 it is also the perfect time to make use of these interfaces as well since we would break BC anyway. This of course requires https://github.com/symfony/psr-http-message-bridge so that the routing component can easily be integrated in symfony itself.
Here my vision:
public function matchRequest(Psr\Http\Message\RequestInterface $request);
public function setCurrentUri(Psr\Http\Message\UriInterface $uri);
. The context for generating URLs only needs to be a URI (for generating relative URLs) and not a full request (with method and headers). And it also currently has "global default parameters" (RequestContext::getParameters) which we need to incorporate with something likesetDefaultParameters(array $parameters)
. Currently there are also thehttpPort
andhttpsPort
on the context. This functionality should be moved to aport
config on the route. This way it is also possible to generate/match routes with different ports which have the same schema. I guess it should be aports
config to be consistent withschemes
which also allows multiple.Open questions:
RequestMatcherInterface::matchRequest
vsRequestMatcherInterface::match
UrlGeneratorInterface
vssetCurrentUri(Psr\Http\Message\UriInterface $uri)
: URL from symfony vs URI from PSR-7UrlGeneratorInterface::generate
vsUrlGeneratorInterface::generateUri
vsUrlGeneratorInterface::generateUrl
Changing everything to URI would probably be too big of a BC break. So I guess we have to live with the URL vs URI inconsistency. I think
matchRequest
andgenerateUrl
are better because they are more explicit. But that would require a BC break in UrlGeneratorInterface which we could avoid if we just leave itgenerate()
(andmatch()
).