Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Chain route #3999

Closed
wants to merge 4 commits into from
Closed

Chain route #3999

wants to merge 4 commits into from

Conversation

DASPRiD
Copy link
Member

@DASPRiD DASPRiD commented Mar 10, 2013

This new route type allows to glue multiple routes together under a single route. This allows to address them with a single name. Common use-cases are:

  • Connecting a path route with a scheme route (forcing HTTPS)
  • Connecting a path route with a wildcard route
  • Basically any case where you have "may_terminate = false" on the parent and only a single child

Here is a simple example of how the feature can be used:

array(
    'routes' => array(
        'login' => array(
            'type' => 'literal',
            'options' => array(
                'route'    => '/login',
                'defaults' => array(/* … */)
            ),
            'chain_routes' => array(
                array(
                    'type' => 'scheme',
                    'options' => array(
                        'scheme' => 'https'
                    )
                )
            )
        )
    )
);

The chains are processed in FIFO order. In the above example, the 'login' route may also have child_routes, in which case the base route of that branch becomes the combined chain of the literal and the scheme route.

The route and factory is designed that way that it allows other modules to easily add conditions (like HTTPS) on a login route without changing the assembling structure.

This PR is so far feature complete, but yet lacks unit tests.

@macnibblet
Copy link
Contributor

Could you by any chance give a prolonged example ?

@DASPRiD
Copy link
Member Author

DASPRiD commented Mar 11, 2013

@macnibblet Sure, take for instance the following route configuration. It will force all shop routes (both the parent and all subsequent children) to use the HTTPS scheme:

array(
    'routes' => array(
        'shop' => array(
            'type' => 'literal',
            'options' => array(
                'route'    => '/shop',
                'defaults' => array(/* … */)
            ),
            'chain_routes' => array(
                array(
                    'type' => 'scheme',
                    'options' => array(
                        'scheme' => 'https'
                    )
                )
            ),
            'may_terminate' => true,
            'child_routes' => array(
                'products' => array(
                    'type' => 'segment',
                    'options' => array(
                        'route' => '/product/:productId',
                        'defaults' => array(/* … */)
                    )
                )
            )
        )
    )
);

@DASPRiD
Copy link
Member Author

DASPRiD commented Mar 11, 2013

Additional information via IRC discussion:

[13:40] <mac_nibblet> DASPRiD: maybe im being abit stupid i dont really see the benifite of it ?
[13:40] <mac_nibblet> DASPRiD: you could just start the shop route with a scheme ?
[13:40] <DASPRiD> well, first of, that would leave you with a route like e.g. "secure/shop/product"
[13:41] <DASPRiD> so adding/removing that scheme bit would always mean that you have to change all your assemble calls
[13:41] <mac_nibblet> Oki, that sounds logical
[13:41] <DASPRiD> secondly, this allows you to add such bits to another moduels route
[13:41] <DASPRiD> without changing those assembling rules
[13:42] <DASPRiD> e.g. you are using zfcuser
[13:42] <DASPRiD> but you want the login route to be https
[13:43] <DASPRiD> then you simply add that chain_routes part in your config, extending the zfcuser config
[13:43] <DASPRiD> and all routes keep working :)
[13:43] <mac_nibblet> yeap thats a nice one
[13:43] <mac_nibblet> but apart from scheme then :P?
[13:44] <DASPRiD> mac_nibblet, i named others in the description
[13:44] <DASPRiD> same is also valid for method route for instance
[13:44] <DASPRiD> or a segment route and a wildcard route
[13:44] <mac_nibblet> oki, i think i follow
[13:45] <DASPRiD> mac_nibblet, sure, you can always do it with child routes
[13:45] <DASPRiD> but this is much more natural :)

@macnibblet
Copy link
Contributor

Would it be possible to add a segment such as versioning on a API ?

return array(
    'router' => array(
        'routes' => array(
            'api' => array(

                'type' => 'hostname',
                'options' => array(

                    'route' => 'api.domain.com',
                ),

                'chain_routes' => array(
                    array(
                        'type'    => 'scheme',
                        'options' => array(
                            'scheme' => 'https'
                        )
                    ),

                    array(
                        'type'    => 'segment',
                        'options' => array(

                            'route'       => '/v:version/',
                            'constraints' => array(

                                'version' => '\d+'
                            )
                        )
                    )
                ),

                'may_terminate' => false,
                'child_routes'  => include __DIR__ . '/routes.php'
            )
        )
    )
);

