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

Pass a route configuration file #17

Closed
NigelGreenway opened this issue Feb 9, 2015 · 17 comments
Closed

Pass a route configuration file #17

NigelGreenway opened this issue Feb 9, 2015 · 17 comments

Comments

@NigelGreenway
Copy link

Hi,

I have been using your fastroute extension and am enjoying it very much.

I was wondering if you would be interested in a Pull Request which adds a method to pass a configuration array?

    /**
     * Add a collection of routes from a configuration
     * array
     *
     * @param  array $routes
     * @param  \League\Route\Strategy\StrategyInterface $strategy
     * @throws \League\Route\Exception\EmptyRoutingPassedException
     * @throws \League\Route\Exception\MalformedRouteConfigurationException
     */
    public function addRoutesFromConfig(
        array $routes = [],
        Strategy\StrategyInterface $strategy = null
    ) {
        // Check if the array is empty
        if (true === empty($routes)) {
            throw new EmptyRoutingPassedException;
        }

        // Loop through each modules routing data
        foreach ($routes as $module => $route) {
            foreach($route as $name => $body) {
                // Check configuration is passed correctly
                if (
                    false === isset($body['method'])
                    || false === isset($body['pattern'])
                    || false === isset($body['controller'])
                ) {
                    throw new MalformedRouteConfigurationException(
                        sprintf(
                            'The route [%s] in the [%s] section has not been configured correctly',
                            $name,
                            $module
                        )
                    );
                }

                $this->addRoute($body['method'], $body['pattern'], $body['controller'], $strategy);
            }
        }
    }

and then called like:

$routes = [
    'demo_page' => [
        'pattern'    => '/demo/{name}',
        'controller' => 'Demo\Controller\DemoController::index',
        'method'     => 'GET',
    ],
];


$router = new RouteCollection();
$router->setStrategy(new RequestResponseStrategy);
$router->addRoutesFromConfig($routes);

In terms of the exceptions, not sure if they are the best place for them as they are not Http exceptions but more argument exceptions?

@philipobenito
Copy link
Member

This is something I'd be interested in adding, it needs more discussion around the format of the configuration array though as there is the possibility of adding named routes in the future but it's not something currently implemented.

Also we have to take per route configuration in to account for strategies.

Your example has the knowledge of modules/sections which I also like but again would need discussion around planned future features. I plan to release a roadmap at some point soon so would you be happy to pick this back up with me once I've finalised that?

@NigelGreenway
Copy link
Author

Yeah, I would be more than happy.

In terms of the named routes part, is it more that I am assuming that everyone would be using named routes? If that is the case, what is the cost of having 2 methods?

$router->addNamedRoutesFromConfig($routes);
// --or--
$router->addRoutesFromConfig($routes);

@philipobenito
Copy link
Member

It will definitely be an optional thing, having several conversations about the benefits of named routes before any decision is made as I don't personally see how I would use them in my own projects that would benefit me over what I'm doing now. I'd love ALL input I can get on that and any code examples of how it would ideally work for people if you have any input?

@NigelGreenway
Copy link
Author

So, last night I was thinking and I have come up with an idea on how it could be done...

This takes named routes into consideration

Instead of having two functions, we could have a function takes three parameters:

  • array $routes | default: []
  • boolean $namesRoutes | default: false
  • StrategyInterface $strategy | default: null

It is a very quick and dirty implementation of my idea but serves its basic purpose

example

I am implementing this into my own micro framework for learning purposes and would be good for me for the following:

The reason for a route config for me would be the ability to separate have them in one file along with separating them into several configs (as per the example). It also allows for developers to go to one place on a project.

The reason for named parameters would be the ability to create a url parser (similar to the path() functionality in twig) for a php, mustache or twig template allowing me to edit routes easily by changing the parameter requirements or doing a search and replace across the project. I can search for every instance of route article_by_category_and_year, if this rendered as /articles/{{ article.category }}/{{ article.year }} then I can use the route name to search on.

@patrickheeney
Copy link

👍 Couldn't you fit all options into an array:

$routes = [
    [
        'pattern' => '/demo/{name}',
        'controller' => 'Demo\Controller\DemoController::index',
        'method' => 'GET',
        'name' => ''demo_page',
        'strategy' => 'FQNS\CustomStrategy'
    ],
];

@philipobenito
Copy link
Member

@patrickheeney yes, this is planned in the near future for v2.

@patrickheeney
Copy link

@philipobenito Sounds good. Was just adding my feedback from your suggestion:

I'd love ALL input I can get on that and any code examples of how it would ideally work for people if you have any input?

@philipobenito
Copy link
Member

Yes, mentioned on the other thread, apologies if I ever appear to be curt, it's generally just jumping on and and addressing issues and discussions in the middle of something else so I get straight to my point, don't want that to discourage you from contributing with discussion and suggestions.

@sagikazarmark
Copy link
Member

👍

@philipobenito
Copy link
Member

@NigelGreenway I'm building a Service Provider to go with version 2.0.0 that will allow use of config. Are you happy with that as a resolution to this?

@NigelGreenway
Copy link
Author

Hi @philipobenito,

Sorry, had my email app closed.

A service provider would be great, cheers.

I would then be able to create the config as I see fit and then create a service provider to manage it?

If you need any help with anything like testing, let me know. Will be glad to help where I can.

@philipobenito
Copy link
Member

@NigelGreenway yup, taking a new tact with config and handling it with bootable service providers in v2 of container, that way there's no real defined structure and anybody can build their own service provider to handle it in a different way if they feel they want to/need to. i.e. migrations from frameworks etc.

So if you're happy with that I'll close this and consider it resolved by that.

@NigelGreenway
Copy link
Author

@philipobenito Yeah, more than happy. Cheers

👍

@kevinsmith
Copy link

To my knowledge, this concept didn't get fleshed out, and at least as of v4 it's not possible to reference an external routes.php file. Mind providing a little insight into the trouble implementing this concept? I'm curious about whether it might be possible in the future.

@maiorano84
Copy link

@kevinsmith I know I'm responding over a year later, but I just found this discussion.

I have an implementation I'm actually currently putting together using Container and a YAML file. Seems to work pretty well so far, but I haven't built out any unit tests yet. Is this something you're still interested in? If so, I can work on decoupling this from my personal implementation and getting a quick Composer package up.

@kevinsmith
Copy link

@maiorano84 I ended up going with a different routing solution, but I’m sure it would make a good reference for someone if you wanted to post it.

@ParadiseStudios
Copy link

I am currently working on a yaml configuration parser as well, so it is a feature at least a couple of us are looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants