Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a command to print the application's routing table #27

Open
wants to merge 34 commits into
base: 2.9.x
Choose a base branch
from

Conversation

settermjd
Copy link

Q A
Documentation yes
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

This PR creates a new command that prints the application's routing table. For each route, it prints its name, path, middleware, and any additional options, in a tabular format, to the terminal.

The motivation for the change was wanting to quickly look up this information while developing Mezzio-based applications, but having to trawl through configuration files to find it. It would be easier to have a command that collated and printed the information to the terminal, helping to save time and avoid missing any listed routes.

@settermjd settermjd self-assigned this Jul 8, 2022
@settermjd settermjd changed the title DRAFT: Add routes command DRAFT: Add a command to print the application's routing table Jul 8, 2022
@froschdesign froschdesign marked this pull request as draft July 8, 2022 15:02
@froschdesign froschdesign changed the title DRAFT: Add a command to print the application's routing table Add a command to print the application's routing table Jul 8, 2022
@Ocramius
Copy link
Member

12fc77a feels out of place - please rebase instead

@settermjd
Copy link
Author

12fc77a feels out of place - please rebase instead

Could I get an assist with that?

@settermjd settermjd marked this pull request as ready for review September 2, 2022 19:52
settermjd and others added 20 commits September 2, 2022 21:53
This change adds the command and it's core description to the README,
and also adds an extended documentation section to the end of the
README, showing what the command does, the options it supports, and some
sample output. The tabular output isn't quite correct, but is there to
at least have a placeholder.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
This change adds the core classes, ListRoutesCommand and
ListRoutesCommandFactory, along with accompanying tests for them. At
this stage, the functionality is extremely limited, covering that the
command can be instantiated correctly, and has the correct
arguments/options, help output strings, command name, and description.
The rest of the functionality is coming.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
This change adds a class that extends the FilterIterator from PHP's SPL.
The intention of the class is to be able to filter the routes in an
application's routing table by one or more of their four core
properties/attributes, those being name, path, method(s), and
middleware.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
These two classes sort a traversable list of Route objects by both name
and path in ascending order.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
This change is quite simplistic, but renders the routing table in
tabular format if there are routes to render, otherwise it renders a
message saying that there are no routes available in the routing table.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
This change updates the code to check that, if a format is supplied,
that it's one of the two available choices, table and json. If it's not
either of those, then an error message is printed.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
The current options are by name and path. Other options may be added in
the future.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
- `ConfigDiscovery\ConfigAggregatorTest`
- `ConfigInjector\AbstractInjectorTestCase`
- `ConfigInjector\ConfigAggregatorInjectorTest`
- related test assets

