[Routing] Add possibility to create a request context with parameters directly #60120
Conversation
| @@ -43,6 +43,7 @@ public function __construct(string $baseUrl = '', string $method = 'GET', string | |||
| $this->setHttpsPort($httpsPort); | |||
| $this->setPathInfo($path); | |||
| $this->setQueryString($queryString); | |||
| $this->setParameters([...$this->parameters, ...$parameters]); | |||
There was a problem hiding this comment.
The class is not final, so we merge it with eventually parent set parameters.
There was a problem hiding this comment.
which parent set parameters are you talking about ?
There was a problem hiding this comment.
A class not final like RequestContext can be extends and call inside it constructor before the parent construct the setter methods:
class MyCustomContext extends RequestContext {
public function __construct()
{
$this->setParameters(['key' => 'value']);
parent::__construct();
}
}See also new added test. Doing just setParameters($parameters); would override existing parameters of that class.
There was a problem hiding this comment.
we could also create a deprecation if (count($this->parameters) > 0) { and simplify this in Symfony 8?
There was a problem hiding this comment.
One other question is maybe what is expected behaviour for:
$this->setParameters(['key' => 'value']);
parent::__construct(parameters: ['other' => 'test']);If not merged we could go for bc layer with:
if (count($parameters) > 0) {
$this->setParameters($parameters);
} elseif (count($this->parameters) > 0) {
trigger_deprecation('symfony/routing', '7.3', 'Set parameters directly via the own constructor argument');
}which in 8.0 gets:
$this->setParameters($parameters);| $this->assertEquals(['foo' => 'bar'], $requestContext->getParameters()); | ||
| } | ||
|
|
||
| public function testConstructParametersBcLayer() |
There was a problem hiding this comment.
not sure what this test case is about
There was a problem hiding this comment.
its about why the if is needed for BC reasons.
Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>
While I worked on the route integration into Sulu. I had to write a few tests for depending on the RequestContext. To improve the DX I think the parameters should be possible also inside the construct directly so instead of:
I can do: