Skip to content

Commit

Permalink
Merge pull request #3544 from alcohol/match-event-to-listener-expecta…
Browse files Browse the repository at this point in the history
…tion

fix for #3382
  • Loading branch information
Seldaek committed Dec 12, 2014
2 parents 55895ab + 113606b commit 76c666e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
29 changes: 29 additions & 0 deletions src/Composer/EventDispatcher/EventDispatcher.php
Expand Up @@ -155,6 +155,7 @@ protected function doDispatch(Event $event)
$return = 0;
foreach ($listeners as $callable) {
if (!is_string($callable) && is_callable($callable)) {
$event = $this->checkListenerExpectedEvent($callable, $event);
$return = false === call_user_func($callable, $event) ? 1 : 0;
} elseif ($this->isPhpScript($callable)) {
$className = substr($callable, 0, strpos($callable, '::'));
Expand Down Expand Up @@ -200,9 +201,37 @@ protected function doDispatch(Event $event)
*/
protected function executeEventPhpScript($className, $methodName, Event $event)
{
$event = $this->checkListenerExpectedEvent(array($className, $methodName), $event);

return $className::$methodName($event);
}

/**
* @param mixed $target
* @param Event $event
* @return Event|CommandEvent
*/
protected function checkListenerExpectedEvent($target, Event $event)
{
if (!$event instanceof Script\Event) {
return $event;
}

try {
$reflected = new \ReflectionParameter($target, 0);
} catch (\ReflectionException $e) {
return $event;
}

$expected = $reflected->getClass()->name;

if (!$event instanceof $expected && $expected === 'Composer\Script\CommandEvent') {
$event = new CommandEvent($event->getName(), $event->getComposer(), $event->getIO(), $event->isDevMode(), $event->getArguments());
}

return $event;
}

/**
* Add a listener for a particular event
*
Expand Down
16 changes: 16 additions & 0 deletions tests/Composer/Test/EventDispatcher/EventDispatcherTest.php
Expand Up @@ -17,6 +17,7 @@
use Composer\Installer\InstallerEvents;
use Composer\TestCase;
use Composer\Script\ScriptEvents;
use Composer\Script\CommandEvent;
use Composer\Util\ProcessExecutor;

class EventDispatcherTest extends TestCase
Expand All @@ -38,6 +39,16 @@ public function testListenerExceptionsAreCaught()
$dispatcher->dispatchCommandEvent(ScriptEvents::POST_INSTALL_CMD, false);
}

public function testDispatcherCanConvertScriptEventToCommandEventForListener()
{
$io = $this->getMock('Composer\IO\IOInterface');
$dispatcher = $this->getDispatcherStubForListenersTest(array(
"Composer\Test\EventDispatcher\EventDispatcherTest::convertEvent"
), $io);

$this->assertEquals(1, $dispatcher->dispatchScript(ScriptEvents::POST_INSTALL_CMD, false));
}

/**
* @dataProvider getValidCommands
* @param string $command
Expand Down Expand Up @@ -205,6 +216,11 @@ public static function call()
throw new \RuntimeException();
}

public static function convertEvent(CommandEvent $event)
{
return false;
}

public static function someMethod()
{
return true;
Expand Down

4 comments on commit 76c666e

@koemeet
Copy link

Choose a reason for hiding this comment

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

This PR causes issues. The checkListenerExpectedEvent causes this exception [ErrorException] Trying to get property of non-object when I run composer install. It trips on this line:

Script Symfony\Cmf\Bundle\CreateBundle\Composer\ScriptHandler::downloadCreate handling the post-install-cmd event terminated with an exception

@Seldaek I had to use --rollback in order to make it all work again.

@alcohol
Copy link
Member

Choose a reason for hiding this comment

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

That's why I created #3547 (see also related #3546)

@koemeet
Copy link

Choose a reason for hiding this comment

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

Ah okay, sorry did not see that one. Good to see a fix coming in 👍

@alcohol
Copy link
Member

Choose a reason for hiding this comment

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

No problem, thanks for reporting regardless 👍

Please sign in to comment.