From 3c30911c13cc89b3cd9bd32b295978f52afb47df Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 17:59:48 +0100 Subject: [PATCH 1/7] Added fix to ensure the provided value is returned unmodified if value not found --- modules/system/classes/PluginManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index f2ba9cdbbc..7dca29e594 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -571,8 +571,7 @@ public function getNamespace(PluginBase|string $plugin): string */ public function normalizeIdentifier(string $code): string { - $code = strtolower($code); - return $this->normalizedMap[$code] ?? $code; + return $this->normalizedMap[strtolower($code)] ?? $code; } /** From acd15fcedaa85013e5026694b2a67bf2b886831d Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 18:00:35 +0100 Subject: [PATCH 2/7] Added test to check for hasPlugin aliasing an uninstalled plugin --- .../unit/system/classes/PluginManagerTest.php | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 10667f5d29..42328b8fce 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -7,6 +7,10 @@ class PluginManagerTest extends TestCase { + const INSTALLED_PLUGIN_COUNT = 13; + const ENABLED_PLUGIN_COUNT = 10; + const PLUGIN_NAMESPACE_COUNT = 14; + public $manager; protected $output; @@ -121,7 +125,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); @@ -163,7 +167,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); @@ -223,7 +227,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); @@ -277,7 +281,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()); @@ -346,6 +350,34 @@ 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 testGetReplacements() { $replacementPluginReplaces = $this->manager->findByIdentifier('Winter.Replacement')->getReplaces(); From a6d3ad8c19168ce9df079806f514d4931497cc94 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 18:08:31 +0100 Subject: [PATCH 3/7] Added test plugin replacing an uninstalled plugin --- .../winter/replacenotinstalled/Plugin.php | 18 ++++++++++++++++++ .../replacenotinstalled/updates/version.yaml | 2 ++ 2 files changed, 20 insertions(+) create mode 100644 tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php create mode 100644 tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml diff --git a/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php b/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php new file mode 100644 index 0000000000..531018adf1 --- /dev/null +++ b/tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php @@ -0,0 +1,18 @@ + 'Winter Sample Plugin', + 'description' => 'Sample plugin used by unit tests.', + 'author' => 'Alexey Bobkov, Samuel Georges', + 'replaces' => [ + 'Winter.NotInstalled' => '>=1.0.3' + ] + ]; + } +} diff --git a/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml b/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml new file mode 100644 index 0000000000..979448dee1 --- /dev/null +++ b/tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml @@ -0,0 +1,2 @@ +1.0.0: Initial build +1.0.1: Updated plugin From 28dce16262974f812100dc0e6e1ac2c3be9dc1d3 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 19:44:09 +0100 Subject: [PATCH 4/7] Added test case for hasPlugin with replaced & uninstalled plugins --- .../unit/system/classes/PluginManagerTest.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 42328b8fce..3d85761f4d 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -378,6 +378,27 @@ public function testHasPluginReplacement() $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 testGetReplacements() { $replacementPluginReplaces = $this->manager->findByIdentifier('Winter.Replacement')->getReplaces(); From 9919a56ba51ca49c6c111e4eca88fb4545abb82d Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 19:44:42 +0100 Subject: [PATCH 5/7] Added fix to handle plugin in replacement map without being in the normalized array --- modules/system/classes/PluginManager.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 7dca29e594..ea6f7c5811 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -473,7 +473,10 @@ public function hasPlugin(PluginBase|string $plugin): bool { $normalized = $this->getNormalizedIdentifier($plugin); - return isset($this->plugins[$normalized]) || isset($this->replacementMap[$normalized]); + return isset($this->plugins[$normalized]) + || isset($this->replacementMap[$normalized]) + // TODO: we should refactor the plugin manager to handle identifiers internally as lowers + || in_array(strtolower($normalized), array_map('strtolower', array_keys($this->replacementMap))); } /** From 7c108f35ce430b0b2c5d853356d8cc22e67fdb72 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 20:00:09 +0100 Subject: [PATCH 6/7] Added tests for resolving plugins with mixed casing --- .../unit/system/classes/PluginManagerTest.php | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 3d85761f4d..8fb863d7b5 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -399,6 +399,53 @@ public function testHasPluginReplacementMixedCase() $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(); From 83c61f16d5b7fbae87ad34fe2f9aaf99d8c6d36c Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 15 Jun 2022 20:00:41 +0100 Subject: [PATCH 7/7] Added patch to allow for replacement map plugins to be resolved as lower case --- modules/system/classes/PluginManager.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index ea6f7c5811..e9e98e2ca8 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -455,8 +455,14 @@ public function findByIdentifier(PluginBase|string $identifier, bool $ignoreRepl return $identifier; } - if (!$ignoreReplacements && is_string($identifier) && isset($this->replacementMap[$identifier])) { - $identifier = $this->replacementMap[$identifier]; + // TODO: we should refactor the plugin manager to handle identifiers internally as lowers + $replacementMap = array_combine( + array_map('strtolower', array_keys($this->replacementMap)), + array_values($this->replacementMap) + ); + + if (!$ignoreReplacements && is_string($identifier) && isset($replacementMap[strtolower($identifier)])) { + $identifier = $replacementMap[strtolower($identifier)]; } if (!isset($this->plugins[$identifier])) {