@DASPRiD
Copy link
Member Author

DASPRiD commented Mar 25, 2013

Yeah @macnibblet, that is one of the use cases :)

@weierophinney
Copy link
Member

@DASPRiD It looks straight-forward, but I see some potential for repetition (different top-level paths needing the same scheme and/or hostname route). I'd be interested in a solution that allows specifying existing named routes in the chain_routes configuration.

@DASPRiD
Copy link
Member Author

DASPRiD commented Mar 26, 2013

@weierophinney: This sounds kina tricky. What exactly do you have in mind there? Should one be able to define "prototype" routes, which can be references there, or would you want to be able to reference existing routes?

@weierophinney
Copy link
Member

@DASPRiD Reference existing routes. This allows defining once, as well as
updating existing routes to chain them to other routes.

On Tuesday, March 26, 2013, Ben Scholzen wrote:

@weierophinney https://github.com/weierophinney: This sounds kina
tricky. What exactly do you have in mind there? Should one be able to
define "prototype" routes, which can be references there, or would you want
to be able to reference existing routes?


Reply to this email directly or view it on GitHubhttps://github.com//pull/3999#issuecomment-15455635
.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

@weierophinney
Copy link
Member

I like this example you did, @DASPRiD - https://gist.github.com/DASPRiD/2f2544eb4b6539fcca0b 👍

@weierophinney
Copy link
Member

@DASPRiD If you can get some tests in, I'm good with this, and will schedule for 2.2.0.

@weierophinney weierophinney mentioned this pull request Apr 5, 2013
@DASPRiD
Copy link
Member Author

DASPRiD commented Apr 28, 2013

Unit tests added, ready for merge.

@macnibblet
Copy link
Contributor

Yay!

@ghost ghost assigned weierophinney Apr 29, 2013
@tomshaw
Copy link
Contributor

tomshaw commented Apr 29, 2013

Can you reference chained routes such as is required in the url view helper and zend navigation route key? You say in your example only a single child?


