Replace kernel reboot with actual boot to reset services

[Seros]

Hi,
we are using codeception in our api-platform project and atm we are blocked because of symfony/symfony#59036. It got resolved for users of phpunit with WebTestCase tests but in codeception we are still struggling with this. I debugged this and compared both the same workflow of tests in codeception and phpunit. There I noticed that in \Symfony\Bundle\FrameworkBundle\KernelBrowser::doRequest the kernel is booted and shut down before the request and in codeception the kernel is just rebooted without calling the function to reset all resettable services. This change should fix this and ensure, that the requests formats are in a valid state before each request. I'll try to provide a test for this later on. For now I just ensured that current tests are not breaking

[TavoNiievez]

@Seros Since this change affects the Connector we do need a test for this.

Both to know that we are not breaking anything and that it will not break again.
For the moment I made a commit to avoid that boot → shutdown → boot directly, trying to follow how it is done in Symfony.

Please follow the contribution guide and we will move forward with this.

[Seros]

I see. Thanks for making this clear and improving the logic. For my particular problem I would need to install and configure api-platform/symfony v4 in the test project to reproduce the faulty behaviour. Would this be ok?

[TavoNiievez]

@Seros Yes.

I would like to have it as an independent branch with a separate job, but for the moment you can send a normal PR, I will take care of setting it up.

[Seros]

@TavoNiievez Added the test, happy to receive feedback

[TavoNiievez]

@Seros Sorry, I've been busy with some PR on the core project.

I'll check it out this weekend.

[Seros]

Hey, just wondering if you have an estimated timeline for when this PR might be reviewed and potentially merged?

[TavoNiievez]

@Seros I just reverted the changes in this conditional:

-        if ($this->hasPerformedRequest && $this->rebootable) {
-            $this->rebootKernel();
-        } else {
-            $this->hasPerformedRequest = true;
-        }
+        if ($this->rebootable) {
+            if ($this->hasPerformedRequest) {
+                $this->rebootKernel();
+            } else {
+                $this->hasPerformedRequest = true;
+            }
+        }

because:

1. it is not necessary for the solution, apparently.
2. there was a logical flow that was being changed and I don't want to take risks with possible side-effects.

I also just modified the CI to test the branch you added in the test project, that was the missing step.

With that we are ready to move on.

Thank you!

[Seros]

Got it. Thanks a lot for merging this!

[Seros]

Hi,

Would it be possible to cut a new release that includes these changes? The fix helps a lot with Symfony compatibility and unblocks my team (and likely others) who were running into issues with kernel service resets.

It would be awesome to have this available in an official release, as it’s currently not included in the latest (v3.5.1).
Thanks again for your hard work on this project!

[TavoNiievez]

@Seros Released as version 3.6.0

I am currently working on making the module PHPStan friendly at the maximum level. I was targeting that version to release the changes but responding to your request, yes, it can wait.

Thank you very much for contributing and for your patience on this matter!