Additionally, the Register/Deregister command tests needed to mark several test methods to run in separate processes and ignore global state to ensure that class overriding would work.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
- Marks internal classes and methods for suppression
- Identifies one `ArgumentTypeCoercion` in a test that is valid, and adds to the baseline.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Bumps [laminas/laminas-diactoros](https://github.com/laminas/laminas-diactoros) from 2.8.0 to 2.11.1.
- [Release notes](https://github.com/laminas/laminas-diactoros/releases)
- [Commits](laminas/laminas-diactoros@2.8.0...2.11.1)

---
updated-dependencies:
- dependency-name: laminas/laminas-diactoros
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
This, small, change enables the ListRoutesCommand by adding it and the
command's name (mezzio:routes:list) in the ConfigProvider's
getConsoleConfig() function.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
The intention here is to avoid having shortcuts that collide with
existing shortcuts, whether from other commands or from the core command
itself.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
After I added the command (in a bit of a hurry) I then noticed that the
command were added in alphabetic order (best way to do it). So this
change fixes up my initial ordering mistake.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
I don't know why I didn't do this originally, but there was no real need
to have a RouteCollector variable (that I know of) as a member variable.
All the application needs is an array of Route objects, as that's all it
iterates over. This change corrects the original oversight.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Again, I don't know why I sorted the routes later, when they could have
been sorted earlier before rendering them. This change corrects it. I'll
put it down to not fully appreciating what I was trying to achieve at
first.

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
.gitignore Outdated Show resolved Hide resolved
src/Routes/ListRoutesCommand.php Show resolved Hide resolved
src/Routes/ListRoutesCommand.php Outdated Show resolved Hide resolved
src/Routes/ListRoutesCommand.php Outdated Show resolved Hide resolved
src/Routes/ListRoutesCommand.php Outdated Show resolved Hide resolved
src/Routes/Sorter/RouteSorterByName.php Outdated Show resolved Hide resolved
src/Routes/Sorter/RouteSorterByName.php Outdated Show resolved Hide resolved
src/Routes/Sorter/RouteSorterByPath.php Outdated Show resolved Hide resolved
src/TemplateResolutionTrait.php Outdated Show resolved Hide resolved

class ListRoutesCommandFactoryTest extends TestCase
{
use ProphecyTrait;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use built-in mocks instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Sure. Will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule of thumb: do not use prophecy anymore. 😅

These classes were commented on during the PR which lead me to check and
remember that they're not used. Given that, this change removes them.
This is being removed after feedback from @Ocramius.
src/Routes/RoutesFilter.php Outdated Show resolved Hide resolved
src/Routes/RoutesFilter.php Outdated Show resolved Hide resolved
src/Routes/RoutesFilter.php Outdated Show resolved Hide resolved
src/Routes/RoutesFilter.php Outdated Show resolved Hide resolved
The change that this commit reverts was unintentionally made when
running php-cbf. This change restores the original functionality as PHP
7.4 is still supported by the library.Revert an unintentional change to
  TemplateResolutionTrait

  The change that this commit reverts was unintentionally made when
  running php-cbf. This change restores the original functionality as
  PHP 7.4 is still supported by the library.
@settermjd
Copy link
Author

I'm not sure how to finish the PR because of how the routes are typically loaded in an application generated with the Mezzio Skeleton. What I mean is that the routing table is typically loaded in public/index.php, using the following code:

(function () {
    /** @var \Psr\Container\ContainerInterface $container */
    $container = require 'config/container.php';

    /** @var \Mezzio\Application $app */
    $app = $container->get(\Mezzio\Application::class);
    $factory = $container->get(\Mezzio\MiddlewareFactory::class);

    // Execute programmatic/declarative middleware pipeline and routing
    // configuration statements
    (require 'config/pipeline.php')($app, $factory, $container);
    (require 'config/routes.php')($app, $factory, $container);

    $app->run();
})();

So, how should the code load the routes so that it can get access to the information?

@weierophinney
Copy link
Contributor

So, how should the code load the routes so that it can get access to the information?

Since commands are typically run in the project root, you can generally assume that you can load the routes file using require 'config/routes.php';. This file, and the config/pipeline.php file, returns an anonymous function, which accepts three arguments:

  • A Mezzio\Application instance
  • A Mezzio\MiddlewareFactory instance
  • The PSR-11 container instance

Once that function has been called, the Mezzio\Router\RouteCollector instance should be fully populated, and you can loop over its getRoutes() method to introspect the various Mezzio\Router\Route instances.

Your command will need to either accept all three of the above objects to its constructor, or you could have it accept just the container, and then pull the other two from the container when you are ready to create the routing table.

As a minimal example:

class RoutingTableCommand extends Commands
{
    public function __construct(private ContainerInterface $container)
    {
        parent::__construct();
    }

    protected function configure(): void
    {
         // setup command information here, such as options or arguments
    }

    protected function execute(InputInterface $input, OutputInterface $outptu): int
    {
        // create routing table
        (require 'config/routes.php')(
            $this->container->get('Mezzio\Application'),
            $this->container->get('Mezzio\MiddlewareFactory'),
            $this->container
        );

        $routes = $container->get('Mezzio\Router\RouteCollector');

        // iterate over the routes
        foreach ($routes->getRoutes() as $route) {
        }

        return 0;
    }
}

@boesing
Copy link
Member

boesing commented Sep 26, 2022

Can we make this configurable? We do have a routes.php in combination with the https://github.com/mezzio/mezzio/blob/3.12.x/src/Container/ApplicationConfigInjectionDelegator.php
If you now expect all routes.php to return a callables, that would be too specific. At least check if the config has the delegator registered and if so, do not require the routes.php.

edit: could be a routeloaderinterface which is either a "nooploader" or a "callables routes.php loader" for example.

@settermjd
Copy link
Author

Can we make this configurable? We do have a routes.php in combination with the https://github.com/mezzio/mezzio/blob/3.12.x/src/Container/ApplicationConfigInjectionDelegator.php If you now expect all routes.php to return a callables, that would be too specific. At least check if the config has the delegator registered and if so, do not require the routes.php.

edit: could be a routeloaderinterface which is either a "nooploader" or a "callables routes.php loader" for example.

Mind if we have a call to talk about this further?

@boesing
Copy link
Member

boesing commented Oct 7, 2022

Sure! I have to travel to munich this evening. Maybe we find some time later or on Sunday/next week?

@boesing
Copy link
Member

boesing commented Oct 13, 2022

Since we did not made a call yet, I took some time to looked into the actual PR.

IMHO, the command itself is properly implemented. Thats actually the exact same way as I would've done that. The commands factory receives the route collector. So thats already nice.

When I see this correct, we need the following tasks finished so that we can merge this:

  • getting rid of the psalm issues, some of them can be fixed by adding this psalm plugin
  • replacement of prophecy with mocks
  • proper namespaces (unit tests are failing as the namespace of some classes is incorrect)

Almost every check is red here, so maybe getting them green would be fine.


Another thing is - I already provided a command in laminas-cache. This command is kinda related to mezzio-router, so is there a reason why we do not put it in that library?

There are other commands in this library which do not properly fit into a specific laminas/mezzio component as they do not provide anything for a specific component (CreateHandler/CreateMiddleware do generate code for PSR-15, Migrate* commands are related to PSR-15 as well).

The Module stuff could be part of mezzio itself since we migrated this to laminas-cli.

Just curious - no need to change this right now, but I would prefer having stuff in the components they belong to. That would prevent having to install mezzio-tooling just for the route table command.

@boesing
Copy link
Member

boesing commented Oct 13, 2022

Oh, there comes to my mind, that there are multiple ways to configure the app to have routes...

This is another way:

(function () {
    /** @var \Psr\Container\ContainerInterface $container */
    $container = require 'config/container.php';

    /** @var \Mezzio\Application $app */
    $app = $container->get(\Mezzio\Application::class);
    $factory = $container->get(\Mezzio\MiddlewareFactory::class);

    // Execute programmatic/declarative middleware pipeline and routing
    // configuration statements
    (require 'config/pipeline.php')($app, $factory, $container);
    (require 'config/routes.php')($app, $factory, $container);

    $app->get('/i-am-route', [IAmRouteRequestHandler::class], 'i-am-route');

    $app->run();
})();

So how should we receive that route which is being added via $app->get()? Imho, the way how mezzio is "configurable" in that way, its impossible to provide a proper command for all kind of project configurations.

So to summarize, there are:

  1. applications using routes.php which returns an array (afair that was the way how expressive was configured in either its 1st version or in even older versions) and/or ConfigProvider (thats actually how we do it in our company, so we have both routes.php and some routes are part of ConfigProvider)
  2. applications using routes.php which returns a callable (consuming application, container, etc.)
  3. applications using ConfigProvider along with delegators for Application

IMHO, thats quite a lot of ways. Does anyone have an idea on how to be compatible with all these types of configurations?

@gsteel
Copy link
Member

gsteel commented Oct 14, 2022

FWIW, I normally setup routes by delegating around RouteCollector so that on the cli, if I need to generate routes, I can just grab the router from the container in the knowledge that all my routes are configured without needing to bootstrap the application - typically, I delete routes.php. In the past I've also used delegators on Application to inject routes

@boesing
Copy link
Member

boesing commented Oct 16, 2022

@gsteel thats exactly the point, there are multiple ways of providing routes.

Since it is impossible to actually catch index.php routes (be it callables router.php or direct Application#get), I would try to Focus on just loading the route collector.

I like the idea of this command, but I wouldnt pay attention to the bunch of ways to provide a route.

Adding some description to the command might be good enough. Routes can be still passed to a delegator if some1 wants to use the command.

@settermjd
Copy link
Author

Sorry about dropping this. Back on it now.

@settermjd
Copy link
Author

So, how should the code load the routes so that it can get access to the information?

Since commands are typically run in the project root, you can generally assume that you can load the routes file using require 'config/routes.php';. This file, and the config/pipeline.php file, returns an anonymous function, which accepts three arguments:

* A `Mezzio\Application` instance

* A `Mezzio\MiddlewareFactory` instance

* The PSR-11 container instance

Once that function has been called, the Mezzio\Router\RouteCollector instance should be fully populated, and you can loop over its getRoutes() method to introspect the various Mezzio\Router\Route instances.

Your command will need to either accept all three of the above objects to its constructor, or you could have it accept just the container, and then pull the other two from the container when you are ready to create the routing table.

As a minimal example:

class RoutingTableCommand extends Commands
{
    public function __construct(private ContainerInterface $container)
    {
        parent::__construct();
    }

    protected function configure(): void
    {
         // setup command information here, such as options or arguments
    }

    protected function execute(InputInterface $input, OutputInterface $outptu): int
    {
        // create routing table
        (require 'config/routes.php')(
            $this->container->get('Mezzio\Application'),
            $this->container->get('Mezzio\MiddlewareFactory'),
            $this->container
        );

        $routes = $container->get('Mezzio\Router\RouteCollector');

        // iterate over the routes
        foreach ($routes->getRoutes() as $route) {
        }

        return 0;
    }
}

Thanks for this, @weierophinney. Getting back in to this to bring it to a close.

@settermjd
Copy link
Author

@gsteel thats exactly the point, there are multiple ways of providing routes.

Since it is impossible to actually catch index.php routes (be it callables router.php or direct Application#get), I would try to Focus on just loading the route collector.

I like the idea of this command, but I wouldnt pay attention to the bunch of ways to provide a route.

Adding some description to the command might be good enough. Routes can be still passed to a delegator if some1 wants to use the command.

So, it seems that the command needs to be configurable by the user, so that they tell it where to look?

@Xerkus Xerkus changed the base branch from 2.6.x to 2.9.x April 30, 2023 19:54
@boesing
Copy link
Member

boesing commented Apr 30, 2023

@settermjd no I would simply load the route collector and extract all known methods from there. if a project adds routes via index.php, these projects cant use this command until they move routes to configuration or to delegators. But let that be the problem of those projects. Id say focus on runtime extraction.

As the project doesn't use it anymore, it's time to remove it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants