InjectTemplateListener::injectTemplate() ignored 'multi-depth' in controller namespace #4080

Closed
Freeaqingme opened this Issue Mar 19, 2013 · 7 comments

Projects

None yet

7 participants

@Freeaqingme
Member

In Zend\Mvc\View\Http\InjectTemplateListener::injectTempate() the view script that needs to be used is tried to deduce. The problem I have is that my Controller is named MyApp\Something\Controller\IndexController.

The template I would expect the indexAction() method to be resolved to is myapp/something/index/index. However, the result I'm currently getting is: myapp/index/index. This is particularly frustrating because I also have a MyApp\Controller\IndexController ;)

I think the actual fix could potentially be applied in either the injectTemplate() method, or the inflectName() method (if it can be fixed at all...)

@Freeaqingme
Member

For what it's worth; I do have a workaround, but it's really nasty and hackish. This could be part of a controller, or action helper (or a listener, but haven't used it in that context): https://gist.github.com/Freeaqingme/172c192887c687bb9b06

It does however treat \Controller\ as a special string, and relies upon the fact that php treats both \ and / the same in file paths (when resolving view paths), and that the inflectName() does a strpos() on .

@weierophinney
Member

The problem is that organization of the source code in a module is up to the developer; a "Controller" subnamespace may or may not exist. As an example, if a module I write only has a handful of classes, and none or very few are related with regards to component, I may have a shallow hierarchy. (As an example, a one-off API I created for the ZF website had a storage class, a service class, a controller, and a module class; I grouped them all together.)

I think a better solution is having a listener attached to the controllers in a module that injects the template. If this happens prior to the default listener, that listener will be a no-op, as it will have discovered the template already. We could potentially distribute a listener like this:

namespace Zend\Mvc\Controller\Listener;

use Zend\EventManager\EventManagerInterface;
use Zend\Mvc\View\Http\InjectTemplateListener;

class InjectTemplateForNamespaceListener extends InjectTemplateListener
{
    protected $namespace;

    public function __construct($namespace)
    {
        $this->namespace = $namespace;
    }

    public function attach(EventManagerInterface $events)
    {
        $this->listeners[] = $events->attach('dispatch', array($this, 'onDispatch'), -1);
    }

    public function onDispatch($e)
    {
        $controller = $e->getTarget();
        $class      = get_class($controller);
        if (strlen($class) <= strlen($this->namespace)
            || (0 !== strpos($class, $this->namespace))
        ) {
            return;
        }

        $model = $e->getResult();
        if (!$model instanceof ViewModel) {
            return;
        }

        $routeMatch = $e->getRouteMatch();
        $controller = $this->deriveControllerClass($class);
        $template   = $this->inflectName($this->namespace);
        if (!empty($template)) {
            $template .= '/';
        }
        $template  .= $this->inflectName($controller);

        $action     = $routeMatch->getParam('action');
        if (null !== $action) {
            $template .= '/' . $this->inflectName($action);
        }
        $model->setTemplate($template);
    }
}

Usage would be:

namespace Foo\Bar;

use Zend\Mvc\Controller\Listener\InjectTemplateForNamespaceListener;
class Module
{
    public function onBootstrap($e)
    {
        $listener = new InjectTemplateForNamespaceListener(__NAMESPACE__);
        $e->getTarget()->getEventManager()->getSharedManager()->attach(
            ''Zend\Stdlib\DispatchableInterface',
            'dispatch',
            array($listener, 'onDispatch'),
            -10
        );
    }
}

This is something we could drop in easily, and then simply document. Would this work for you, @Freeaqingme ? Also, @devosc -- thoughts? (pinging you as you commented on Dolf's gist)

@devosc
Contributor
devosc commented Apr 6, 2013

That doesn't quite work for a flattened ZF2 project arranged as

/application
/application/src/Admin/Controller/IndexController.php
/application/src/Member/Controller/IndexController.php
/application/src/IndexController.php
/application/view/admin/index/index.phtml
/application/view/member/index/index.phtml
/application/view/index/index.phtml

Namespaces
Application\Admin\Controller
Application\Member\Controller
Application

Whether the schema is ideal is questionable. Although one aspect of it is
that, it is like having multiple modules consolidated into one, or rather,
it would be very easy to promote them to ZF2 standard modules.

I think the biggest part of the problem, is that the term 'Controller' ends
up in the view script file path. That seems odd and unnecessary. I realize
this may be a complicated matter, but I think having a convention of
removing the term controller from the end of the namespace prior to
concatenating the controller should be ok (but it might cause a BC.
However, there is already one existing convention, so another similar one
should be understandable).

With the above template listener the paths searched for are (assuming two
namespaced listeners)

application\admin\controller/index/index

application/index/index

Which makes the namespaced listener nothing more than a prefix and still
doesn't remove the term controller. It would also be nice if the term
'application' was not in the file path also that is more tolerable and
doesn't seem out of place.

If there was just one namespaced listener for 'Application' then for the
admin index script the listener resolves to application/index/index, which
is incorrect...

On Thu, Apr 4, 2013 at 8:36 AM, weierophinney notifications@github.comwrote:

The problem is that organization of the source code in a module is up to
the developer; a "Controller" subnamespace may or may not exist. As an
example, if a module I write only has a handful of classes, and none or
very few are related with regards to component, I may have a shallow
hierarchy. (As an example, a one-off API I created for the ZF website had a
storage class, a service class, a controller, and a module class; I grouped
them all together.)

I think a better solution is having a listener attached to the controllers
in a module that injects the template. If this happens prior to the default
listener, that listener will be a no-op, as it will have discovered the
template already. We could potentially distribute a listener like this:

namespace Zend\Mvc\Controller\Listener;
use Zend\EventManager\EventManagerInterface;use Zend\Mvc\View\Http\InjectTemplateListener;
class InjectTemplateForNamespaceListener extends InjectTemplateListener{
protected $namespace;

public function __construct($namespace)
{
    $this->namespace = $namespace;
}

public function attach(EventManagerInterface $events)
{
    $this->listeners[] = $events->attach('dispatch', array($this, 'onDispatch'), -1);
}

public function onDispatch($e)
{
    $controller = $e->getTarget();
    $class      = get_class($controller);
    if (strlen($class) <= strlen($this->namespace)
        || (0 !== strpos($class, $this->namespace))
    ) {
        return;
    }

    $model = $e->getResult();
    if (!$model instanceof ViewModel) {
        return;
    }

    $routeMatch = $e->getRouteMatch();
    $controller = $this->deriveControllerClass($class);
    $template   = $this->inflectName($this->namespace);
    if (!empty($template)) {
        $template .= '/';
    }
    $template  .= $this->inflectName($controller);

    $action     = $routeMatch->getParam('action');
    if (null !== $action) {
        $template .= '/' . $this->inflectName($action);
    }
    $model->setTemplate($template);
}}

Usage would be:

namespace Foo\Bar;
use Zend\Mvc\Controller\Listener\InjectTemplateForNamespaceListener;class Module{
public function onBootstrap($e)
{
$listener = new InjectTemplateForNamespaceListener(NAMESPACE);
$e->getTarget()->getEventManager()->getSharedManager()->attach(
''Zend\Stdlib\DispatchableInterface', 'dispatch', array($listener, 'onDispatch'),
-10
);
}}

This is something we could drop in easily, and then simply document. Would
this work for you, @Freeaqingme https://github.com/Freeaqingme ? Also,
@devosc https://github.com/devosc -- thoughts? (pinging you as you
commented on Dolf's gist)


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/issues/4080#issuecomment-15897326
.

Greg

@peuh
peuh commented Dec 6, 2013

Hi @weierophinney,

Would you mind explaining why in deriveControllerSubNamespace you slice $nsArray by 2? (here)

The comment says : Remove the first two elements representing the module and controller directory

But if we have a subnamespace (the '\User' part of a module namespace like Myapp\User) it'll be at the second position ($nsArray[1]).

If I replace :

$subNsArray = array_slice($nsArray, 2);

by :

$subNsArray = array_slice($nsArray, 1);

It works for such a name space :

Appname\User

with the following settings :

module namespace : Appname\User
controller path : module/Appname/User/src/Appname/User/Controller/IndexController
view path : module/Appname/User/view/appname/user/Index/index.phtml

module namespace : User
controller path : module/User/src/User/Controller/IndexController
view path : module/User/view/user/Index/index.phtml

The only situation where it doesn't work is when in the route option 'NAMESPACE' includes \Controller or anything else not part of the module namespace.

Looking at the code in injectTemplate(MvcEvent $e), one could think that it's the module namespace you specify in 'NAMESPACE' not the controller namespace

IMHO that leads to confusion.

Wouldn't it be possible to have another option for routes (i.e: FULLNAMESPACE ) so we can discriminate between both scenario

And then in injectTemplate :

    $module     = $this->deriveModuleNamespace($controller);
    if ($namespace = $routeMatch->getParam(ModuleRouteListener::MODULE_NAMESPACE)) {
        $controllerSubNs = $this->deriveControllerSubNamespace($namespace);
        if (!empty($controllerSubNs)) {
            if (!empty($module)) {
                $module .= '/' . $controllerSubNs;
            } else {
                $module = $controllerSubNs;
            }
        }
    }
     if ($namespace = $routeMatch->getParam(ModuleRouteListener::FULL_NAMESPACE)) {
        $controllerSubNs = $this->deriveControllerSubSubNamespace($namespace);
        if (!empty($controllerSubNs)) {
            if (!empty($module)) {
                $module .= '/' . $controllerSubNs;
            } else {
                $module = $controllerSubNs;
            }
        }
    }

Or you could simply decide that NAMESPACE must be the module namespace and sub namespaces should be specified in the controller option of the route (e.g : 'controller' => 'Controller\Index', ). Wouldn't it make sense?

@Freeaqingme & @devosc : your thoughts?

That would be great to have that situation handled out the box.

@pauloelr
Contributor
pauloelr commented Jun 3, 2014

I don't think one InjectTemplateListener can by itself handle all possible cases.
I think the best solution could be a per module configurable InjectTemplateListener
That way we can keep bc and provide a way to configure the template path on each module

Something like this:

return array(
    'view_manager' => array(
        'template_inject' => array(
            __NAMESPACE__ => 'name/of/module/view/templates/:controllerName/:actionName',
        ),
    ),
);

Using wildcards like :controllerName and :actionName to replace by proper names
if the key configuration for that especific module is not present it fallback to default behavior

If this couldn't be implemented on ZF2 at least is a good thing to think about for ZF3 since now with PSR-4 and composer support to it more namespaced modules will be showing up

@pauloelr
Contributor
pauloelr commented Jun 3, 2014

Ops.

I Just found out that a similar approuch was already implemented on #5670

@adamlundrigan
Member

Can this be closed now given that #5670 has been merged and documented?

@Ocramius Ocramius self-assigned this Jan 26, 2015
@Ocramius Ocramius closed this Jan 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment