Api: Internal routing bug: doesn't work with middleware

[assembledadam]

Any internal request in conjunction with a route that contains middleware, like this:

$api->version('v1', ['middleware' => 'test'], function ($api) {

});

Results in a:

[ReflectionException]      
  Class test does not exist 

Despite it being define properly inside Kernel.php:

 protected $routeMiddleware = [
        'test' => \App\Http\Middleware\Test::class,
        'auth' => \App\Http\Middleware\Authenticate::class,
        'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
        'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class,
        'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
    ];

I've created a test case to illustrate the bug here https://github.com/assembledadam/laravel

Just run php artisan testcase in the root folder, and to view the middleware working visit localhost/index.php/test/blahblah

Stack trace (from my application, not the test case!):

n Error Occured:
Class oauth does not exist
File: /usr/www/app/vendor/laravel/framework/src/Illuminate/Container/Container.php
Line: 738
#0 /usr/www/app/vendor/laravel/framework/src/Illuminate/Container/Container.php(738): ReflectionClass->__construct('oauth')
#1 /usr/www/app/vendor/laravel/framework/src/Illuminate/Container/Container.php(633): Illuminate\Container\Container->build('oauth', Array)
#2 /usr/www/app/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(697): Illuminate\Container\Container->make('oauth', Array)
#3 /usr/www/app/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(123): Illuminate\Foundation\Application->make('oauth')
#4 [internal function]: Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Dingo\Api\Http\InternalRequest))
#5 /usr/www/app/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(32): call_user_func(Object(Closure), Object(Dingo\Api\Http\InternalRequest))
#6 /usr/www/app/vendor/dingo/api/src/Http/Middleware/PrepareController.php(45): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Dingo\Api\Http\InternalRequest))
#7 [internal function]: Dingo\Api\Http\Middleware\PrepareController->handle(Object(Dingo\Api\Http\InternalRequest), Object(Closure))
#8 /usr/www/app/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(124): call_user_func_array(Array, Array)
#9 [internal function]: Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Dingo\Api\Http\InternalRequest))
#10 /usr/www/app/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(32): call_user_func(Object(Closure), Object(Dingo\Api\Http\InternalRequest))
#11 [internal function]: Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Dingo\Api\Http\InternalRequest))
#12 /usr/www/app/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): call_user_func(Object(Closure), Object(Dingo\Api\Http\InternalRequest))
#13 /usr/www/app/vendor/laravel/framework/src/Illuminate/Routing/Router.php(726): Illuminate\Pipeline\Pipeline->then(Object(Closure))
#14 /usr/www/app/vendor/laravel/framework/src/Illuminate/Routing/Router.php(699): Illuminate\Routing\Router->runRouteWithinStack(Object(Illuminate\Routing\Route), Object(Dingo\Api\Http\InternalRequest))
#15 /usr/www/app/vendor/laravel/framework/src/Illuminate/Routing/Router.php(675): Illuminate\Routing\Router->dispatchToRoute(Object(Dingo\Api\Http\InternalRequest))
#16 /usr/www/app/vendor/dingo/api/src/Routing/Adapter/Laravel.php(80): Illuminate\Routing\Router->dispatch(Object(Dingo\Api\Http\InternalRequest))
#17 /usr/www/app/vendor/dingo/api/src/Routing/Router.php(574): Dingo\Api\Routing\Adapter\Laravel->dispatch(Object(Dingo\Api\Http\InternalRequest), 'v1')
#18 /usr/www/app/vendor/dingo/api/src/Dispatcher.php(540): Dingo\Api\Routing\Router->dispatch(Object(Dingo\Api\Http\InternalRequest))
#19 /usr/www/app/vendor/dingo/api/src/Dispatcher.php(445): Dingo\Api\Dispatcher->dispatch(Object(Dingo\Api\Http\InternalRequest))
#20 /usr/www/app/vendor/dingo/api/src/Dispatcher.php(372): Dingo\Api\Dispatcher->queueRequest('post', 'users', Array, '{"old_id":"301"...')
#21 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/Scripts/User.php(240): Dingo\Api\Dispatcher->post('users', Array, '{"old_id":"301"...')
#22 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/Scripts/AbstractMigration.php(178): Candybanana\CreoMigration\ETL\Scripts\User->insert(Array)
#23 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/Scripts/User.php(68): Candybanana\CreoMigration\ETL\Scripts\AbstractMigration->run(NULL, '114947')
#24 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/ETL.php(348): Candybanana\CreoMigration\ETL\Scripts\User->__construct(Object(Candybanana\CreoMigration\ETL\ETL), '0adf04fce0090b6...')
#25 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/ETL.php(262): Candybanana\CreoMigration\ETL\ETL->runEntityScripts(Array)
#26 /usr/www/app/vendor/candybanana/creo-migration/src/ETL/ETL.php(91): Candybanana\CreoMigration\ETL\ETL->run()
#27 /usr/www/app/vendor/candybanana/creo-migration/src/Console/Command/Migrate.php(87): Candybanana\CreoMigration\ETL\ETL->__construct(Array, Object(Candybanana\CreoMigration\Console\Command\Migrate), Object(PDO))
#28 [internal function]: Candybanana\CreoMigration\Console\Command\Migrate->fire()
#29 /usr/www/app/vendor/laravel/framework/src/Illuminate/Container/Container.php(507): call_user_func_array(Array, Array)
#30 /usr/www/app/vendor/laravel/framework/src/Illuminate/Console/Command.php(169): Illuminate\Container\Container->call(Array)
#31 /usr/www/app/vendor/symfony/console/Command/Command.php(259): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#32 /usr/www/app/vendor/laravel/framework/src/Illuminate/Console/Command.php(155): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#33 /usr/www/app/vendor/symfony/console/Application.php(844): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#34 /usr/www/app/vendor/symfony/console/Application.php(192): Symfony\Component\Console\Application->doRunCommand(Object(Candybanana\CreoMigration\Console\Command\Migrate), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#35 /usr/www/app/vendor/symfony/console/Application.php(123): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#36 /usr/www/app/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(107): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#37 /usr/www/app/artisan(36): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#38 {main}

