!!! TASK: Remove dispatched state from ActionRequest

[kitsunet]

I will dig a bit into history as I guess something (that obviously no one cares about) was lost in the PSR HTTP refactoring. I think (as we can see in the dispatch loop) there was a possibility before for a controller to gain control but decide it does not finally dispatch the request and thus the dispatch loop would continue and try to find another controller to take care, so it was the controllers responsibility to set this, but I am not sure what conditions it previously attached to it being dispatched.

Upgrade instructions

Review instructions

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[mhsdesign]

as discussed we concluded that this logic might be faulty and actually do harm without the isDispatched in place.

background:

the dispatcher is either used for main request, in which case this logic is not needed,
or for subrequest like in a fusion plugin

scenario:

a subrequest, while being handled, throws a StopActionException.
which is catched here. We check if its a main request and get the main request which is now to be handled in the loop, like a forward.

Thesis:

Now it will actually render the outer response inside the plugin
But previously because the main request was dispatched === true, it would not be handled.

So in theory this is dead code, and now must be removed.

[mhsdesign]

Yes the thesis seems to be correct. Using throwStatus in a plugin will result in the main request rendering being started again an again until the memory is exhausted.

Removing this code will actually make the tests dispatchIgnoresStopExceptionsForFirstLevelActionRequests and dispatchCatchesStopExceptionOfActionRequestsAndRollsBackToTheParentRequest fail, so i wanted to research about the original intentions.
It seems that with the introduction ff05084#diff-450585af3bb27be2be268192637242195c5d6308512fd53dac9fa248ff319686 of sub request, this logic was introduced. (perviously all first level StopActionException where silently swallowed)

} catch (\F3\FLOW3\MVC\Exception\StopActionException $stopActionException) {
    if ($request instanceof \F3\FLOW3\MVC\Web\SubRequest && $request->isDispatched()) {
        throw $stopActionException;
    }
}

[mhsdesign]

i removed the

if (!$request->isMainRequest()) {
    $request = $request->getParentRequest();
}

part in 0ed5f28 ;)

[mhsdesign]

We further discussed that the ActionReponse doesnt need to be mutable anymore, and all its setters should be either deprecated or made internal.

One must use the ForwardException with attached $nextRequest instead to keep the dispatch loop going.