From 162497e4d9465fae95cb44b6135506eac54c9a05 Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Wed, 6 Feb 2019 10:15:26 +0100 Subject: [PATCH] More refactoring --- src/ComposerInstaller/Installer.php | 8 +- src/ComposerInstaller/Plugin.php | 8 +- src/ComposerInstaller/PluginInstaller.php | 8 +- tests/ComposerInstaller/CmsInstallerTest.php | 37 ++------ tests/ComposerInstaller/InstallerTestCase.php | 22 ++++- .../ComposerInstaller/PluginInstallerTest.php | 85 ++++++++----------- 6 files changed, 72 insertions(+), 96 deletions(-) diff --git a/src/ComposerInstaller/Installer.php b/src/ComposerInstaller/Installer.php index 3e5ac8d..5d698ac 100644 --- a/src/ComposerInstaller/Installer.php +++ b/src/ComposerInstaller/Installer.php @@ -22,9 +22,9 @@ class Installer extends LibraryInstaller * @param string $packageType * @return bool */ - public function supports($packageType) + public function supports($packageType): bool { - throw new RuntimeException('This method needs to be overridden.'); + throw new RuntimeException('This method needs to be overridden.'); // @codeCoverageIgnore } /** @@ -73,9 +73,7 @@ protected function postInstall(PackageInterface $package) if (is_dir($packageVendorDir)) { $success = $this->filesystem->removeDirectory($packageVendorDir); if (!$success) { - // @codeCoverageIgnoreStart - throw new RuntimeException('Could not completely delete ' . $path . ', aborting.'); - // @codeCoverageIgnoreEnd + throw new RuntimeException('Could not completely delete ' . $path . ', aborting.'); // @codeCoverageIgnore } } } diff --git a/src/ComposerInstaller/Plugin.php b/src/ComposerInstaller/Plugin.php index dfba2d8..79029f2 100644 --- a/src/ComposerInstaller/Plugin.php +++ b/src/ComposerInstaller/Plugin.php @@ -23,10 +23,8 @@ class Plugin implements PluginInterface */ public function activate(Composer $composer, IOInterface $io) { - $cmsInstaller = new CmsInstaller($io, $composer); - $composer->getInstallationManager()->addInstaller($cmsInstaller); - - $pluginInstaller = new PluginInstaller($io, $composer); - $composer->getInstallationManager()->addInstaller($pluginInstaller); + $installationManager = $composer->getInstallationManager(); + $installationManager->addInstaller(new CmsInstaller($io, $composer)); + $installationManager->addInstaller(new PluginInstaller($io, $composer)); } } diff --git a/src/ComposerInstaller/PluginInstaller.php b/src/ComposerInstaller/PluginInstaller.php index 023be97..7aedd89 100644 --- a/src/ComposerInstaller/PluginInstaller.php +++ b/src/ComposerInstaller/PluginInstaller.php @@ -33,7 +33,7 @@ public function supports($packageType): bool public function getInstallPath(PackageInterface $package): string { // place into `vendor` directory as usual if Pluginkit is not supported - if (!$this->supportsPluginkit($package)) { + if ($this->supportsPluginkit($package) !== true) { return parent::getInstallPath($package); } @@ -73,7 +73,7 @@ public function getInstallPath(PackageInterface $package): string protected function postInstall(PackageInterface $package) { // only continue if Pluginkit is supported - if (!$this->supportsPluginkit($package)) { + if ($this->supportsPluginkit($package) !== true) { return; } @@ -82,8 +82,8 @@ protected function postInstall(PackageInterface $package) /** * Checks if the package has explicitly required this installer; - * otherwise the installer will fall back to the behavior of the LibraryInstaller - * (Pluginkit is not yet supported by the plugin) + * otherwise (if the Pluginkit is not yet supported by the plugin) + * the installer will fall back to the behavior of the LibraryInstaller * * @param PackageInterface $package * @return bool diff --git a/tests/ComposerInstaller/CmsInstallerTest.php b/tests/ComposerInstaller/CmsInstallerTest.php index ee9100b..d543d5f 100644 --- a/tests/ComposerInstaller/CmsInstallerTest.php +++ b/tests/ComposerInstaller/CmsInstallerTest.php @@ -3,7 +3,6 @@ namespace Kirby\ComposerInstaller; use Composer\Package\Package; -use Composer\Package\RootPackage; use Composer\Repository\InstalledArrayRepository; class CmsInstallerTest extends InstallerTestCase @@ -30,11 +29,9 @@ public function testGetInstallPathDefault() public function testGetInstallPathCustomPaths() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $rootPackage->setExtra([ + $this->initRootPackage()->setExtra([ 'kirby-cms-path' => 'cms' ]); - $this->composer->setPackage($rootPackage); $package = $this->cmsPackageFactory(); $this->assertEquals('cms', $this->installer->getInstallPath($package)); @@ -46,11 +43,9 @@ public function testGetInstallPathCustomPaths() */ public function testGetInstallPathUnsafe1() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $rootPackage->setExtra([ + $this->initRootPackage()->setExtra([ 'kirby-cms-path' => '.' ]); - $this->composer->setPackage($rootPackage); $package = $this->cmsPackageFactory(); $this->installer->getInstallPath($package); @@ -62,17 +57,9 @@ public function testGetInstallPathUnsafe1() */ public function testGetInstallPathUnsafe2() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $rootPackage->setExtra([ + $this->initRootPackage()->setExtra([ 'kirby-cms-path' => 'vendor' ]); - $this->composer->setPackage($rootPackage); - - $this->composer->getConfig()->merge([ - 'config' => [ - 'vendor-dir' => 'vendor' - ] - ]); $package = $this->cmsPackageFactory(); $this->installer->getInstallPath($package); @@ -84,11 +71,9 @@ public function testGetInstallPathUnsafe2() */ public function testGetInstallPathUnsafe3() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $rootPackage->setExtra([ + $this->initRootPackage()->setExtra([ 'kirby-cms-path' => 'custom-vendor' ]); - $this->composer->setPackage($rootPackage); $package = $this->cmsPackageFactory(); $this->assertEquals('custom-vendor', $this->installer->getInstallPath($package)); @@ -105,9 +90,6 @@ public function testGetInstallPathUnsafe3() public function testInstall() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - $package = $this->cmsPackageFactory(); $this->assertEquals('kirby', $this->installer->getInstallPath($package)); $this->installer->install(new InstalledArrayRepository(), $package); @@ -116,13 +98,10 @@ public function testInstall() $this->assertDirectoryNotExists($this->testDir . '/kirby/vendor'); } - public function testUpdateCode() + public function testUpdate() { $repo = new InstalledArrayRepository(); - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - $initial = $this->cmsPackageFactory(); $this->assertEquals('kirby', $this->installer->getInstallPath($initial)); $this->installer->install($repo, $initial); @@ -131,8 +110,10 @@ public function testUpdateCode() $this->assertFileExists($this->testDir . '/kirby/vendor-created.txt'); $this->assertDirectoryNotExists($this->testDir . '/kirby/vendor'); - unlink($this->testDir . '/kirby/index.php'); + $this->filesystem->emptyDirectory($this->testDir . '/kirby'); $this->assertFileNotExists($this->testDir . '/kirby/index.php'); + $this->assertFileNotExists($this->testDir . '/kirby/vendor-created.txt'); + $this->assertDirectoryNotExists($this->testDir . '/kirby/vendor'); $target = $this->cmsPackageFactory(); $this->assertEquals('kirby', $this->installer->getInstallPath($target)); @@ -154,7 +135,7 @@ protected function cmsPackageFactory(): Package $package->setInstallationSource('dist'); $package->setDistType('mock'); $package->setExtra([ - 'with-vendor-dir' => true + 'with-vendor-dir' => true // tell the MockDownloader to create a `vendor` dir ]); return $package; diff --git a/tests/ComposerInstaller/InstallerTestCase.php b/tests/ComposerInstaller/InstallerTestCase.php index b08f4eb..489a930 100644 --- a/tests/ComposerInstaller/InstallerTestCase.php +++ b/tests/ComposerInstaller/InstallerTestCase.php @@ -8,15 +8,18 @@ use Composer\Config; use Composer\Downloader\DownloadManager; use Composer\IO\NullIO; +use Composer\Package\RootPackage; use Composer\Util\Filesystem; class InstallerTestCase extends TestCase { protected $testDir; - protected $io; + protected $composer; protected $filesystem; protected $installer; + protected $io; + protected $rootPackage; public function setUp() { @@ -32,9 +35,7 @@ public function setUp() $this->composer->setDownloadManager($downloadManager); // initialize test dir and switch to it to make relative paths work - if (!is_dir($this->testDir)) { - $this->filesystem->ensureDirectoryExists($this->testDir); - } + $this->filesystem->ensureDirectoryExists($this->testDir); chdir($this->testDir); } @@ -42,4 +43,17 @@ public function tearDown() { $this->filesystem->removeDirectory($this->testDir); } + + /** + * Initializes a root Kirby site package and returns it + * + * @return RootPackage + */ + public function initRootPackage(): RootPackage + { + $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); + $this->composer->setPackage($rootPackage); + + return $rootPackage; + } } diff --git a/tests/ComposerInstaller/PluginInstallerTest.php b/tests/ComposerInstaller/PluginInstallerTest.php index 19a581a..1eecd23 100644 --- a/tests/ComposerInstaller/PluginInstallerTest.php +++ b/tests/ComposerInstaller/PluginInstallerTest.php @@ -4,11 +4,13 @@ use Composer\Package\Link; use Composer\Package\Package; -use Composer\Package\RootPackage; use Composer\Repository\InstalledArrayRepository; class PluginInstallerTest extends InstallerTestCase { + const SUPPORTED = 1; + const VENDOR_DIR = 2; + public function setUp() { parent::setUp(); @@ -25,29 +27,26 @@ public function testSupports() public function testGetInstallPathNoSupport() { - $package = $this->pluginPackageFactory(false, false); + $package = $this->pluginPackageFactory(); $this->assertEquals($this->testDir . '/vendor/superwoman/superplugin', $this->installer->getInstallPath($package)); } public function testGetInstallPathDefault() { - $package = $this->pluginPackageFactory(true, false); + $package = $this->pluginPackageFactory(self::SUPPORTED); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($package)); } public function testGetInstallPathNoVendor() { - $package = new Package('superplugin', '1.0.0.0', '1.0.0'); - $package->setType('kirby-plugin'); - $package->setRequires([ - new Link('superwoman/superplugin', 'getkirby/composer-installer') - ]); + $package = $this->pluginPackageFactory(self::SUPPORTED, 'superplugin'); + $this->assertEquals('superplugin', $package->getPrettyName()); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($package)); } public function testGetInstallPathInstallerName() { - $package = $this->pluginPackageFactory(true, false); + $package = $this->pluginPackageFactory(self::SUPPORTED); $package->setExtra([ 'installer-name' => 'another-name' ]); @@ -56,16 +55,14 @@ public function testGetInstallPathInstallerName() public function testGetInstallPathCustomPaths() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $rootPackage->setExtra([ + $this->initRootPackage()->setExtra([ 'kirby-plugin-path' => 'data/plugins' ]); - $this->composer->setPackage($rootPackage); - $package = $this->pluginPackageFactory(true, false); + $package = $this->pluginPackageFactory(self::SUPPORTED); $this->assertEquals('data/plugins/superplugin', $this->installer->getInstallPath($package)); - $package = $this->pluginPackageFactory(true, false); + $package = $this->pluginPackageFactory(self::SUPPORTED); $package->setExtra([ 'installer-name' => 'another-name' ]); @@ -74,10 +71,7 @@ public function testGetInstallPathCustomPaths() public function testInstallNoSupport() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $package = $this->pluginPackageFactory(false, true); + $package = $this->pluginPackageFactory(self::VENDOR_DIR); $this->assertEquals($this->testDir . '/vendor/superwoman/superplugin', $this->installer->getInstallPath($package)); $this->installer->install(new InstalledArrayRepository(), $package); $this->assertFileExists($this->testDir . '/vendor/superwoman/superplugin/index.php'); @@ -87,10 +81,7 @@ public function testInstallNoSupport() public function testInstallWithoutVendorDir() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $package = $this->pluginPackageFactory(true, false); + $package = $this->pluginPackageFactory(self::SUPPORTED); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($package)); $this->installer->install(new InstalledArrayRepository(), $package); $this->assertFileExists($this->testDir . '/site/plugins/superplugin/index.php'); @@ -100,10 +91,7 @@ public function testInstallWithoutVendorDir() public function testInstallVendorDir() { - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $package = $this->pluginPackageFactory(true, true); + $package = $this->pluginPackageFactory(self::SUPPORTED | self::VENDOR_DIR); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($package)); $this->installer->install(new InstalledArrayRepository(), $package); $this->assertFileExists($this->testDir . '/site/plugins/superplugin/index.php'); @@ -115,10 +103,7 @@ public function testUpdateNoSupport() { $repo = new InstalledArrayRepository(); - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $initial = $this->pluginPackageFactory(false, true); + $initial = $this->pluginPackageFactory(self::VENDOR_DIR); $this->assertEquals($this->testDir . '/vendor/superwoman/superplugin', $this->installer->getInstallPath($initial)); $this->installer->install($repo, $initial); $repo->addPackage($initial); @@ -126,10 +111,12 @@ public function testUpdateNoSupport() $this->assertFileExists($this->testDir . '/vendor/superwoman/superplugin/vendor-created.txt'); $this->assertDirectoryExists($this->testDir . '/vendor/superwoman/superplugin/vendor'); - unlink($this->testDir . '/vendor/superwoman/superplugin/index.php'); + $this->filesystem->emptyDirectory($this->testDir . '/vendor/superwoman/superplugin'); $this->assertFileNotExists($this->testDir . '/vendor/superwoman/superplugin/index.php'); + $this->assertFileNotExists($this->testDir . '/vendor/superwoman/superplugin/vendor-created.txt'); + $this->assertDirectoryNotExists($this->testDir . '/vendor/superwoman/superplugin/vendor'); - $target = $this->pluginPackageFactory(false, true); + $target = $this->pluginPackageFactory(self::VENDOR_DIR); $this->assertEquals($this->testDir . '/vendor/superwoman/superplugin', $this->installer->getInstallPath($target)); $this->installer->update($repo, $initial, $target); $this->assertFileExists($this->testDir . '/vendor/superwoman/superplugin/index.php'); @@ -141,10 +128,7 @@ public function testUpdateWithoutVendorDir() { $repo = new InstalledArrayRepository(); - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $initial = $this->pluginPackageFactory(true, false); + $initial = $this->pluginPackageFactory(self::SUPPORTED); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($initial)); $this->installer->install($repo, $initial); $repo->addPackage($initial); @@ -152,10 +136,12 @@ public function testUpdateWithoutVendorDir() $this->assertFileNotExists($this->testDir . '/site/plugins/superplugin/vendor-created.txt'); $this->assertDirectoryNotExists($this->testDir . '/site/plugins/superplugin/vendor'); - unlink($this->testDir . '/site/plugins/superplugin/index.php'); + $this->filesystem->emptyDirectory($this->testDir . '/site/plugins/superplugin'); $this->assertFileNotExists($this->testDir . '/site/plugins/superplugin/index.php'); + $this->assertFileNotExists($this->testDir . '/site/plugins/superplugin/vendor-created.txt'); + $this->assertDirectoryNotExists($this->testDir . '/site/plugins/superplugin/vendor'); - $target = $this->pluginPackageFactory(true, false); + $target = $this->pluginPackageFactory(self::SUPPORTED); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($target)); $this->installer->update($repo, $initial, $target); $this->assertFileExists($this->testDir . '/site/plugins/superplugin/index.php'); @@ -167,10 +153,7 @@ public function testUpdateVendorDir() { $repo = new InstalledArrayRepository(); - $rootPackage = new RootPackage('getkirby/amazing-site', '1.0.0.0', '1.0.0'); - $this->composer->setPackage($rootPackage); - - $initial = $this->pluginPackageFactory(true, true); + $initial = $this->pluginPackageFactory(self::SUPPORTED | self::VENDOR_DIR); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($initial)); $this->installer->install($repo, $initial); $repo->addPackage($initial); @@ -178,10 +161,12 @@ public function testUpdateVendorDir() $this->assertFileExists($this->testDir . '/site/plugins/superplugin/vendor-created.txt'); $this->assertDirectoryNotExists($this->testDir . '/site/plugins/superplugin/vendor'); - unlink($this->testDir . '/site/plugins/superplugin/index.php'); + $this->filesystem->emptyDirectory($this->testDir . '/site/plugins/superplugin'); $this->assertFileNotExists($this->testDir . '/site/plugins/superplugin/index.php'); + $this->assertFileNotExists($this->testDir . '/site/plugins/superplugin/vendor-created.txt'); + $this->assertDirectoryNotExists($this->testDir . '/site/plugins/superplugin/vendor'); - $target = $this->pluginPackageFactory(true, true); + $target = $this->pluginPackageFactory(self::SUPPORTED | self::VENDOR_DIR); $this->assertEquals('site/plugins/superplugin', $this->installer->getInstallPath($target)); $this->installer->update($repo, $initial, $target); $this->assertFileExists($this->testDir . '/site/plugins/superplugin/index.php'); @@ -192,24 +177,24 @@ public function testUpdateVendorDir() /** * Creates a dummy plugin package * - * @param bool $supported Whether the plugin package has required the installer - * @param bool $vendorDir Whether the plugin has a vendor directory committed + * @param int $flags Combination of self::SUPPORTED and self::VENDOR_DIR + * @param string $name Custom package name of the plugin package * @return Package */ - protected function pluginPackageFactory(bool $supported, bool $vendorDir): Package + protected function pluginPackageFactory(int $flags = 0, string $name = 'superwoman/superplugin'): Package { - $package = new Package('superwoman/superplugin', '1.0.0.0', '1.0.0'); + $package = new Package($name, '1.0.0.0', '1.0.0'); $package->setType('kirby-plugin'); $package->setInstallationSource('dist'); $package->setDistType('mock'); - if ($supported === true) { + if ($flags & self::SUPPORTED) { $package->setRequires([ new Link('superwoman/superplugin', 'getkirby/composer-installer') ]); } - if ($vendorDir === true) { + if ($flags & self::VENDOR_DIR) { $package->setExtra([ 'with-vendor-dir' => true ]);