[cariboufute]

I confirm this. I have the same problem when I run functional tests while calling routes with middleware 'api', which is a middleware from the Laravel framework. It was working before the update.

Here is the error log.

1) Tests\Functional\Http\Controllers\Api\AuthenticateControllerTest::testAuthenticate
ReflectionException: Class api does not exist

/home/vagrant/Code/rms-php-api/vendor/laravel/framework/src/Illuminate/Container/Container.php:738
/home/vagrant/Code/rms-php-api/vendor/laravel/framework/src/Illuminate/Container/Container.php:633
/home/vagrant/Code/rms-php-api/vendor/laravel/framework/src/Illuminate/Foundation/Application.php:697
/home/vagrant/Code/rms-php-api/vendor/dingo/api/src/Http/Middleware/Request.php:155
/home/vagrant/Code/rms-php-api/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:155
/home/vagrant/Code/rms-php-api/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:508
/home/vagrant/Code/rms-php-api/tests/Functional/Http/Controllers/Api/AuthenticateControllerTest.php:27

The AuthenticateControllerTest raising the error

class AuthenticateControllerTest extends TestCase
{
    public function setUp()
    {
        parent::setUp();
        Artisan::call('migrate:refresh');
        $this->seed();
    }

    public function testAuthenticate()
    {
        $user = User::first();
        $credentials = ['email' => $user->email, 'password' => $user->email];
        $response = $this->call('POST', 'api/auth', $credentials);
        $content = json_decode($response->content());

        $this->assertSame(200, $response->getStatusCode());
        $this->assertNotNull($content->token);
    }
}

[cariboufute]

P.S.: Using the dev-master#3062aac74327d589e0fed92d276225ee7aaab377 commit for dingo/api in composer.json removes the bug.

[jasonlewis]

Seems to have been an ordering of providers issue.

I've pushed a fix which should eliminate this so you can add the provider wherever you want in the app.php config file.

Please update and let me know.

[assembledadam]

Hi Jason - using 9c28813049bbc8bb3a62f60e9199f398e0845264 and still getting the problem on both my app and the testcase.

I verified the code changes were in my local vendor dir.

[stueynet]

Agreed with @assembledadam the latest build didn't fix it. The quick fix is to move it above RouteServiceProvider in app.php:

        Dingo\Api\Provider\LaravelServiceProvider::class,
        App\Providers\RouteServiceProvider::class,

[jasonlewis]

Aye, register Dingo first. I'll take another look at this tomorrow.

[cariboufute]

I confirm the quick fix that @stueynet gave. All my tests pass with his fix but I have the same reported error when I keep the former order of providers.

[assembledadam]

Yep, definitely the ordering of providers that's the problem. Reordering seems to have no adverse effects so far.

[jasonlewis]

Alright so my plan is to re-work the "cloning" of the Laravel router, which is the root cause of all these other issues.

The idea behind the clone was to fix an issue with the internal dispatcher screwing up the current route on the router instance.

