Add controller namespace prefix to template mapping #5670

Merged
merged 3 commits into from Mar 4, 2014

Projects

None yet

8 participants

@Xerkus

Zf2 modules are not at the core of the framework unlike zf1.
We no longer rely on naming conventions but rather on design by contract. As such current controller to template name resolution, where only top level namespace and class name are used, makes no sense. And there are valid use cases where it does not work at all.

Consider controller class Vendor\Module\Controller\FooController:
it will be resolved to vendor/foo/action. So as Vendor\OtherModule\Controller\FooController and even Vendor\OtherModule\Controller\Bar\FooController.

Unless... namespace parameter is specified in route, then resulting template name will suddenly change. Even if controller class does not match that namespace (surprise, mothaf...a!). Most unexpected and unreliable behaviour.

To fix that annoying bit this PR introduces new behaviour. for controller class to view template name resolution
It is very simple:
1. strip \Controller\ namespace
2. strip trailing Controller in classname
3. inflect CamelCase to dash
4. replace namespace separator with slash

Eg: Xerkus\FooModule\Controller\Bar\FooController -> xerkus/foo-module/bar/foo/action

To prevent BC break, this behaviour will only be applied if namespace is whitelisted:

'view_manager' => array(
    'controller_map' => array(
        // for one of my modules
        'Xerkus\FooModule' => true,
        // for all modules under my vendor namespace
        'Xerkus' => true,
        //for one controller
        'ZfcUser\Controller\UserController' => true
    ),
);

Most common use case is for modules that follow PSR-0, namely <Vendor name>\(Namespace\)*<Class Name> rule.

As side effect of whitelist introduction, this PR introduces a way to specify a map of namespaces to template name prefixes

Eg

'view_manager' => array(
    'controller_map' => array(
        'Xerkus\FooModule' => 'xrks-foo',
        // Xerkus\FooModule\Controller\BarController -> xrks-foo/bar/action
        'ZfcUser' => 'zf-commons/zfc-user',
        // ZfcUser\Controller\UserController -> zf-commons/zfc-user/user/action
        'ZfcUser\Controller\UserController' => 'user'
        // ZfcUser\Controller\UserController -> user/action
    ),
);

I consider that more of an edge case use case.

As side note: i'd want to see that behaviour as default in zf3, unless it will be handled completely differently.

@Xerkus Xerkus Add controller namespace prefix to template mapping
InjectTemplateListener changes behaviour when controller is matched by
map entry.  Mapped namespace prefix is replaced with provided value and
rest of the controller inflected. If map entry value is boolean true,
then whole controller class is inflected. In all cases\Controller\
namespace and trailing Controller are stripped.

Most notable use case is when module name is not top level namespace

With map entry 'Vendor\Module' => true and controller class
Vendor\ModuleName\Controller\FooController resulting template name will be
vendor/module-name/foo/action-name
2042f7e
@Ocramius Ocramius and 1 other commented on an outdated diff Jan 7, 2014
library/Zend/Mvc/View/Http/InjectTemplateListener.php
+ {
+ krsort($map);
+ $this->controllerMap = $map;
+ }
+
+ public function mapController($controller)
+ {
+ foreach ($this->controllerMap as $rule => $map) {
+ if (
+ false == $map
+ || !($controller === $rule || strpos($controller, $rule . '\\') === 0)
+ ) {
+ continue;
+ }
+
+ if (is_string($map)) {
@Ocramius
Ocramius Jan 7, 2014

Are there cases when it's not a string?

@Xerkus
Xerkus Jan 7, 2014

boolean true

@Ocramius
Ocramius Jan 7, 2014

Ok, so this needs more documentation then.

Also, I'd wrap all this giant foreach contents' into a private method

@weierophinney
Zend Framework member

2 things:

  • First, what if you don't use a "Controller" segment in your namespace? or in your class name?
  • Second, what is the performance impact?

The first question is what prevented us from implementing this for subnamespaces previously, as we cannot accurately guess the namespace, nor enforce naming conventions on the classes themselves. The second question also quickly arose, due to the logic heuristics necessary to resolve.

I think you may have these largely solved by having the controller_map, which makes it opt-in behavior, and explicit.

Can you ask on the ML to see if you can get more people to test this, please?

@Xerkus

\Controller\ namespace is not required, if it is present - then it is stripped.
Trailing Controller is stripped from class name unless class name is exactly Controller
I intentionally moved from "fake module" namespace and class name to FQCN mapping.

Using perfect real life example with Apigility:

'view_manager' => array(
    'controller_map' => array(
        //For all apigility modules
        'ZF\Apigility' => true,
    ),
);

For controller ZF\Apigility\Admin\Controller\AppController mapping will be zf/apigility/admin/app/<action>
With current behaviour it is zf/app/<action>
It is good as long as in whole ZF top level namespace there is no controller named AppController

Performance impact should be neglectible since this mapping should happen once per dispatch. And i expect controller map to hold one entry per vendor or module name at most, in realistic usage scenario.
Impl uses simple string operations, no regexes.
Essentially it is whitelisting with optional prefix replacement. Not much logic required.

Will post on ML a bit later

@danizord

👍

@prolic

👍

@EvanDotPro
Zend Framework member

Put me down as 👍 as well.

@weierophinney weierophinney added this to the 2.3.0 milestone Mar 3, 2014
@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney [#5670] CS fixes
- Added docblock
- Fluent interface
7d157a4
@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney Merge branch 'feature/5670' into develop
Close #5670
c81fac6
@weierophinney weierophinney merged commit 0063122 into zendframework:develop Mar 4, 2014

1 check passed

Details default The Travis CI build passed
@weierophinney weierophinney self-assigned this Mar 4, 2014
@vnagara

👍 Previous behaviour annoyed me too and did some impact.
Nice done.

@adamlundrigan
Zend Framework member

The PR containing documentation for this addition hasn't been merged (zendframework/zf2-documentation#1298)

@pauloelr pauloelr referenced this pull request in fabiocarneiro/EdpModuleLayouts Aug 25, 2014
@fabiocarneiro fabiocarneiro fix if condition
otherwise it would find the name in any place, including the controller name
5f0e45c
@Xerkus Xerkus deleted the Xerkus:feature/controller-to-template-map branch Oct 31, 2014
@Xerkus Xerkus added a commit to Xerkus/zend-mvc that referenced this pull request May 17, 2016
@Xerkus Xerkus Remove legacy pseudomodule template name resolution
Whitelisted classname based template name resolution from ^2.3.0 is now
default

Template resolution follows these rules:
1. strip \Controller\ namespace if present
2. strip trailing Controller in classname
3. inflect CamelCase to dash
4. replace namespace separator with slash

@see zendframework/zendframework#5670
1e075b2
@Xerkus Xerkus referenced this pull request in zendframework/zend-mvc May 17, 2016
Merged

[BC break] Remove old InjectTemplateListener behavior #139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment