Skip to content

Commit dd081a4

Browse files
lachiebollachiebol
committed
Apply PR feedback
1 parent 974e882 commit dd081a4

7 files changed

Lines changed: 27 additions & 156 deletions

File tree

‎API.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ class API extends \Piwik\Plugin\API
3030
*
3131
* @return array<int, string>
3232
*/
33-
public function getPluginWhitelist(): array
33+
public function getAllowedPlugins(): array
3434
{
3535
Piwik::checkUserHasSomeViewAccess();
3636

37-
return $this->getPluginListProvider()->getPluginsForSpecGeneration();
37+
return $this->getPluginListProvider()->getAllowedPlugins();
3838
}
3939

4040
/**

‎Generation/PluginListProvider.php‎

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,9 @@ public function __construct(?Manager $pluginManager = null)
3030
/**
3131
* @return string[]
3232
*/
33-
public function getPluginsForSpecGeneration(): array
33+
public function getAllowedPlugins(): array
3434
{
35-
$pluginNames = [];
36-
37-
foreach ($this->pluginManager->getInstalledPluginsName() as $pluginName) {
38-
if (!$this->shouldIncludePlugin($pluginName)) {
39-
continue;
40-
}
41-
42-
$pluginNames[] = $pluginName;
43-
}
35+
$pluginNames = array_values($this->pluginManager->getActivatedPlugins());
4436

4537
$this->dispatchUpdatePluginListEvent($pluginNames);
4638

@@ -51,22 +43,6 @@ public function getPluginsForSpecGeneration(): array
5143
}));
5244
}
5345