foreach ($this->routes as $key => $route) {
$chainOptions = $options;
$chainOptions['has_child'] = ((isset($options['has_child']) ? $options['has_child'] : false) || $key !== $lastRouteKey);
Copy link
Member

Choose a reason for hiding this comment

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

This is confusing. Please break it into multiple lines.

@DASPRiD
Copy link
Member Author

DASPRiD commented Apr 29, 2013

@tomshaw I don't really understand your question.

weierophinney added a commit that referenced this pull request Apr 29, 2013
weierophinney added a commit that referenced this pull request Apr 29, 2013
- Broke one statement into two for readability
- Alphabetize invokables list
- CS fixes (commas after all array items)
weierophinney added a commit that referenced this pull request Apr 29, 2013
@DASPRiD
Copy link
Member Author

DASPRiD commented Apr 29, 2013

Thanks @weierophinney for incorporating the changes, as I'm quite busy with $-work right now :)

@weierophinney
Copy link
Member

Merged to develop for release with 2.2.0.

@DASPRiD
Copy link
Member Author

DASPRiD commented Apr 29, 2013

@weierophinney GitHub complains about unmerged commits. What is this about?

@weierophinney
Copy link
Member

@DASPRiD I merged to develop, not master, as it's a new feature. That's why it's complaining.

@tommyseus
Copy link
Contributor

Is it possible to use chains and children at the same time? I don't get this example to work.

expected: http://www.domain.tld/foo/product/c3po
actual: http://www.domain.tld/foo/product/c3po/product/c3po

TreeRouteStackTest:

public function testChainRoute2()
    {
        $request = new PhpRequest();
        $request->setUri('http://localhost/');

        $stack = new TreeRouteStack();
        $stack->setRequestUri($request->getUri());

        $stack->addPrototype(
            'hostnamePrototype',
            array('type' => 'Hostname',  'options' => array('route' => 'www.domain.tld'))
        );
        $stack->addRoute(
            'foo',
            array(
                'type' => 'literal',
                'options' => array(
                    'route' => '/foo'
                ),
                'child_routes' => array(
                    'bar' => array(
                        'type' => 'segment',
                        'options' => array(
                            'route' => '/product/:productId',
                            'defaults' => array('productId' => 'c3po')
                        ),
                    )
                ),
                'chain_routes' => array(
                    'hostnamePrototype'
                ),
            )
        );

        $this->assertEquals(
            'http://www.domain.tld/foo/product/c3po',
            $stack->assemble(array(), array('name' => 'foo/bar'))
        );
    }

@AydinHassan
Copy link
Contributor

I am having the same issue as @tommyseus. I tried to secure the ZfcUser routes with the following:

return array(
    'router' => array(
        'routes' => array(
            'zfcuser' => array(
                'route' => '/user',
                'type' => 'Literal',
                'chain_routes' => array(
                    array(
                        'type' => 'scheme',
                        'options' => array(
                            'scheme' => 'https'
                        )
                    )
                )
            ),
        ),
    ),
);

When echo'ing the route through the URL helper, I get:

https://domain.com/user/change-email/change-email

@koushik1991
Copy link

Site links:

http://test.scampaigns.com/Frontend/index
https://test.scampaigns.com/Frontend/index
Problem:

Number 1 is working, number 2 is giving 404 error.

The problem is with HTTP the site is working fine. but with HTTPS only the default controller is working but other controllers are not working with the https:

Below is my module.config.php

return array(
'router' => array(
'routes' => array(
'home' => array(
'type' => 'Zend\Mvc\Router\Http\Literal',
'options' => array(
'route' => '/',
'defaults' => array(
'controller' => 'Application\Controller\Index',
'action' => 'index',
),
),
),
'application' => array(
'type' => 'Literal',
'options' => array(
'route' => '/',
'defaults' => array(
'NAMESPACE' => 'Application\Controller',
'controller' => 'Index',
'action' => 'index',
),
),
'may_terminate' => true,
'child_routes' => array(
'default' => array(
'type' => 'Segment',
'options' => array(
'route' => '[:controller[/:action]][/:id][/:pId][/:devId]',
'constraints' => array(
'controller' => '[a-zA-Z][a-zA-Z0-9_-]',
'action' => '[a-zA-Z][a-zA-Z0-9
-]_',
),
'defaults' => array(
),
),
),
),
),
),
),
'service_manager' => array(
'abstract_factories' => array(
'Zend\Cache\Service\StorageCacheAbstractServiceFactory',
'Zend\Log\LoggerAbstractServiceFactory',
),
'aliases' => array(
'translator' => 'MvcTranslator',
),
),
'translator' => array(
'locale' => 'en_US',
'translation_file_patterns' => array(
array(
'type' => 'gettext',
'base_dir' => DIR . '/../language',
'pattern' => '%s.mo',
),
),
),
'controllers' => array(
'invokables' => array(
'Application\Controller\Index' => 'Application\Controller\IndexController',
'Application\Controller\Developer' => 'Application\Controller\DeveloperController',
'Application\Controller\Template' => 'Application\Controller\TemplateController',
'Application\Controller\Admin' => 'Application\Controller\AdminController',
'Application\Controller\Frontend' => 'Application\Controller\FrontendController',
'Application\Controller\Ajaxcall' => 'Application\Controller\AjaxcallController'
),
),
'view_manager' => array(
'display_not_found_reason' => true,
'display_exceptions' => true,
'doctype' => 'HTML5',
'not_found_template' => 'error/404',
'exception_template' => 'error/index',
'template_map' => array(
'layout/layout' => DIR . '/../view/layout/layout.phtml',
'application/index/index' => DIR . '/../view/application/index/index.phtml',
'error/404' => DIR . '/../view/error/404.phtml',
'error/index' => DIR . '/../view/error/index.phtml',
),
'template_path_stack' => array(
DIR . '/../view',
),
),
'translator' => array(
'locale' => 'en_US',
'translation_file_patterns' => array(
array(
'type' => 'gettext',
'base_dir' => DIR . '/../language',
'pattern' => '%s.mo',
),
),
),
'console' => array(
'router' => array(
'routes' => array(
),
),
),
);

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

Successfully merging this pull request may close these issues.

7 participants