From 1f1889cf39d711805a474f283016ea958cbf95bd Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 21 Jun 2022 18:01:46 +0100 Subject: [PATCH 1/2] Added tests for replaced plugins --- .../tests/classes/PluginManagerTest.php | 112 ++++++++++++++++-- .../winter/replacenotinstalled/Plugin.php | 20 ++++ .../replacenotinstalled/updates/version.yaml | 2 + 3 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 modules/system/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php create mode 100644 modules/system/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml diff --git a/modules/system/tests/classes/PluginManagerTest.php b/modules/system/tests/classes/PluginManagerTest.php index ac5fb41a24..426cfb14ea 100644 --- a/modules/system/tests/classes/PluginManagerTest.php +++ b/modules/system/tests/classes/PluginManagerTest.php @@ -2,17 +2,20 @@ namespace System\Tests\Classes; -use Illuminate\Support\Facades\Artisan; use System\Tests\Bootstrap\TestCase; use System\Classes\PluginManager; use System\Classes\UpdateManager; use System\Classes\VersionManager; -use Cache; use ReflectionClass; + use Winter\Storm\Database\Model as ActiveRecord; class PluginManagerTest extends TestCase { + const INSTALLED_PLUGIN_COUNT = 13; + const ENABLED_PLUGIN_COUNT = 10; + const PLUGIN_NAMESPACE_COUNT = 14; + public $manager; protected $output; @@ -66,7 +69,6 @@ public function setUp() : void self::callProtectedMethod($manager, 'loadDisabled'); $manager->loadPlugins(); self::callProtectedMethod($manager, 'loadDependencies'); - self::callProtectedMethod($manager, 'detectPluginReplacements'); $this->manager = $manager; } @@ -128,7 +130,7 @@ public function testLoadPlugins() { $result = $this->manager->loadPlugins(); - $this->assertCount(12, $result); + $this->assertCount(static::INSTALLED_PLUGIN_COUNT, $result); $this->assertArrayHasKey('Winter.NoUpdates', $result); $this->assertArrayHasKey('Winter.Sample', $result); $this->assertArrayHasKey('Winter.Tester', $result); @@ -170,7 +172,7 @@ public function testGetPlugins() { $result = $this->manager->getPlugins(); - $this->assertCount(9, $result); + $this->assertCount(static::ENABLED_PLUGIN_COUNT, $result); $this->assertArrayHasKey('Winter.NoUpdates', $result); $this->assertArrayHasKey('Winter.Sample', $result); $this->assertArrayHasKey('Winter.Tester', $result); @@ -230,7 +232,7 @@ public function testGetPluginNamespaces() { $result = $this->manager->getPluginNamespaces(); - $this->assertCount(13, $result); + $this->assertCount(static::PLUGIN_NAMESPACE_COUNT, $result); $this->assertArrayHasKey('\winter\noupdates', $result); $this->assertArrayHasKey('\winter\sample', $result); $this->assertArrayHasKey('\winter\tester', $result); @@ -284,7 +286,7 @@ public function testPluginDetails() public function testUnregisterall() { $result = $this->manager->getPlugins(); - $this->assertCount(9, $result); + $this->assertCount(static::ENABLED_PLUGIN_COUNT, $result); $this->manager->unregisterAll(); $this->assertEmpty($this->manager->getPlugins()); @@ -353,6 +355,102 @@ public function testReplacement() $this->assertEquals('Winter.Replacement', $this->manager->findByIdentifier('Winter.Original')->getPluginIdentifier()); } + public function testHasPluginReplacement() + { + // check a replaced plugin + $this->assertTrue($this->manager->hasPlugin('Winter.Original')); + $this->assertTrue($this->manager->isDisabled('Winter.Original')); + // check a replacement plugin + $this->assertTrue($this->manager->hasPlugin('Winter.Replacement')); + $this->assertFalse($this->manager->isDisabled('Winter.Replacement')); + // check a plugin where the replacement is invalid + $this->assertTrue($this->manager->hasPlugin('Winter.InvalidReplacement')); + $this->assertTrue($this->manager->isDisabled('Winter.InvalidReplacement')); + // check a plugin replacing a plugin not found on disk + $this->assertTrue($this->manager->hasPlugin('Winter.ReplaceNotInstalled')); + $this->assertFalse($this->manager->isDisabled('Winter.ReplaceNotInstalled')); + // ensure searching for the alias of a replacement (plugin not installed) + $this->assertTrue($this->manager->hasPlugin('Winter.NotInstalled')); + + $this->assertInstanceOf(\Winter\Replacement\Plugin::class, $this->manager->findByIdentifier('Winter.Original')); + $this->assertInstanceOf(\Winter\Replacement\Plugin::class, $this->manager->findByIdentifier('Winter.Replacement')); + + // check getting a plugin via it's not installed original plugin identifier + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.NotInstalled')); + $this->assertNull($this->manager->findByIdentifier('Winter.NotInstalled', true)); + + // force getting the original plugin + $this->assertInstanceOf(\Winter\Original\Plugin::class, $this->manager->findByIdentifier('Winter.Original', true)); + } + + public function testHasPluginReplacementMixedCase() + { + // test checking casing of installed plugin (resolved via getNormalizedIdentifier()) + $this->assertTrue($this->manager->hasPlugin('Winter.ReplaceNotInstalled')); + $this->assertTrue($this->manager->hasPlugin('Winter.replaceNotInstalled')); + $this->assertTrue($this->manager->hasPlugin('Winter.replacenotInstalled')); + $this->assertTrue($this->manager->hasPlugin('winter.replacenotInstalled')); + $this->assertTrue($this->manager->hasPlugin('winter.replacenotinstalled')); + + // test checking casing of installed replaced plugin (resolved via getNormalizedIdentifier() & replacementMap) + $this->assertTrue($this->manager->hasPlugin('Winter.Original')); + $this->assertTrue($this->manager->hasPlugin('Winter.original')); + $this->assertTrue($this->manager->hasPlugin('winter.original')); + + // test checking casing of uninstalled plugin (resolved via strtolower() on replacement keys) + $this->assertTrue($this->manager->hasPlugin('Winter.NotInstalled')); + $this->assertTrue($this->manager->hasPlugin('Winter.notInstalled')); + $this->assertTrue($this->manager->hasPlugin('winter.notInstalled')); + $this->assertTrue($this->manager->hasPlugin('Winter.notinstalled')); + } + + public function testExistsReplacementMixedCase() + { + // test checking casing of installed plugin (resolved via getNormalizedIdentifier()) + $this->assertTrue($this->manager->exists('Winter.ReplaceNotInstalled')); + $this->assertTrue($this->manager->exists('Winter.replaceNotInstalled')); + $this->assertTrue($this->manager->exists('Winter.replacenotInstalled')); + $this->assertTrue($this->manager->exists('winter.replacenotInstalled')); + $this->assertTrue($this->manager->exists('winter.replacenotinstalled')); + + // test checking casing of installed replaced plugin (resolved via getNormalizedIdentifier() & replacementMap) + $this->assertFalse($this->manager->exists('Winter.Original')); + $this->assertFalse($this->manager->exists('Winter.original')); + $this->assertFalse($this->manager->exists('winter.original')); + + // test checking casing of uninstalled plugin (resolved via strtolower() on replacement keys) + $this->assertTrue($this->manager->exists('Winter.NotInstalled')); + $this->assertTrue($this->manager->exists('Winter.notInstalled')); + $this->assertTrue($this->manager->exists('winter.notInstalled')); + $this->assertTrue($this->manager->exists('Winter.notinstalled')); + } + + public function testFindByIdentifierReplacementMixedCase() + { + // test resolving plugin with mixed casing + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.ReplaceNotInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.replaceNotInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.replacenotInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('winter.replacenotInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('winter.replacenotinstalled')); + + // test resolving replacement plugin with mixed casing + $this->assertInstanceOf(\Winter\Replacement\Plugin::class, $this->manager->findByIdentifier('Winter.Original')); + $this->assertInstanceOf(\Winter\Replacement\Plugin::class, $this->manager->findByIdentifier('Winter.original')); + $this->assertInstanceOf(\Winter\Replacement\Plugin::class, $this->manager->findByIdentifier('winter.original')); + + // test resolving original plugin with mixed casing when ignoring replacements + $this->assertInstanceOf(\Winter\Original\Plugin::class, $this->manager->findByIdentifier('Winter.Original', true)); + $this->assertInstanceOf(\Winter\Original\Plugin::class, $this->manager->findByIdentifier('Winter.original', true)); + $this->assertInstanceOf(\Winter\Original\Plugin::class, $this->manager->findByIdentifier('winter.original', true)); + + // test resolving replacement plugin of uninstalled plugin with mixed casing + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.NotInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.notInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('winter.notInstalled')); + $this->assertInstanceOf(\Winter\ReplaceNotInstalled\Plugin::class, $this->manager->findByIdentifier('Winter.notinstalled')); + } + public function testGetReplacements() { $replacementPluginReplaces = $this->manager->findByIdentifier('Winter.Replacement')->getReplaces(); diff --git a/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php b/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php new file mode 100644 index 0000000000..963b667704 --- /dev/null +++ b/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php @@ -0,0 +1,20 @@ + 'Winter Sample Plugin', + 'description' => 'Sample plugin used by unit tests.', + 'author' => 'Alexey Bobkov, Samuel Georges', + 'replaces' => [ + 'Winter.NotInstalled' => '>=1.0.3' + ] + ]; + } +} diff --git a/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml b/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml new file mode 100644 index 0000000000..979448dee1 --- /dev/null +++ b/modules/system/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml @@ -0,0 +1,2 @@ +1.0.0: Initial build +1.0.1: Updated plugin From d3c36f58a20f294630c57c71903cc5be99060d03 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 21 Jun 2022 18:02:21 +0100 Subject: [PATCH 2/2] Updated the plugin manager to use lower case ids internally --- modules/system/classes/PluginManager.php | 79 ++++++++++++++++-------- 1 file changed, 53 insertions(+), 26 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 04fed249d8..73f2c75146 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -136,7 +136,7 @@ public function loadPlugins(): array // Sort all the plugins by number of dependencies $this->sortByDependencies(); - return $this->plugins; + return $this->getAllPlugins(); } /** @@ -173,14 +173,15 @@ public function loadPlugin(string $namespace, string $path): ?PluginBase } $classId = $this->getIdentifier($pluginObj); + $lowerClassId = strtolower($classId); - $this->plugins[$classId] = $pluginObj; - $this->normalizedMap[strtolower($classId)] = $classId; + $this->plugins[$lowerClassId] = $pluginObj; + $this->normalizedMap[$lowerClassId] = $classId; $replaces = $pluginObj->getReplaces(); if ($replaces) { foreach ($replaces as $replace) { - $this->replacementMap[$replace] = $classId; + $this->replacementMap[strtolower($replace)] = $lowerClassId; } } @@ -226,7 +227,7 @@ public function loadPluginFlags(): void ]; }); - list($this->pluginFlag, $this->replacementMap, $this->activeReplacementMap) = $data; + list($this->pluginFlags, $this->replacementMap, $this->activeReplacementMap) = $data; } /** @@ -409,7 +410,14 @@ public function exists(PluginBase|string $plugin): bool */ public function getPlugins(): array { - return array_diff_key($this->plugins, $this->pluginFlags); + $plugins = array_diff_key($this->plugins, $this->pluginFlags); + $keys = []; + + foreach ($plugins as $code => $plugin) { + $keys[] = $this->normalizedMap[$code]; + } + + return array_combine($keys, $plugins); } /** @@ -419,7 +427,13 @@ public function getPlugins(): array */ public function getAllPlugins(): array { - return $this->plugins; + $plugins = []; + + foreach ($this->plugins as $code => $plugin) { + $plugins[$this->normalizedMap[$code]] = $plugin; + } + + return $plugins; } /** @@ -427,7 +441,7 @@ public function getAllPlugins(): array */ public function findByNamespace(string $namespace): ?PluginBase { - $identifier = $this->getIdentifier($namespace); + $identifier = $this->getIdentifier($namespace, true); return $this->plugins[$identifier] ?? null; } @@ -441,12 +455,10 @@ public function findByIdentifier(PluginBase|string $identifier, bool $ignoreRepl return $identifier; } - if (!$ignoreReplacements && is_string($identifier) && isset($this->replacementMap[$identifier])) { - $identifier = $this->replacementMap[$identifier]; - } + $identifier = $this->getNormalizedIdentifier($identifier, true); - if (!isset($this->plugins[$identifier])) { - $identifier = $this->getNormalizedIdentifier($identifier); + if (!$ignoreReplacements && isset($this->replacementMap[$identifier])) { + $identifier = $this->replacementMap[$identifier]; } return $this->plugins[$identifier] ?? null; @@ -457,7 +469,7 @@ public function findByIdentifier(PluginBase|string $identifier, bool $ignoreRepl */ public function hasPlugin(PluginBase|string $plugin): bool { - $normalized = $this->getNormalizedIdentifier($plugin); + $normalized = $this->getNormalizedIdentifier($plugin, true); return isset($this->plugins[$normalized]) || isset($this->replacementMap[$normalized]); } @@ -518,7 +530,7 @@ public function getVendorAndPluginNames(): array * Resolves a plugin identifier (Author.Plugin) from a plugin class name * (Author\Plugin) or PluginBase instance. */ - public function getIdentifier(PluginBase|string $plugin): string + public function getIdentifier(PluginBase|string $plugin, bool $lower = false): string { $namespace = Str::normalizeClassName($plugin); if (strpos($namespace, '\\') === null) { @@ -529,7 +541,7 @@ public function getIdentifier(PluginBase|string $plugin): string $slice = array_slice($parts, 1, 2); $namespace = implode('.', $slice); - return $namespace; + return $lower ? strtolower($namespace) : $namespace; } /** @@ -565,9 +577,10 @@ public function normalizeIdentifier(string $code): string * Returns the normalized identifier (i.e. Winter.Blog) from the provided * string or PluginBase instance. */ - public function getNormalizedIdentifier(PluginBase|string $plugin): string + public function getNormalizedIdentifier(PluginBase|string $plugin, bool $lower = false): string { - return $this->normalizeIdentifier($this->getIdentifier($plugin)); + $identifier = $this->normalizeIdentifier($this->getIdentifier($plugin)); + return $lower ? strtolower($identifier) : $identifier; } /** @@ -600,7 +613,7 @@ public function getRegistrationMethodValues(string $methodName): array public function getPluginFlags(PluginBase|string $plugin): array { - $code = $this->getNormalizedIdentifier($plugin); + $code = $this->getNormalizedIdentifier($plugin, true); return $this->pluginFlags[$code] ?? []; } @@ -609,7 +622,7 @@ public function getPluginFlags(PluginBase|string $plugin): array */ protected function flagPlugin(PluginBase|string $plugin, string $flag): void { - $code = $this->getNormalizedIdentifier($plugin); + $code = $this->getNormalizedIdentifier($plugin, true); $this->pluginFlags[$code][$flag] = true; } @@ -618,7 +631,7 @@ protected function flagPlugin(PluginBase|string $plugin, string $flag): void */ protected function unflagPlugin(PluginBase|string $plugin, string $flag): void { - $code = $this->getNormalizedIdentifier($plugin); + $code = $this->getNormalizedIdentifier($plugin, true); unset($this->pluginFlags[$code][$flag]); } @@ -652,7 +665,7 @@ protected function loadDisabled(): void */ public function isDisabled(PluginBase|string $plugin): bool { - $code = $this->getNormalizedIdentifier($plugin); + $code = $this->getNormalizedIdentifier($plugin, true); // @TODO: Limit this to only disabled flags if we add more than disabled flags return !empty($this->pluginFlags[$code]); @@ -671,9 +684,18 @@ public function getReplacementMap(): array */ public function getActiveReplacementMap(PluginBase|string $plugin = null): array|string|null { - return $plugin - ? $this->activeReplacementMap[$this->getNormalizedIdentifier($plugin)] ?? null - : $this->activeReplacementMap; + if ($plugin) { + return $this->normalizedMap[ + $this->activeReplacementMap[$this->getNormalizedIdentifier($plugin, true)] ?? null + ] ?? null; + } + + $map = []; + foreach ($this->activeReplacementMap as $key => $value) { + $map[$this->normalizedMap[$key]] = $this->normalizedMap[$value]; + } + + return $map; } /** @@ -694,7 +716,12 @@ protected function detectPluginReplacements(): void // Only allow one of the replaced plugin or the replacing plugin to exist // at once depending on whether the version constraints are met or not - if ($this->plugins[$replacement]->canReplacePlugin($target, $this->plugins[$target]->getPluginVersion())) { + if ( + $this->plugins[$replacement]->canReplacePlugin( + $this->normalizeIdentifier($target), + $this->plugins[$target]->getPluginVersion() + ) + ) { // Set the plugin flags to disable the target plugin $this->flagPlugin($target, static::DISABLED_REPLACED); $this->unflagPlugin($replacement, static::DISABLED_REPLACEMENT_FAILED);