lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 125 forks source link

bugfix(routing): Resolve a routing issue found in octane #221

Closed ClaraLeigh closed 1 year ago

ClaraLeigh commented 1 year ago

Description

When using octane, I found that subsequent requests would often fail, with two different errors showing:

App\xxxxx::handle(): Argument #1 ($model) must be of type App\Model\Xxxxx, string given, called in /var/task/vendor/lorisleiva/laravel-actions/src/Concerns/AsController.php on line 14

Too few arguments to function App\xxxxx::handle(), 0 passed

From these errors, I can determine that route binding and dependancy injection is breaking on occasion. As such we need to ensure that we are resolving the arguments correctly before calling the handle() method.

Changes Made

File src/Concerns/AsController.php

File src/Decorators/ControllerDecorator.php

Issues remaining

I feel like there is a double up now, that ControllerDecorator runs twice as it seems to resolve things like jsonResponse and htmlResponse. This is why the new execute function uses run and not callAction or __invoke

Testing - needs work

This needs to be tested in a replicable octane environment, for the moment I tested by manually disabling the call to replaceRouteMethod in the decorator's constructor and then updating the AsController::__invoke method to look like so:

$decorator = (new ControllerDecorator($this, \Route::current()));
$decorator->replaceRouteMethod();
return $decorator->execute();

This would then cause the default AsController::__invoke function to run first, first like it does in subsequent Octane calls

Related Issues

lorisleiva commented 1 year ago

essentially runs the ControllerDecorator again. This is not ideal but works for now.

Hi there, thank you for tackling this issue but could we solve this differently? I don't think running something twice can be an option.

ClaraLeigh commented 1 year ago

@lorisleiva yea I would like to but the reason for putting everything here is the hope that someone might work out what the real problem is. A bit low on time to dig into it further at this time