scribe
scribe copied to clipboard
Url parameter access on formRequest does not work
Hello,
In a FormRequest, access directly to url parameter does not work (null). Here is a code example:
<?php
namespace App\Http\Requests;
use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Support\Facades\Route;
class UserRequest extends FormRequest
{
public function rules(): array
{
$this->user; // null
$this->route('user'); // null
Route::current()->parameters()['user']; // not null
}
}
Only the third one works but i prefer to avoid this.
I ran php artisan scribe:generate
Ran into the same problem here. Third option does indeed work.
Same here but not even the third option works. Im using ver. 4.12.0
Thanks for all the reports. Help fixing this would also be appreciated.
Hi,
Same problem here, i have a basic formRequest with the following rule :
Rule::unique('users', 'login')->ignore($this->user->login, 'login')
And scribe throw exception cause $this->user is null
I have dived into your package and started to write a solution. My solution work but only if i specify an @urlParam
For example : route PUT /api/users/{user} and controller :
/**
* @urlParam user int required L'identifiant de l'utilisateur. Exemple: 300
*/
public function update(UserRequest $request, UserModel $user){ // ... }
And here the following update that i have wrote into scribe Knuckles\Scribe\Extracting\Strategies\GetFromFormRequestBase.php
public function __invoke(ExtractedEndpointData $endpointData, array $routeRules = []): ?array
{
return $this->getParametersFromFormRequest($endpointData, $endpointData->method, $endpointData->route);
}
public function getParametersFromFormRequest(ExtractedEndpointData $endpointData, ReflectionFunctionAbstract $method, Route $route): array
{
if (!$formRequestReflectionClass = $this->getFormRequestReflectionClass($method)) {
return [];
}
if (!$this->isFormRequestMeantForThisStrategy($formRequestReflectionClass)) {
return [];
}
$className = $formRequestReflectionClass->getName();
/*
* Get the params name/example value
*/
$params = [];
foreach($endpointData->urlParameters as $param) {
$params[$param->name] = $param->example;
}
/**
* instanciate a new request with the route parameters
*/
$request = \Illuminate\Http\Request::create($route->uri(), $route->methods()[0], $params);
if (Globals::$__instantiateFormRequestUsing) {
$formRequest = call_user_func_array(Globals::$__instantiateFormRequestUsing, [$className, $route, $method]);
} else {
/**
* instanciate a new form request
*/
$formRequest = \Illuminate\Foundation\Http\FormRequest::createFrom($request, new $className);
}
/**
* bind the route parameters to the form request
*/
$route->bind($formRequest);
/**
* set the parameters to the route
*/
foreach($endpointData->urlParameters as $param) {
$route->setParameter($param->name, $param->example);
}
/**
* convert params to model instances
*/
app('router')->substituteBindings($route);
app('router')->substituteImplicitBindings($route);
$formRequest->server->set('REQUEST_METHOD', $route->methods()[0]);
$parametersFromFormRequest = $this->getParametersFromValidationRules(
$this->getRouteValidationRules($formRequest),
$this->getCustomParameterData($formRequest)
);
return $this->normaliseArrayAndObjectParameters($parametersFromFormRequest);
}
But i have found 2 problems and thats why i don't PR your my solution. First : $endpointData->urlParameters have 2 keys, the one added by Strategies\UrlParameters\GetFromLaravelAPI::class and the second my @urlParams added by Strategies\UrlParameters\GetFromUrlParamTag::class, that don't cause problem but it's little strange that my @urlParams doesn't override the first one ?
Second : in the case i don't specify the @urlParams i have an not found exception throw by model binding cause GetFromLaravelAPI create a tag user_id and substitution doesn't work.
Hope it help. :)
I've not looked at this in detail yet, but if you're unhappy with the results of GetFromLaravelAPI and want to adjust it, you can either:
- Remove the strategy from your config to disable it entirely, OR
- Use the
normalizeEndpointUsing()
hook to customise or disable URL normalization (the process that converts/{user}
to/{user_id}
). There are examples in the docs - https://scribe.knuckles.wtf/laravel/hooks#normalizeendpointurlusing
I have already think to that, in fact i have no problem with this strategies, i just wanted to warn you about my fix problems, maybe i wasn't clear enough
Perhaps we can move normalizeEndpointUsing()
to after extracting all data, and it would be nice if ExtractedEndpointData
marked the urlParams
example to Eloquent model binding so when extracting the body params it could try to do the models stratigies databaseFirst
, factoryCreate
or factoryMake
.
I had a similar issue, which I was able to solve by using instantiateFormRequestUsing.
Here's a simple example where the rules depend on a route param:
class SomeFormRequest extends FormRequest
{
public Customer $customer;
protected function prepareForValidation()
{
$this->customer = $this->route('customer');
}
public function rules()
{
return [
'user_id' => ['required', Rule::exists('users', 'id')->where('customer_id', $this->customer->id)],
];
}
}
And in AppServiceProvider's boot method:
if (class_exists(Scribe::class)) {
Scribe::instantiateFormRequestUsing(function (string $formRequestClassName) {
if ($formRequestClassName === SomeFormRequest::class) {
$formRequest = new $formRequestClassName();
$formRequest->customer = new Customer();
}
return $formRequest;
});
}
Note that the Scribe documentation's example uses app()->makeWith()
to instantiate the formrequest, which doesn't work in my experience. When a FormRequest is resolved this way, it automatically triggers the ValidatesWhenResolved
trait, meaning that a ValidationException is thrown if the rules don't pass (which they probably won't since there are no request params). But you can instantiate the FormRequest using new
, and then add "mock" dependencies, before passing it back to Scribe.
So basically this workaround works by having the dependencies defined as class attributes, which are normally populated in prepareForValidation(), but can also be set manually.
@mortenscheel Thank you!! Using your solution solved #526
I've just hit this issue after installing Scribe for the first time. I will look into a fix for this.
I believe that in order to fix this issue, the path, annotated, attribute and method arguments should be merged and iterated over. If the types extend Model then instantiateExampleModel should be used to set the parameter on the route after it is bound using the request in getParametersFromFormRequest
.
I'm still getting to know the codebase, for anyone experienced with the codebase does this sound like a suitable solution?
From what I can see the models are only instantiated for responses, would instantiating them for typed method arguments be a possibility?
Not gonna lie, I'm thoroughly confused here...is the problem model binding or something else?
Correct, model binding isn't working in rules()
I've added a test here
When in the rules()
method, $this->route('post')
, $this->post
and Route::current()->parameters()['post']
should all return a post model. The first two return null, the latter returns a string.
Ah, okay. Tbh, you have a better chance of fixing this than I do at the moment. 😅 Don't use Laravel much these days, and not been a fan of model binding tbh, especially within validation (and I'm also on vacation). PRs are welcome.
Cool. I started hacking on it but need to get into the codebase a little more. I'll see what I can come up with.
I'm also with the same issue. Any luck here?
Hey @igorsgm I haven't looked at this for a while, but it was a sticking point for us.
Have you tried the solution @mortenscheel mentioned?
I managed to fix it by creating a custom bodyParameter strategy: GetFromFormRequestExtended
:
use Illuminate\Foundation\Http\FormRequest as LaravelFormRequest;
use Illuminate\Routing\Route;
use Knuckles\Camel\Extraction\ExtractedEndpointData;
use Knuckles\Scribe\Extracting\Shared\UrlParamsNormalizer;
use Knuckles\Scribe\Extracting\Strategies\BodyParameters\GetFromFormRequest;
use Knuckles\Scribe\Tools\Globals;
class GetFromFormRequestExtended extends GetFromFormRequest
{
public function __invoke(ExtractedEndpointData $endpointData, array $routeRules = []): ?array
{
return $this->getParametersFromFormRequestWithBinding($endpointData, $endpointData->route);
}
/**
* Overriding Knuckles\Scribe\Extracting\Strategies\GetFromFormRequestBase::getParametersFromFormRequest
* to bind the request to the route and set the route parameters properly.
*/
public function getParametersFromFormRequestWithBinding(ExtractedEndpointData $endpointData, Route $route): array
{
if (!$formRequestReflectionClass = $this->getFormRequestReflectionClass($endpointData->method)) {
return [];
}
if (!$this->isFormRequestMeantForThisStrategy($formRequestReflectionClass)) {
return [];
}
$className = $formRequestReflectionClass->getName();
if (Globals::$__instantiateFormRequestUsing) {
$formRequest = call_user_func_array(Globals::$__instantiateFormRequestUsing, [$className, $route, $endpointData->method]);
} else {
$formRequest = new $className;
}
// Set the route properly, so it works for users who have code that checks for the route.
/** @var LaravelFormRequest $formRequest */
$formRequest->setRouteResolver(function () use ($formRequest, $route, $endpointData) {
// Also need to bind the request to the route in case their code tries to inspect current request
$route = $route->bind($formRequest);
// ADDING THIS LINE TO SET ROUTE PARAMETERS
return $this->setRouteParametersAfterBinding($endpointData, $route);
});
$formRequest->server->set('REQUEST_METHOD', $route->methods()[0]);
$parametersFromFormRequest = $this->getParametersFromValidationRules(
$this->getRouteValidationRules($formRequest),
$this->getCustomParameterData($formRequest)
);
return $this->normaliseArrayAndObjectParameters($parametersFromFormRequest);
}
/**
* Dynamically sets route parameters for a given route based on URL parameters and method signatures.
* It assigns parameters using the first instance of type-hinted Eloquent models, the first case of enums,
* or a default example value.
*
* Note: Assumes model instances can be fetched with first() and enums are instantiated from their first case.
*/
protected function setRouteParametersAfterBinding(ExtractedEndpointData $endpointData, Route $route): Route
{
$typeHintedEloquentModels = UrlParamsNormalizer::getTypeHintedEloquentModels($endpointData->method);
$typeHintedEnums = UrlParamsNormalizer::getTypeHintedEnums($endpointData->method);
foreach ($endpointData->urlParameters as $urlParameter) {
$paramName = $urlParameter->name;
$routeParameter = match (true) {
array_key_exists($paramName, $typeHintedEloquentModels) => $typeHintedEloquentModels[$paramName]::first(),
array_key_exists($paramName, $typeHintedEnums) => $typeHintedEnums[$paramName]->getCases()[0]?->getValue(),
default => $urlParameter->example,
};
$route->setParameter($paramName, $routeParameter);
}
return $route;
}
}
Then, inside scribe.php
strategies:
'bodyParameters' => [
// Strategies\BodyParameters\GetFromFormRequest::class, --> Comment this line
GetFromFormRequestExtended::class, // Add your new custom strategy
Strategies\BodyParameters\GetFromInlineValidator::class,
Strategies\BodyParameters\GetFromBodyParamAttribute::class,
Strategies\BodyParameters\GetFromBodyParamTag::class,
],
After that, it worked as I intended.
Solution from @igorsgm above works, with one caveat (at least in our case). We also needed to disable endpoint URL normalisation using:
Scribe::normalizeEndpointUrlUsing(fn ($url) => $url);
This was because the route parameter was being normalised from the model name (eg: user
) to id
, which meant $typeHintedEloquentModels[$paramName]
was empty.
Another solution here would be to simply inject all $typeHintedEloquentModels
values as route parameters:
foreach ($typeHintedEloquentModels as $name => $model) {
$route->setParameter($name, $model::first());
}
... but I'm not sure if there would be other implications with that approach.