Fix artisan routes Output

[hskrasek]

I noticed after changing my api routes to use Route::api(), that when I ran artisan routes those routes were gone. So did some poking around, and I believe I came up with a solution that resolves the issue.

Tested it in one of my projects, and artisan routes is returning the correct input.

Hopefully the test I added adequately covers the change, if not let me know and I can add another test or two.. after I think of them.

[jasonlewis]

What happens if there is a duplicate route in both the API routes and the default routes?

Say, for example:

$this->router->api(['version' => 'v1', 'prefix' => 'api'], function() { $this->router->get('foo', function() { return 'bar'; }); });
$this->router->get('foo', function() { return 'baz'; });

Do both routes still show up in artisan routes?

[hskrasek]

No, if you were to do the above, you would only get a single route. Is such a thing allowed? If you made the route foo as an API route and a regular route, wouldn't your router always route to the API version anyway?

Also it looks like if you do the following:

$this->router->get('foo', function() { return 'baz'; });
$this->router->get('foo', function() { return 'baz'; });
$this->router->get('foo', function() { return 'baz'; });
...

You would only get a single route

[jasonlewis]

The API is able to have the same URIs as regular routes. That's why they are registered in their own route collections. My above example has a prefix for the API routes meaning that you'd hit it on /api/foo, but it would still be registered as foo in the collection, which is why it's overwriting the other route.

I've been thinking about this and I was thinking of just adding an artisan api:routes command. I haven't looked much into it though. But I think the idea of having a separate command for your API routes would be fine.

[hskrasek]

I didn't notice the prefix in your example, my bad. I would have to test to see what happens in that scenario. As for the artisan api:routes command, if I were to create that, do you want a separate PR, or could I tack it onto this one?

[hskrasek]

Also a quick test show that your example above would infact still return both routes. I can adjust the unit test to match your example, if that's the result you're intending.

[jasonlewis]

And it shows up in artisan routes?

[hskrasek]

Correct, you would get two different listings after running artisan routes

[jasonlewis]

Okay cool! I'll have a play with this shortly and will then merge. 😄 Cheers.

[hskrasek]

Sounds good! As for that artisan api:routes command, I think it would still be a good idea, with maybe changed output to show the versions the routes apply to, protected, etc. So if I get time tonight or tomorrow, you might see another PR

[jasonlewis]

Hm, there are still going to be problems with this so I think we should just have an api:routes command.

The biggest problem I've seen after checking it out is that if I have the following it will only register the last route.

Route::api(['version' => 'v1', 'prefix' => 'api'], function()
{
    Route::get('users', 'Api\V1\UsersController@index');
});

Route::api(['version' => 'v2', 'prefix' => 'api'], function()
{
    Route::get('users', 'Api\V2\UsersController@index');
});

If you run artisan routes it only shows the second route (Api\V2\UsersController@index). This isn't a huge deal but it is misleading as it's not showing all the registered API routes. I'm going to close this for now but feel free to have a crack at the api:routes command and send a PR over.

[hskrasek]

Yea I realized this last night as I was getting ready to passout. I was trying to maintain the functions original function of returning a RouteCollection, but in cases like the above, it results in duplicates not showing up.

I'll mess around with the idea of the api:routes command and see what happens.

[jasonlewis]

@hskrasek Have you had a chance to look at a new command yet? Otherwise I'm going to look at throwing something together later. Cheers.

[hskrasek]

Very briefly, but I'm starting my last week of finals this week. Then I'll be done with undergrad and be able to do more of this stuff! I'll try to mess around with it tomorrow, if not for sure Wednesday afternoon.

---Hunter Skrasekhunterskrasek@me.com

On May 4, 2014 at 10:22:25 PM CDT, Jason Lewis notifications@github.com wrote:@hskrasek Have you had a chance to look at a new command yet? Otherwise I'm going to look at throwing something together later. Cheers. —Reply to this email directly or view it on GitHub.