54-
private function shouldIncludePlugin(string $pluginName): bool
55-
{
56-
if (in_array($pluginName, OpenApiDocs::PLUGIN_BLOCKLIST, true)) {
57-
return false;
58-
}
59-
60-
if (
61-
!$this->pluginManager->isPluginActivated($pluginName)
62-
|| !$this->pluginManager->isPluginInFilesystem($pluginName)
63-
) {
64-
return false;
65-
}
66-
67-
return $this->pluginHasApiFile($pluginName);
68-
}
69-
7046
private function shouldIncludeEventProvidedPlugin(string $pluginName): bool
7147
{
7248
if (in_array($pluginName, OpenApiDocs::PLUGIN_BLOCKLIST, true)) {

‎Tasks.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function schedule()
5252

5353
public function generateConfiguredPluginSpecs(): void
5454
{
55-
$pluginNames = $this->pluginListProvider->getPluginsForSpecGeneration();
55+
$pluginNames = $this->pluginListProvider->getAllowedPlugins();
5656

5757
foreach ($pluginNames as $pluginName) {
5858
try {

‎config/plugins.php‎

Lines changed: 0 additions & 74 deletions
This file was deleted.

‎tests/Unit/APITest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,19 @@ public function testGetOpenApiSpecReturnsDecodedJsonForPlugin()
6262
$this->assertSame($expectedSpec, $result);
6363
}
6464

65-
public function testGetPluginWhitelistReturnsProviderValues(): void
65+
public function testGetAllowedPluginsReturnsProviderValues(): void
6666
{
6767
$provider = $this->createMock(PluginListProvider::class);
6868
$provider->expects($this->once())
69-
->method('getPluginsForSpecGeneration')
69+
->method('getAllowedPlugins')
7070
->willReturn(['Login', 'ActivityLog']);
7171

7272
$api = $this->getMockBuilder(API::class)
7373
->onlyMethods(['getPluginListProvider'])
7474
->getMock();
7575
$api->method('getPluginListProvider')->willReturn($provider);
7676

77-
$this->assertSame(['Login', 'ActivityLog'], $api->getPluginWhitelist());
77+
$this->assertSame(['Login', 'ActivityLog'], $api->getAllowedPlugins());
7878
}
7979

8080
public function testGetOpenApiSpecThrowsExceptionWhenFileMissing()

‎tests/Unit/Generation/PluginListProviderTest.php‎

Lines changed: 18 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,100 +24,76 @@
2424
*/
2525
class PluginListProviderTest extends TestCase
2626
{
27-
public function testGetPluginsForSpecGenerationIncludesPluginWhenEligible(): void
27+
public function testGetAllowedPluginsIncludesActivatedPluginWhenEligible(): void
2828
{
2929
$provider = $this->makeProvider(
3030
['HasApi'],
3131
['HasApi' => true],
32-
['HasApi' => true],
3332
true
3433
);
3534

36-
$this->assertSame(['HasApi'], $provider->getPluginsForSpecGeneration());
35+
$this->assertSame(['HasApi'], $provider->getAllowedPlugins());
3736
}
3837

39-
public function testGetPluginsForSpecGenerationExcludesBlocklistedPlugin(): void
38+
public function testGetAllowedPluginsExcludesBlocklistedPlugin(): void
4039
{
4140
$provider = $this->makeProvider(
4241
['ConnectAccounts'],
4342
['ConnectAccounts' => true],
44-
['ConnectAccounts' => true],
4543
true
4644
);
4745

48-
$this->assertSame([], $provider->getPluginsForSpecGeneration());
46+
$this->assertSame([], $provider->getAllowedPlugins());
4947
}
5048

51-
public function testGetPluginsForSpecGenerationExcludesInactivePlugin(): void
49+
public function testGetAllowedPluginsExcludesPluginNotInFilesystem(): void
5250
{
5351
$provider = $this->makeProvider(
5452
['InactivePlugin'],
5553
['InactivePlugin' => false],
56-
['InactivePlugin' => true],
57-
true
58-
);
59-
60-
$this->assertSame([], $provider->getPluginsForSpecGeneration());
61-
}
62-
63-
public function testGetPluginsForSpecGenerationExcludesPluginNotInFilesystem(): void
64-
{
65-
$provider = $this->makeProvider(
66-
['MissingPlugin'],
67-
['MissingPlugin' => true],
68-
['MissingPlugin' => false],
6954
true
7055
);
7156

72-
$this->assertSame([], $provider->getPluginsForSpecGeneration());
57+
$this->assertSame([], $provider->getAllowedPlugins());
7358
}
7459

75-
public function testGetPluginsForSpecGenerationExcludesPluginWithoutApiFile(): void
60+
public function testGetAllowedPluginsExcludesPluginWithoutApiFile(): void
7661
{
7762
$provider = $this->makeProvider(
7863
['NoApi'],
7964
['NoApi' => true],
80-
['NoApi' => true],
8165
false
8266
);
8367

84-
$this->assertSame([], $provider->getPluginsForSpecGeneration());
68+
$this->assertSame([], $provider->getAllowedPlugins());
8569
}
8670

87-
public function testGetPluginsForSpecGenerationAppliesEventUpdates(): void
71+
public function testGetAllowedPluginsAppliesEventUpdates(): void
8872
{
8973
$provider = $this->makeProvider(
9074
['HasApi', 'Login'],
9175
['HasApi' => true, 'Login' => true],
9276
['HasApi' => true, 'Login' => true],
93-
['HasApi' => true, 'Login' => true],
9477
static function (string $eventName, array $params): void {
9578
$pluginNames = &$params[0];
9679
$pluginNames[] = 'Login';
9780
}
9881
);
9982

100-
$this->assertSame(['HasApi', 'Login'], $provider->getPluginsForSpecGeneration());
83+
$this->assertSame(['HasApi', 'Login'], $provider->getAllowedPlugins());
10184
}
10285

103-
public function testGetPluginsForSpecGenerationReturnsEmptyListWhenNoPluginsInstalled(): void
86+
public function testGetAllowedPluginsReturnsEmptyListWhenNoPluginsInstalled(): void
10487
{
105-
$provider = $this->makeProvider([], [], [], false);
88+
$provider = $this->makeProvider([], [], false);
10689

107-
$this->assertSame([], $provider->getPluginsForSpecGeneration());
90+
$this->assertSame([], $provider->getAllowedPlugins());
10891
}
10992

110-
public function testGetPluginsForSpecGenerationDropsInvalidEventUpdatesButKeepsUnactivatedInstalledPlugins(): void
93+
public function testGetAllowedPluginsDropsInvalidEventUpdatesButKeepsEventAddedPlugins(): void
11194
{
11295
$provider = $this->makeProvider(
11396
['HasApi', 'Login', 'InactivePlugin', 'NoApi', 'ConnectAccounts'],
114-
[
115-
'HasApi' => true,
116-
'Login' => true,
117-
'InactivePlugin' => false,
118-
'NoApi' => true,
119-
'ConnectAccounts' => true,
120-
],
12197
[
12298
'HasApi' => true,
12399
'Login' => true,
@@ -142,32 +118,25 @@ static function (string $eventName, array $params): void {
142118
}
143119
);
144120

145-
$this->assertSame(['HasApi', 'Login', 'InactivePlugin'], $provider->getPluginsForSpecGeneration());
121+
$this->assertSame(['HasApi', 'Login', 'InactivePlugin'], $provider->getAllowedPlugins());
146122
}
147123

148124
/**
149-
* @param string[] $installedPlugins
150-
* @param array<string, bool> $activatedByPlugin
125+
* @param string[] $activatedPlugins
151126
* @param array<string, bool> $inFilesystemByPlugin
152127
* @param bool|array<string, bool> $hasApiFile
153128
*/
154129
private function makeProvider(
155-
array $installedPlugins,
156-
array $activatedByPlugin,
130+
array $activatedPlugins,
157131
array $inFilesystemByPlugin,
158132
$hasApiFile,
159133
?callable $postEventCallback = null
160134
): PluginListProvider {
161135
$pluginManager = $this->createConfiguredMock(Manager::class, [
162-
'getInstalledPluginsName' => $installedPlugins,
136+
'getActivatedPlugins' => $activatedPlugins,
163137
'getPluginsLoadedAndActivated' => [],
164138
]);
165139

166-
$pluginManager->method('isPluginActivated')
167-
->willReturnCallback(static function (string $pluginName) use ($activatedByPlugin): bool {
168-
return $activatedByPlugin[$pluginName] ?? false;
169-
});
170-
171140
$pluginManager->method('isPluginInFilesystem')
172141
->willReturnCallback(static function (string $pluginName) use ($inFilesystemByPlugin): bool {
173142
return $inFilesystemByPlugin[$pluginName] ?? false;

‎tests/Unit/TasksTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public function testGenerateConfiguredPluginSpecsLogsPerPluginFailuresAndContinu
8989

9090
$logger = new FakeLogger();
9191
$pluginListProvider = $this->createMock(PluginListProvider::class);
92-
$pluginListProvider->method('getPluginsForSpecGeneration')
92+
$pluginListProvider->method('getAllowedPlugins')
9393
->willReturn(['RollUpReporting', 'Login']);
9494

9595
$tasks = new Tasks($service, $logger, $pluginListProvider);

0 commit comments

Comments
 (0)