core
core copied to clipboard
WriteListener is called on Get operations
API Platform version(s) affected: 3.3 +
Description
A Get operation using a Controller is always calling the WriteProcessor after the Controller is called, which causes an "Invalid identifier or value configuration" if you are using another identifier.
How to reproduce
Create a Get operation like so :
new Get(
uriTemplate: '/something/by_something/{not_id}',
controller: SomethingController::class,
output: SomethingOutput::class,
read: false
)
Possible Solution
Sadly I don't come with a simple fix BUT I figured out that the persist attribute is true after being resolved in WriteListener. This is caused by the fact that the Request attribute _api_persist is not defined and defined as true by default, which is a shame since writing in a Get operation looks kind of weird.
Additional Context
This exact setup works perfectly using 3.2.x
Also I figured out that on a simple operation like :
new Get(
output: SomethingOutput::class,
provider: SomethingProvider::class,
)
The WriteProcessor is also called which makes no sense.
Could you propose a fix? Indeed the condition should be:
if (null === ($canWrite = $operation->canWrite())) {
$canWrite = !$request->isMethodSafe();
}
Of course, I will try to do it asap 🙂
Alright finally had the time to check into it, and it might sound stupid, but I'm having some trouble validating my pull request. I did follow the steps explained by the docs about contributing but I cannot run PHPUnit on my fork since I'm getting the following message :
PHPUnit is not installed. Please run ./vendor/bin/simple-phpunit to install it
So I ran the Simple PHPUnit, but I'm not able to install PHPUnit since I get the following dependency error :
Updating dependencies
Your requirements could not be resolved to an installable set of packages.
Problem 1
- phpunit/php-code-coverage 9.2.31 requires nikic/php-parser ^4.18 || ^5.0 -> found nikic/php-parser[v4.18.0, v4.19.0, v4.19.1, v5.0.0, v5.0.1, v5.0.2, v5.1.0] but it conflicts with your root composer.json require (4.16).
- phpunit/php-code-coverage 9.2.32 requires nikic/php-parser ^4.19.1 || ^5.1.0 -> found nikic/php-parser[v4.19.1, v5.1.0] but it conflicts with your root composer.json require (4.16).
- Root composer.json requires phpunit/php-code-coverage ^9.2.31 -> satisfiable by phpunit/php-code-coverage[9.2.31, 9.2.32].
Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
I did not dive into it really yet but is it normal that I get this issue while trying to setup my fork ? Isn't there something wrong with the dependencies here ?
Thanks in advance for helping out 🙂
Alright, I found out that the composer.json file generated by the Symfony PHPUnit Bridge was using the 4.6 version of nikic/php-parser because of the SYMFONY_PHPUNIT_REQUIRE variable defined in the phpunit.xml.dist file. Setting it to <env name="SYMFONY_PHPUNIT_REQUIRE" value="nikic/php-parser:^5.1"/> resolves the PHPUnit installation.
However, I don't even need to update anything, tests are failing because of the Doctrine MongoDB Bundle being incorrect on a method declaration.
Why do I struggle so much running tests ? What's wrong here ? 😆
Hi, I guess you're using windows? Try to run only the test at ./tests/Symfony/EventListener/WriteListenerTest.php or is it WriteProcessorTest that you want? It's possible that for some tests you need the php mongodb extension.
I took a look, not sure where to look at. What's your value for use_symfony_listeners?
Hi ! Yes sadly I am on Windows 😄 I didn't think about running only the WriteListenerTest since I wanted to be sure that everything is running but hey I guess this is fine. My tests are valid using the following bit of code updated :
$canWrite = $operation?->canWrite();
if (null === $canWrite) {
$canWrite = !$request->isMethodSafe();
}
if (!$canWrite) {
return;
}
Trying to run Behat but I am having the same mongodb trouble. I don't know if I can avoid this... Never used Behat before 🫤
Just open a PR on the code change I'll handle the rest, thanks!
Alright, pull request is ready. Thanks 😃
try https://github.com/api-platform/core/issues/4891
Hello. Thanks for your suggestion. I didn't try it but even though it may be working in our case, I don't think that it resolves the issue because of the fact that :
- This use to work using version 3.2 and the fact that it does not work anymore is not expected
- In my opinion, I shoud not have to set
write: falseon a Get operation
I do have a fix for my initial request already, but now I would like to fix the discovered issue since it should be working
Well it's been a month now I guess we're leaving this uncorrected :(