Skip to content

[FIX] Has plugin fix #576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jaxwilko wants to merge 7 commits into from
Closed

[FIX] Has plugin fix #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c30911
Added fix to ensure the provided value is returned unmodified if valu…
jaxwilko Jun 15, 2022
acd15fc
Added test to check for hasPlugin aliasing an uninstalled plugin
jaxwilko Jun 15, 2022
a6d3ad8
Added test plugin replacing an uninstalled plugin
jaxwilko Jun 15, 2022
28dce16
Added test case for hasPlugin with replaced & uninstalled plugins
jaxwilko Jun 15, 2022
9919a56
Added fix to handle plugin in replacement map without being in the no…
jaxwilko Jun 15, 2022
7c108f3
Added tests for resolving plugins with mixed casing
jaxwilko Jun 15, 2022
83c61f1
Added patch to allow for replacement map plugins to be resolved as lo…
jaxwilko Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions modules/system/classes/PluginManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand All @@ -473,7 +479,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)));
}

/**
Expand Down Expand Up @@ -571,8 +580,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;
}

/**
Expand Down
18 changes: 18 additions & 0 deletions tests/fixtures/plugins/winter/replacenotinstalled/Plugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php namespace Winter\ReplaceNotInstalled;

use System\Classes\PluginBase;

class Plugin extends PluginBase
{
public function pluginDetails()
{
return [
'name' => 'Winter Sample Plugin',
'description' => 'Sample plugin used by unit tests.',
'author' => 'Alexey Bobkov, Samuel Georges',
'replaces' => [
'Winter.NotInstalled' => '>=1.0.3'
]
];
}
}
2 changes: 2 additions & 0 deletions tests/fixtures/plugins/winter/replacenotinstalled/updates/version.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
1.0.0: Initial build
1.0.1: Updated plugin
108 changes: 104 additions & 4 deletions tests/unit/system/classes/PluginManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -346,6 +350,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();
Expand Down