dingo / api

A RESTful API package for the Laravel and Lumen frameworks.
BSD 3-Clause "New" or "Revised" License
9.33k stars 1.25k forks source link

FormRequest $request->validated() doest not work in new dingo version #1643

Closed narmiel closed 4 years ago

narmiel commented 5 years ago
Q A
Bug? yes
New Feature? no
Framework Laravel
Framework version 5.7
Package version 2.2.3
PHP version 7.2

Actual Behaviour

I tried to upgrade package from 2.0.0-alpha2 to ^2.2.3 Since 2.0.0-alpha2 class Dingo\Api\Http\FormRequest do not inherited from Illuminate\Foundation\Http\FormRequest

Dingo\Api\Http\FormRequest doesnt have more field validator method setValidator etc

Expected Behaviour

For example $request->validated() should return same as befofe (like in laravel) all validated data, but new dingo version doesn't have this method

Steps to Reproduce

call $request->validated(); where $request any class which extend Dingo\Api\Http\FormRequest

Possible Solutions

dunno :) but i cant upgrade package now

specialtactics commented 5 years ago

Hi there @narmiel

There was a PR a long time ago actually that caused this problem I think https://github.com/dingo/api/pull/1542

I will have a look about adding that function to the FormRequest in Dingo - however as a workaround, you could potentially just use Laravel's FormRequest for now.

fbingha commented 5 years ago

You can also get the validator instance which will then allow you access to the validated() function.

$yourRequest->getValidatorInstance()->validated();

kisexu commented 5 years ago

You can also get the validator instance which will then allow you access to the validated() function.

$yourRequest->getValidatorInstance()->validated();

getValidatorInstance does not exist my dingo/api version is 2.3 , laravel version is 6.0

jourdon commented 5 years ago

Is the problem still unsolved?

mlshvdv commented 4 years ago

It seems that still unsolved

reallyli commented 4 years ago

It seems that still unsolved

specialtactics commented 4 years ago

Hi, this problem can be easily solved by using Illuminate\Foundation\Http\FormRequest instead of the Dingo FormRequest.

I will close this for now, but continue looking at the two from requests and see what needs to be done.

christoph-kluge commented 4 years ago

@specialtactics what a coincidence. I was looking at this case right now.

I did a diff on the Dingo FormRequest and Laravel's FormRequest and saw the following things:

Do you think it makes sense use adapters for the FormRequest? I'm up for ideas and willing to contribute here.

specialtactics commented 4 years ago

Hey @christoph-kluge , at this point I don't understand/see what is the point of using Dingo's FormRequest over Laravel's FormRequest.

If someone can please outline me a use-case for this, that would be great, otherwise, please use Laravel's FormRequest as it works perfectly with Dingo's router (as far as I can see).

christoph-kluge commented 4 years ago

The only reason I can imagine right now is to keep compatibility for lumen. Lumen itself does not contain a FormRequest.

If you want to use it with lumen you got to pull the whole laravel framework because the FormRequest is part of foundation and not illuminate/http.

I‘m not a lumen-user that‘s why my initial thought was as well to eliminate the need of FormRequest until I realized that dingo supports lumen too.

specialtactics commented 4 years ago

Right, what's weird I guess is that this issue is for laravel not lumen.

I'll keep that in mind, but since I don't work with Lumen myself, someone else will need to do a PR to fix that up (if they want).

christoph-kluge commented 4 years ago

@specialtactics from what I see it's because the existing FormRequest does not extend the Laravel FormRequest. This means that the Dingo FormRequest and the Laravel FormRequest needs synced every now and then (as of now it's outdated because the method is missing inside Dingo).

Another idea: How about adding a deprecated message for the laravel users so they get error-messages and can gracefully update their installations to use the original one and remove it at a later stage or throw an exception so developer's get direct feedback?

$this->app->resolving(FormRequest::class, function (FormRequest $request, Application $app) {
  trigger_error(FormRequest::class . ' is deprecated for laravel. Please use the Laravel's FormRequest instead.', E_USER_DEPRECATED);

  $this->initializeRequest($request, $app['request']);

  $request->setContainer($app)->setRedirector($app->make(Redirector::class));
});
specialtactics commented 4 years ago

I think if we were to do that, we would technically need to tag 3.0.0, because we can't do anything to break existing functionality (even though it is to some degree already broken I guess...).

I agree though in the case of Laravel we should do something, maybe a deprecated tag on formrequest, rename it to lumenformrequest or put it under a lumen dir, and maybe a log::warning or something ?

Any other ideas?