But I can possibly work around that. So, for now, stick with placing the provider first (which is usually how I do it, but order shouldn't matter), and I'll get back to you all with what I can come up with.

[Luddinus]

I'm adding middlewares dynamically and I get the error, even I have DIngoProvider first...

I can't fix the problem, I'll trye to downgrading laravel...

[catalinux]

How do you add these middlewares dinamically? In the last version there is a config in dingo for global middlewares

[Luddinus]

@catalinux

In a service provider I "inject" the illuminateRouter and:

$router->middleware($middlewareKey, $middlewareRouter);

What do you mean with that config?

[Luddinus]

Anyway, I fix in my case downgrading to b36db17 commit (dec 2015)...

[jasonlewis]

Yeah hopefully resolve those issues as well with upcoming changes. Stay tuned, I'll update here when I've got something.

[jasonlewis]

Okay folks can someone try upgrading to latest dev-master changes and letting me know if this is resolved?

I've reverted (what I believe to be) the root cause of the issue which was cloning of the Laravel router which prevented middlewares and bindings to be registered on the router when dispatching routes.

At the moment cloning is now done prior to dispatching the request and then the cloned instance is unset. This is temporary. I'm hoping I can re-work the internal dispatcher itself so that the router does not need to be cloned at all.

[Luddinus]

Well, in my case I don't get the error of "Class 'middleware-class' doesn't exists'.

But I do get sometimes "Maximum function nesting level of '200' reached". It didn't happen before.

[assembledadam]

It's working with that fix, although the cloning seems pretty nasty. As you say hopefully it can be re-worked to not be necessary.

Happy for you to close this and open a new ticket/feature request for improving the fix to this one.

But I think i've discovered another bug (seems to be a habit of mine!) which i'm trying to get to the bottom of. Basically I'm making a request the the same endpoint with different input vars (which completely changes the output).

Problem is, the second request to the same endpoint isn't even reaching my API controller. It just spits back out the same response as the prior request, almost like it's cached. I've checked that it 100% isn't any kind of cache I've implemented. It also isn't related to the fix you just applied. Stumped so far.

Trying to replicate it in a test case but having trouble at the moment.

(sorry accidentally closed - reopened).

Update: Figured it out. I didn't realise Laravel core 'caches' the controller instances (via the IOC Container's make()) for subsequent requests - so anything in your controller (and dependancies of that controller) cannot be unique to that request.

[jasonlewis]

Where does Laravel do that? And you mean for subsequent requests during the same initial request, right?

[assembledadam]

I mean subsequent requests indeed. I believe I tracked it down to Illuminate\Routing\ControllerDispatcher::makeController() and within there you've got $this->container->make($controller);

Just means anything you set in the constructor of your controllers is obviously only called upon on the first initialisation/call - but with Dingo's internal requests, you may well make more than that.

In my case this came to light as I was processing some request inputs in the constructor, and the second request inputs were different to the first, and without a call to 'refresh' the state of those inputs in my app it meant the inputs were stale. Fix/workaround was simply to move out of the constructor, but it certainly isn't as clean with the constructor being the only real 'boot' method of a controller.

It's a bit of a gotcha, but I don't know if I'd call this a bug (and nothing to do with this issue, sorry!). But having said that I imagine users may expect that their controllers are initialised for each internal request, but if I'm the first person to moan, perhaps not!

[jasonlewis]

I actually think this is more because Dingo is storing the controller instance to prevent it from being instantiated multiple times. In fact it was you that brought this to my attention: #922

[assembledadam]

That would be amazing and a little schizophrenic on my part! But that was actually different - that was a single request hitting the controller twice. This is two completely separate requests hitting the controller only once collectively.

[jasonlewis]

Hm, two completely separate requests shouldn't know anything about the other request, meaning the controller _should_ be instantiated again as nothing is "stored" between requests.

You did mention internal requests though, which, if that's the case then yes it will only be instantiated once.

If you need to you could force the instance to be forgotten by the container.

app()->forgetInstance('Your\Controller\Here');

[jasonlewis]

This is an interesting point though and might be worth baking in...

[assembledadam]

I absolutely think it would be worth baking it in - I don't think it's intuitive for a user to have to manually clear out the IOC for internal requests.

As an example, in my case this was being used for a long-running ETL command line script, and I was hitting the same endpoint with different input vars like this:

$categories = \API::header('Content-Type', 'application/json')
                        ->get('media/categories', ['site_id' => 1, 'type' => 'download']);

And then in a later script (same process)

$categories = \API::header('Content-Type', 'application/json')
                        ->get('media/categories', ['site_id' => 1, 'type' => 'video']);

_(Using Dingo's API facade of course)_

With the 'type' input being processed in the controller constructor. Was a bit of a head-scratcher for me when I was getting 'download' categories in my second request!

[assembledadam]

This is causing me pain again, now I'm certain that this is something that the lib would benefit from being baked in :)

[assembledadam]

This was causing me massive grief - this seems to do the trick (would be nice to have a better reference to the container though, but it's 1.38am right now :)

[Casperhr]

I can confirm this issue. running a older version from april. Is this fixed ?

[assembledadam]

The middleware bug should be fixed yes - the controllers being reinitialised between requests is still not merged I believe, check the PR related to this issue.

[specialtactics]

Hey guys, would anyone care to comment on this issue?

Is anyone still having it?

[cariboufute]

Sorry, been a while I am not working on the project using Dingo, so can't help here.

[specialtactics]

I will close due to lack of response then, if anyone still has this issue - please make a new issue using the new template.