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

Invalid attaching of the ConsoleResponseSender listener #12

Closed
fedys opened this issue Jul 10, 2016 · 5 comments
Closed

Invalid attaching of the ConsoleResponseSender listener #12

fedys opened this issue Jul 10, 2016 · 5 comments

Comments

@fedys
Copy link

fedys commented Jul 10, 2016

This issue is related to the #10

Because of the ConsoleResponseSender is attached via the delegator and delegators are called before initializers then the EventManagerAwareInitializer (re)sets the EventManager which causes lost of all previously attached listeners including the ConsoleResponseSender.

@kachar
Copy link

kachar commented Aug 10, 2016

@weierophinney
Copy link
Member

@fedys I think you're mis-diagnosing the root cause. The EventManagerAwareInitializer only resets the EM instance if one is not already present in the instance (see the lines immediately preceding the one you linked).

Note: I'm not saying the problem doesn't exist; I'm saying that the diagnosis is likely incorrect. I'll see if I can resolve it shortly.

@fedys
Copy link
Author

fedys commented Aug 29, 2016

@weierophinney The problem I described really exists. The root cause is $eventManager->getSharedManager() returns null and therefore the second part of the condition is not met.

@weierophinney
Copy link
Member

The problem is in zend-mvc's SendResponseListener implementation/service configuration.

SendResponseListener will lazy-load an event manager instance if none is already present when getEventManager() is invoked; however, the instance is created with no arguments, which means that with zend-eventmanager v3, no shared manager is present. Also in v3, there is no way to inject a shared manager after the fact; it must be injected at instantiation (this was a change made to aid performance).

The solution will be to provide a factory for the SendResponseListener within zend-mvc that pulls the EventManager service and pushes it into a new SendResponseListener instance.

You can test this locally in the meantime. Create a factory class as follows, in module/Application/src/SendResponseListenerFactory.php (or module/Application/src/Application/SendResponseListenerFactory.php, if still using a PSR-0 structure from an older application):

namespace Application;

use Interop\Container\ContainerInterface;
use Zend\Mvc\SendResponseListener;

class SendResponseListenerFactory
{
    public function __invoke(ContainerInterface $container)
    {
        $listener = new SendResponseListener();
        $listener->setEventManager($container->get('EventManager'));
        return $listener;
    }
}

Add the following to module/Application/config/module.config.php:

'service_manager' => [
    'factories' => [
        \Zend\Mvc\SendResponseListener::class => \Application\SendResponseListenerFactory::class,
    ]
]

In addition to the above, I tested by adding a single console route, and adding console usage via the Application\Module::getConsoleUsage() method. I can verify that all works as expected once the new factory is registered; without the factory registered, no output is generated.

I'll get a PR ready for zend-mvc shortly, and tag this and other related issues when it's ready to merge.

weierophinney added a commit to weierophinney/zend-mvc that referenced this issue Aug 29, 2016
Reported in:

- zendframework/zend-mvc-console#10
- zendframework/zend-mvc-console#11
- zendframework/zend-mvc-console#12

The `SendResponseListener` was lazy-instantiating an event manager on
first request to `getEventManager()`. However, because initializers run
after delegators, this meant that the EM instance composed did not have
a shared EM instance, which triggered the initializer to re-inject,
losing any listeners injected by delegators.

This patch introduces a factory for the `SendResponseListener`. The
factory creates the instance, and then injects it with an EM instance
pulled from the container; since these are guaranteed to have a shared
EM instance, the initializer will skip injection, keeping any listeners
injected by delegators.
weierophinney added a commit to zendframework/zend-mvc that referenced this issue Aug 29, 2016
Reported in:

- zendframework/zend-mvc-console#10
- zendframework/zend-mvc-console#11
- zendframework/zend-mvc-console#12

The `SendResponseListener` was lazy-instantiating an event manager on
first request to `getEventManager()`. However, because initializers run
after delegators, this meant that the EM instance composed did not have
a shared EM instance, which triggered the initializer to re-inject,
losing any listeners injected by delegators.

This patch introduces a factory for the `SendResponseListener`. The
factory creates the instance, and then injects it with an EM instance
pulled from the container; since these are guaranteed to have a shared
EM instance, the initializer will skip injection, keeping any listeners
injected by delegators.
@weierophinney
Copy link
Member

To get the fix, please update your zend-mvc to 3.0.3:

$ composer require "zendframework/zend-mvc:^3.0.3"

weierophinney added a commit that referenced this issue Aug 29, 2016
Updates the test written for #11, and modifies it as follows:

- Uses the new `SendResponseListenerFactory` to create the
  `SendResponseListener` instance.
- Registers the `ConsoleResponseSenderDelegatorFactory` directly in the test
  setup, instead of wrapping it in a callable; we're only interested in the
  final state of the event manager.
- Updates the minimum zend-mvc requirement to 3.0.3, which introduces the
  `SendResponseListenerFactory`.
weierophinney added a commit that referenced this issue Aug 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants