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

Add controller namespace prefix to template mapping #5670

Merged
merged 3 commits into from
Mar 4, 2014
Merged

Add controller namespace prefix to template mapping #5670

merged 3 commits into from
Mar 4, 2014

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented Jan 3, 2014

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.

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
continue;
}

if (is_string($map)) {
Copy link
Member

Choose a reason for hiding this comment

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

Are there cases when it's not a string?

Copy link
Member Author

Choose a reason for hiding this comment

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

boolean true

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so this needs more documentation then.

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

@weierophinney
Copy link
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
Copy link
Member Author

Xerkus commented Jan 13, 2014

\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
Copy link
Contributor

👍

1 similar comment
@prolic
Copy link
Contributor

prolic commented Jan 14, 2014

👍

@EvanDotPro
Copy link
Member

Put me down as 👍 as well.

@weierophinney weierophinney added this to the 2.3.0 milestone Mar 3, 2014
weierophinney added a commit that referenced this pull request Mar 4, 2014
Add controller namespace prefix to template mapping

Conflicts:
	tests/ZendTest/Mvc/View/InjectTemplateListenerTest.php
weierophinney added a commit that referenced this pull request Mar 4, 2014
- Added docblock
- Fluent interface
weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney merged commit 0063122 into zendframework:develop Mar 4, 2014
@weierophinney weierophinney self-assigned this Mar 4, 2014
@vnagara
Copy link
Contributor

vnagara commented Mar 20, 2014

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

@adamlundrigan
Copy link
Contributor

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

pauloelr referenced this pull request in fabiocarneiro/EdpModuleLayouts Aug 25, 2014
otherwise it would find the name in any place, including the controller name
@Xerkus Xerkus deleted the feature/controller-to-template-map branch October 31, 2014 17:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants