From 42f0135704643b81212170c27387241f684286a6 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 17 Aug 2019 10:57:38 +0800 Subject: [PATCH] Check plugin dependencies at PluginManager --- app/Console/Commands/PluginEnableCommand.php | 9 ++++--- app/Http/Controllers/PluginController.php | 13 +++++---- app/Services/PluginManager.php | 12 +++++++++ .../CommandsTest/PluginEnableCommandTest.php | 4 +-- tests/PluginControllerTest.php | 27 +++++++------------ tests/ServicesTest/PluginManagerTest.php | 12 ++++++--- 6 files changed, 43 insertions(+), 34 deletions(-) diff --git a/app/Console/Commands/PluginEnableCommand.php b/app/Console/Commands/PluginEnableCommand.php index e62e415d..f7bc2626 100644 --- a/app/Console/Commands/PluginEnableCommand.php +++ b/app/Console/Commands/PluginEnableCommand.php @@ -28,11 +28,12 @@ class PluginEnableCommand extends Command */ public function handle(PluginManager $plugins) { - $plugin = $plugins->get($this->argument('name')); - if ($plugin) { - $plugins->enable($this->argument('name')); + $name = $this->argument('name'); + $result = $plugins->enable($name); + if ($result === true) { + $plugin = $plugins->get($name); $this->info(trans('admin.plugins.operations.enabled', ['plugin' => $plugin->title])); - } else { + } elseif ($result === false) { $this->warn(trans('admin.plugins.operations.not-found')); } } diff --git a/app/Http/Controllers/PluginController.php b/app/Http/Controllers/PluginController.php index 42e235a8..6f0fc2a4 100644 --- a/app/Http/Controllers/PluginController.php +++ b/app/Http/Controllers/PluginController.php @@ -29,9 +29,12 @@ class PluginController extends Controller switch ($request->get('action')) { case 'enable': - $requirements = $plugins->getUnsatisfied($plugin); - if ($requirements->isNotEmpty()) { - $reason = $requirements->map(function ($detail, $name) { + $result = $plugins->enable($name); + + if ($result === true) { + return json(trans('admin.plugins.operations.enabled', ['plugin' => $plugin->title]), 0); + } else { + $reason = $result['unsatisfied']->map(function ($detail, $name) { $constraint = $detail['constraint']; if (! $detail['version']) { return trans('admin.plugins.operations.unsatisfied.disabled', compact('name')); @@ -43,10 +46,6 @@ class PluginController extends Controller return json(trans('admin.plugins.operations.unsatisfied.notice'), 1, compact('reason')); } - $plugins->enable($name); - - return json(trans('admin.plugins.operations.enabled', ['plugin' => $plugin->title]), 0); - case 'disable': $plugins->disable($name); diff --git a/app/Services/PluginManager.php b/app/Services/PluginManager.php index 35601245..1e7f7cdd 100644 --- a/app/Services/PluginManager.php +++ b/app/Services/PluginManager.php @@ -246,16 +246,28 @@ class PluginManager return $this->all()->get($name); } + /** + * @return bool|array Return `true` if succeeded, or return information if failed. + */ public function enable($plugin) { $plugin = is_string($plugin) ? $this->get($plugin) : $plugin; if ($plugin && ! $plugin->isEnabled()) { + $unsatisfied = $this->getUnsatisfied($plugin); + if ($unsatisfied->isNotEmpty()) { + return compact('unsatisfied'); + } + $this->enabled->put($plugin->name, ['version' => $plugin->version]); $this->saveEnabled(); $plugin->setEnabled(true); $this->dispatcher->dispatch(new Events\PluginWasEnabled($plugin)); + + return true; + } else { + return false; } } diff --git a/tests/CommandsTest/PluginEnableCommandTest.php b/tests/CommandsTest/PluginEnableCommandTest.php index 3fa7221c..f92c7836 100644 --- a/tests/CommandsTest/PluginEnableCommandTest.php +++ b/tests/CommandsTest/PluginEnableCommandTest.php @@ -10,12 +10,12 @@ class PluginEnableCommandTest extends TestCase public function testEnablePlugin() { $this->mock(PluginManager::class, function ($mock) { - $mock->shouldReceive('get')->with('nope')->once()->andReturn(null); $mock->shouldReceive('get') ->with('my-plugin') ->once() ->andReturn(new Plugin('', ['title' => 'My Plugin'])); - $mock->shouldReceive('enable')->with('my-plugin')->once(); + $mock->shouldReceive('enable')->with('nope')->once()->andReturn(false); + $mock->shouldReceive('enable')->with('my-plugin')->once()->andReturn(true); }); $this->artisan('plugin:enable nope') diff --git a/tests/PluginControllerTest.php b/tests/PluginControllerTest.php index 34443f23..b1d97ff8 100644 --- a/tests/PluginControllerTest.php +++ b/tests/PluginControllerTest.php @@ -89,17 +89,15 @@ class PluginControllerTest extends TestCase ->with('fake2') ->once() ->andReturn(new Plugin('', ['name' => 'fake2'])); - $mock->shouldReceive('getUnsatisfied') - ->withArgs(function ($plugin) { - $this->assertEquals('fake2', $plugin->name); - - return true; - }) + $mock->shouldReceive('enable') + ->with('fake2') ->once() - ->andReturn(collect([ - 'dep' => ['version' => '0.0.0', 'constraint' => '^6.6.6'], - 'whatever' => ['version' => null, 'constraint' => '^1.2.3'], - ])); + ->andReturn([ + 'unsatisfied' => collect([ + 'dep' => ['version' => '0.0.0', 'constraint' => '^6.6.6'], + 'whatever' => ['version' => null, 'constraint' => '^1.2.3'], + ]), + ]); $mock->shouldReceive('get') ->with('fake3') @@ -107,15 +105,8 @@ class PluginControllerTest extends TestCase ->andReturn(new Plugin('', ['name' => 'fake3', 'title' => 'Fake'])); $mock->shouldReceive('enable') ->with('fake3') - ->once(); - $mock->shouldReceive('getUnsatisfied') - ->withArgs(function ($plugin) { - $this->assertEquals('fake3', $plugin->name); - - return true; - }) ->once() - ->andReturn(collect([])); + ->andReturn(true); $mock->shouldReceive('get') ->with('fake4') diff --git a/tests/ServicesTest/PluginManagerTest.php b/tests/ServicesTest/PluginManagerTest.php index 228a720a..3c2f0af1 100644 --- a/tests/ServicesTest/PluginManagerTest.php +++ b/tests/ServicesTest/PluginManagerTest.php @@ -401,10 +401,14 @@ class PluginManagerTest extends TestCase $reflection = new ReflectionClass($manager); $property = $reflection->getProperty('plugins'); $property->setAccessible(true); - $plugin = new Plugin('', ['name' => 'fake']); - $property->setValue($manager, collect(['fake' => $plugin])); + $property->setValue($manager, collect([ + 'fake' => new Plugin('', ['name' => 'fake']), + 'dep' => new Plugin('', ['name' => 'dep', 'require' => ['a' => '*']]), + ])); - $manager->enable('fake'); + $this->assertFalse($manager->enable('nope')); + + $this->assertTrue($manager->enable('fake')); Event::assertDispatched(Events\PluginWasEnabled::class, function ($event) { $this->assertEquals('fake', $event->plugin->name); @@ -415,6 +419,8 @@ class PluginManagerTest extends TestCase 'fake', json_decode(resolve(\App\Services\Option::class)->get('plugins_enabled'), true)[0]['name'] ); + + $this->assertTrue($manager->enable('dep')['unsatisfied']->isNotEmpty()); } public function testDisable()