From 3fc9ede24b85cbca0914060e6043abdc2cb1e406 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 18 Feb 2019 18:14:46 +0100 Subject: [PATCH] Add plugin callbacks for deactivation and uninstall, fixes #3000 --- src/Composer/Installer/PluginInstaller.php | 24 ++-- src/Composer/Plugin/PluginInterface.php | 18 +++ src/Composer/Plugin/PluginManager.php | 108 +++++++++++++++++- .../Fixtures/plugin-v1/Installer/Plugin.php | 11 ++ .../Fixtures/plugin-v2/Installer/Plugin2.php | 11 ++ .../Fixtures/plugin-v3/Installer/Plugin2.php | 11 ++ .../Fixtures/plugin-v4/Installer/Plugin1.php | 11 ++ .../Fixtures/plugin-v4/Installer/Plugin2.php | 11 ++ .../Fixtures/plugin-v5/Installer/Plugin5.php | 11 ++ .../Fixtures/plugin-v6/Installer/Plugin6.php | 11 ++ .../Fixtures/plugin-v7/Installer/Plugin7.php | 11 ++ .../Fixtures/plugin-v8/Installer/Plugin8.php | 11 ++ .../Fixtures/plugin-v9/Installer/Plugin.php | 11 ++ .../Test/Plugin/PluginInstallerTest.php | 32 +++++- 14 files changed, 282 insertions(+), 10 deletions(-) diff --git a/src/Composer/Installer/PluginInstaller.php b/src/Composer/Installer/PluginInstaller.php index 62a16fc62..a52e1937e 100644 --- a/src/Composer/Installer/PluginInstaller.php +++ b/src/Composer/Installer/PluginInstaller.php @@ -70,7 +70,7 @@ class PluginInstaller extends LibraryInstaller $this->composer->getPluginManager()->registerPackage($package, true); } catch (\Exception $e) { // Rollback installation - $this->io->writeError('Plugin installation failed, rolling back'); + $this->io->writeError('Plugin initialization failed, uninstalling plugin'); parent::uninstall($repo, $package); throw $e; } @@ -81,12 +81,22 @@ class PluginInstaller extends LibraryInstaller */ public function update(InstalledRepositoryInterface $repo, PackageInterface $initial, PackageInterface $target) { - $extra = $target->getExtra(); - if (empty($extra['class'])) { - throw new \UnexpectedValueException('Error while installing '.$target->getPrettyName().', composer-plugin packages should have a class defined in their extra key to be usable.'); - } - parent::update($repo, $initial, $target); - $this->composer->getPluginManager()->registerPackage($target, true); + + try { + $this->composer->getPluginManager()->deactivatePackage($initial, true); + $this->composer->getPluginManager()->registerPackage($target, true); + } catch (\Exception $e) { + // Rollback installation + $this->io->writeError('Plugin initialization failed, uninstalling plugin'); + parent::uninstall($repo, $target); + throw $e; + } + } + + public function uninstall(InstalledRepositoryInterface $repo, PackageInterface $package) + { + $this->composer->getPluginManager()->uninstallPackage($package, true); + parent::uninstall($repo, $package); } } diff --git a/src/Composer/Plugin/PluginInterface.php b/src/Composer/Plugin/PluginInterface.php index 5158b66f6..27b8c9754 100644 --- a/src/Composer/Plugin/PluginInterface.php +++ b/src/Composer/Plugin/PluginInterface.php @@ -36,4 +36,22 @@ interface PluginInterface * @param IOInterface $io */ public function activate(Composer $composer, IOInterface $io); + + /** + * Remove any hooks from Composer + * + * @param Composer $composer + * @param IOInterface $io + */ + public function deactivate(Composer $composer, IOInterface $io); + + /** + * Prepare the plugin to be uninstalled + * + * This will be called after deactivate + * + * @param Composer $composer + * @param IOInterface $io + */ + public function uninstall(Composer $composer, IOInterface $io); } diff --git a/src/Composer/Plugin/PluginManager.php b/src/Composer/Plugin/PluginManager.php index 786d846c5..7408359bd 100644 --- a/src/Composer/Plugin/PluginManager.php +++ b/src/Composer/Plugin/PluginManager.php @@ -144,7 +144,7 @@ class PluginManager $oldInstallerPlugin = ($package->getType() === 'composer-installer'); - if (in_array($package->getName(), $this->registeredPlugins)) { + if (isset($this->registeredPlugins[$package->getName()])) { return; } @@ -200,16 +200,82 @@ class PluginManager if ($oldInstallerPlugin) { $installer = new $class($this->io, $this->composer); $this->composer->getInstallationManager()->addInstaller($installer); + $this->registeredPlugins[$package->getName()] = $installer; } elseif (class_exists($class)) { $plugin = new $class(); $this->addPlugin($plugin); - $this->registeredPlugins[] = $package->getName(); + $this->registeredPlugins[$package->getName()] = $plugin; } elseif ($failOnMissingClasses) { throw new \UnexpectedValueException('Plugin '.$package->getName().' could not be initialized, class not found: '.$class); } } } + /** + * Deactivates a plugin package + * + * If it's of type composer-installer it is unregistered from the installers + * instead for BC + * + * @param PackageInterface $package + * + * @throws \UnexpectedValueException + */ + public function deactivatePackage(PackageInterface $package) + { + if ($this->disablePlugins) { + return; + } + + $oldInstallerPlugin = ($package->getType() === 'composer-installer'); + + if (!isset($this->registeredPlugins[$package->getName()])) { + return; + } + + if ($oldInstallerPlugin) { + $installer = $this->registeredPlugins[$package->getName()]; + unset($this->registeredPlugins[$package->getName()]); + $this->composer->getInstallationManager()->removeInstaller($installer); + } else { + $plugin = $this->registeredPlugins[$package->getName()]; + unset($this->registeredPlugins[$package->getName()]); + $this->removePlugin($plugin); + } + } + + /** + * Uninstall a plugin package + * + * If it's of type composer-installer it is unregistered from the installers + * instead for BC + * + * @param PackageInterface $package + * + * @throws \UnexpectedValueException + */ + public function uninstallPackage(PackageInterface $package) + { + if ($this->disablePlugins) { + return; + } + + $oldInstallerPlugin = ($package->getType() === 'composer-installer'); + + if (!isset($this->registeredPlugins[$package->getName()])) { + return; + } + + if ($oldInstallerPlugin) { + $this->deactivatePackage($package); + } else { + $plugin = $this->registeredPlugins[$package->getName()]; + unset($this->registeredPlugins[$package->getName()]); + $this->removePlugin($plugin); + $this->uninstallPlugin($plugin); + } + } + /** * Returns the version of the internal composer-plugin-api package. * @@ -240,6 +306,44 @@ class PluginManager } } + /** + * Removes a plugin, deactivates it and removes any listener the plugin has set on the plugin instance + * + * Ideally plugin packages should be deactivated via deactivatePackage, but if you use Composer + * programmatically and want to deregister a plugin class directly this is a valid way + * to do it. + * + * @param PluginInterface $plugin plugin instance + */ + public function removePlugin(PluginInterface $plugin) + { + $index = array_search($plugin, $this->plugins, true); + if ($index === false) { + return; + } + + $this->io->writeError('Unloading plugin '.get_class($plugin), true, IOInterface::DEBUG); + unset($this->plugins[$index]); + $plugin->deactivate($this->composer, $this->io); + + $this->composer->getEventDispatcher()->removeListener($plugin); + } + + /** + * Notifies a plugin it is being uninstalled and should clean up + * + * Ideally plugin packages should be uninstalled via uninstallPackage, but if you use Composer + * programmatically and want to deregister a plugin class directly this is a valid way + * to do it. + * + * @param PluginInterface $plugin plugin instance + */ + public function uninstallPlugin(PluginInterface $plugin) + { + $this->io->writeError('Uninstalling plugin '.get_class($plugin), true, IOInterface::DEBUG); + $plugin->uninstall($this->composer, $this->io); + } + /** * Load all plugins and installers from a repository * diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v1/Installer/Plugin.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v1/Installer/Plugin.php index f80acd325..c757d4b09 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v1/Installer/Plugin.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v1/Installer/Plugin.php @@ -12,5 +12,16 @@ class Plugin implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v1'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v1'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v1'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v2/Installer/Plugin2.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v2/Installer/Plugin2.php index db5a4462e..32090b66d 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v2/Installer/Plugin2.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v2/Installer/Plugin2.php @@ -12,5 +12,16 @@ class Plugin2 implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v2'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v2'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v2'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v3/Installer/Plugin2.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v3/Installer/Plugin2.php index 861c1679b..034388162 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v3/Installer/Plugin2.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v3/Installer/Plugin2.php @@ -12,5 +12,16 @@ class Plugin2 implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v3'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v3'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v3'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin1.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin1.php index 93bcabc98..2eaee6a3f 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin1.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin1.php @@ -13,5 +13,16 @@ class Plugin1 implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v4-plugin1'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v4-plugin1'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v4-plugin1'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin2.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin2.php index d946deb89..3c5311a82 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin2.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v4/Installer/Plugin2.php @@ -13,5 +13,16 @@ class Plugin2 implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v4-plugin2'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v4-plugin2'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v4-plugin2'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v5/Installer/Plugin5.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v5/Installer/Plugin5.php index a2ac37bc5..fb9f08a6d 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v5/Installer/Plugin5.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v5/Installer/Plugin5.php @@ -10,5 +10,16 @@ class Plugin5 implements PluginInterface { public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v5'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v5'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v5'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v6/Installer/Plugin6.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v6/Installer/Plugin6.php index e46c0fcb0..acce1f972 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v6/Installer/Plugin6.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v6/Installer/Plugin6.php @@ -10,5 +10,16 @@ class Plugin6 implements PluginInterface { public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v6'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v6'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v6'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v7/Installer/Plugin7.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v7/Installer/Plugin7.php index 5560a6047..84734ce3b 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v7/Installer/Plugin7.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v7/Installer/Plugin7.php @@ -10,5 +10,16 @@ class Plugin7 implements PluginInterface { public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v7'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v7'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v7'); } } diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v8/Installer/Plugin8.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v8/Installer/Plugin8.php index 7e9a0aab1..4534e13ef 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v8/Installer/Plugin8.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v8/Installer/Plugin8.php @@ -13,6 +13,17 @@ class Plugin8 implements PluginInterface, Capable public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v8'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v8'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v8'); } public function getCapabilities() diff --git a/tests/Composer/Test/Plugin/Fixtures/plugin-v9/Installer/Plugin.php b/tests/Composer/Test/Plugin/Fixtures/plugin-v9/Installer/Plugin.php index 74e1beb8b..870f11cd1 100644 --- a/tests/Composer/Test/Plugin/Fixtures/plugin-v9/Installer/Plugin.php +++ b/tests/Composer/Test/Plugin/Fixtures/plugin-v9/Installer/Plugin.php @@ -14,5 +14,16 @@ class Plugin implements PluginInterface public function activate(Composer $composer, IOInterface $io) { + $io->write('activate v9'); + } + + public function deactivate(Composer $composer, IOInterface $io) + { + $io->write('deactivate v9'); + } + + public function uninstall(Composer $composer, IOInterface $io) + { + $io->write('uninstall v9'); } } diff --git a/tests/Composer/Test/Plugin/PluginInstallerTest.php b/tests/Composer/Test/Plugin/PluginInstallerTest.php index 633c5ab18..bd83ce16f 100644 --- a/tests/Composer/Test/Plugin/PluginInstallerTest.php +++ b/tests/Composer/Test/Plugin/PluginInstallerTest.php @@ -19,6 +19,9 @@ use Composer\Package\CompletePackage; use Composer\Package\Loader\JsonLoader; use Composer\Package\Loader\ArrayLoader; use Composer\Plugin\PluginManager; +use Symfony\Component\Console\Output\OutputInterface; +use Composer\IO\BufferIO; +use Composer\EventDispatcher\EventDispatcher; use Composer\Autoload\AutoloadGenerator; use Composer\Test\TestCase; use Composer\Util\Filesystem; @@ -96,7 +99,7 @@ class PluginInstallerTest extends TestCase return __DIR__.'/Fixtures/'.$package->getPrettyName(); })); - $this->io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + $this->io = new BufferIO(); $dispatcher = $this->getMockBuilder('Composer\EventDispatcher\EventDispatcher')->disableOriginalConstructor()->getMock(); $this->autoloadGenerator = new AutoloadGenerator($dispatcher); @@ -108,6 +111,7 @@ class PluginInstallerTest extends TestCase $this->composer->setRepositoryManager($rm); $this->composer->setInstallationManager($im); $this->composer->setAutoloadGenerator($this->autoloadGenerator); + $this->composer->setEventDispatcher(new EventDispatcher($this->composer, $this->io)); $this->pm = new PluginManager($this->io, $this->composer); $this->composer->setPluginManager($this->pm); @@ -140,6 +144,7 @@ class PluginInstallerTest extends TestCase $plugins = $this->pm->getPlugins(); $this->assertEquals('installer-v1', $plugins[0]->version); + $this->assertEquals('activate v1'.PHP_EOL, $this->io->getOutput()); } public function testInstallMultiplePlugins() @@ -158,6 +163,7 @@ class PluginInstallerTest extends TestCase $this->assertEquals('installer-v4', $plugins[0]->version); $this->assertEquals('plugin2', $plugins[1]->name); $this->assertEquals('installer-v4', $plugins[1]->version); + $this->assertEquals('activate v4-plugin1'.PHP_EOL.'activate v4-plugin2'.PHP_EOL, $this->io->getOutput()); } public function testUpgradeWithNewClassName() @@ -176,7 +182,29 @@ class PluginInstallerTest extends TestCase $installer->update($this->repository, $this->packages[0], $this->packages[1]); $plugins = $this->pm->getPlugins(); + $this->assertCount(1, $plugins); $this->assertEquals('installer-v2', $plugins[1]->version); + $this->assertEquals('activate v1'.PHP_EOL.'deactivate v1'.PHP_EOL.'activate v2'.PHP_EOL, $this->io->getOutput()); + } + + public function testUninstall() + { + $this->repository + ->expects($this->once()) + ->method('getPackages') + ->will($this->returnValue(array($this->packages[0]))); + $this->repository + ->expects($this->exactly(1)) + ->method('hasPackage') + ->will($this->onConsecutiveCalls(true, false)); + $installer = new PluginInstaller($this->io, $this->composer); + $this->pm->loadInstalledPlugins(); + + $installer->uninstall($this->repository, $this->packages[0]); + + $plugins = $this->pm->getPlugins(); + $this->assertCount(0, $plugins); + $this->assertEquals('activate v1'.PHP_EOL.'deactivate v1'.PHP_EOL.'uninstall v1'.PHP_EOL, $this->io->getOutput()); } public function testUpgradeWithSameClassName() @@ -196,6 +224,7 @@ class PluginInstallerTest extends TestCase $plugins = $this->pm->getPlugins(); $this->assertEquals('installer-v3', $plugins[1]->version); + $this->assertEquals('activate v2'.PHP_EOL.'deactivate v2'.PHP_EOL.'activate v3'.PHP_EOL, $this->io->getOutput()); } public function testRegisterPluginOnlyOneTime() @@ -213,6 +242,7 @@ class PluginInstallerTest extends TestCase $plugins = $this->pm->getPlugins(); $this->assertCount(1, $plugins); $this->assertEquals('installer-v1', $plugins[0]->version); + $this->assertEquals('activate v1'.PHP_EOL, $this->io->getOutput()); } /**