hotmeteor / spectator

OpenAPI testing for PHP
MIT License
285 stars 52 forks source link

SpectatorServiceProvider prependMiddlewareToGroup causing problems after upgrading to 1.6.0.1 #116

Closed lsfiege closed 2 years ago

lsfiege commented 2 years ago

Hello!

After upgrading from v1.5.1 to v1.6.0.1 I'm experincing an error indicating Array to string conversion coming from a custom Route::bind directive in my RouteServiceProvider. The error it's caused in the findOrFail() method of my Route::bind directive, but I'm not quite sure why this is happening only after upgrading spectator.

I'm not sure how I can reproduce the scenario in this package to send you more information, but I suspected that it's probably related to the middleware chaining, so I changed the prependMiddlewareToGroup to appendMiddlewareToGroup in the SpectatorServiceProvider (Line 38) and everything worked again.

Is this a bug?

hotmeteor commented 2 years ago

Hm, interesting. I've not seen that before.

Would you be able to share the part of your spec that triggers this, as well as your test?

lsfiege commented 2 years ago

Sorry, cannot share the exact code but can adapt it to illustrate the situation:

I have a custom bind for my routes in the RouteServiceProvider

Route::bind('user', function ($value) {
    return User::where('name', $value)->firstOrFail();
});

If I add a dump line to inspect the received value prior to the return of the bind callback I can see that the request it's being executed twice, and the received $value in the callback have the correct type and value in the first time, but in the second it's an object representing the retrieved model

Route::bind('user', function ($value) {
    dump(gettype($value));
    return User::where('name', $value)->firstOrFail();
});

image

Tracing more over the exception I can see that the Array to string conversion error comes from the Laravel Eloquent builder, but IMHO the issue can be related to the changes made to the Middleware of spectator image

This issue happened only after a manual upgrade of Spectator (composer update hotmeteor/spectator). Maybe this bug is related to #118 ? because also I started to have failing tests that hit endpoints with unique validation rules for some request body params

lsfiege commented 2 years ago

Update: Tested manually the fixes introduced with #123 and the fix proposed with the open PR #113 and the issue is solved (only after applying the change of #113). So I will close this issue since the PR solves this

hotmeteor commented 2 years ago

OK so I understand, you're saying #113 needs to be merged and then your issue is resolved?

lsfiege commented 2 years ago

Yup, that's correct. Feel free to leave this issue as opened if you need it.