diff --git a/app/Http/Controllers/PluginController.php b/app/Http/Controllers/PluginController.php index 481dbc74..42e235a8 100644 --- a/app/Http/Controllers/PluginController.php +++ b/app/Http/Controllers/PluginController.php @@ -8,10 +8,9 @@ use App\Services\PluginManager; class PluginController extends Controller { - public function config($name) + public function config(PluginManager $plugins, $name) { - $plugin = plugin($name); - + $plugin = $plugins->get($name); if ($plugin && $plugin->isEnabled() && $plugin->hasConfigView()) { return $plugin->getConfigView(); } else { @@ -21,7 +20,8 @@ class PluginController extends Controller public function manage(Request $request, PluginManager $plugins) { - $plugin = plugin($name = $request->get('name')); + $name = $request->input('name'); + $plugin = $plugins->get($name); if ($plugin) { // Pass the plugin title through the translator. @@ -29,18 +29,16 @@ class PluginController extends Controller switch ($request->get('action')) { case 'enable': - if (! $plugins->isRequirementsSatisfied($plugin)) { - $reason = []; - - foreach ($plugins->getUnsatisfiedRequirements($plugin) as $name => $detail) { + $requirements = $plugins->getUnsatisfied($plugin); + if ($requirements->isNotEmpty()) { + $reason = $requirements->map(function ($detail, $name) { $constraint = $detail['constraint']; - if (! $detail['version']) { - $reason[] = trans('admin.plugins.operations.unsatisfied.disabled', compact('name')); + return trans('admin.plugins.operations.unsatisfied.disabled', compact('name')); } else { - $reason[] = trans('admin.plugins.operations.unsatisfied.version', compact('name', 'constraint')); + return trans('admin.plugins.operations.unsatisfied.version', compact('name', 'constraint')); } - } + })->values()->all(); return json(trans('admin.plugins.operations.unsatisfied.notice'), 1, compact('reason')); } @@ -55,7 +53,7 @@ class PluginController extends Controller return json(trans('admin.plugins.operations.disabled', ['plugin' => $plugin->title]), 0); case 'delete': - $plugins->uninstall($name); + $plugins->delete($name); return json(trans('admin.plugins.operations.deleted'), 0); @@ -69,8 +67,8 @@ class PluginController extends Controller public function getPluginData(PluginManager $plugins) { - return $plugins->getPlugins() - ->map(function ($plugin) { + return $plugins->all() + ->map(function ($plugin) use ($plugins) { return [ 'name' => $plugin->name, 'title' => trans($plugin->title ?: 'EMPTY'), @@ -80,20 +78,12 @@ class PluginController extends Controller 'url' => $plugin->url, 'enabled' => $plugin->isEnabled(), 'config' => $plugin->hasConfigView(), - 'dependencies' => $this->getPluginDependencies($plugin), + 'dependencies' => [ + 'all' => $plugin->require, + 'unsatisfied' => $plugins->getUnsatisfied($plugin), + ], ]; }) ->values(); } - - protected function getPluginDependencies(Plugin $plugin) - { - $plugins = app('plugins'); - - return [ - 'isRequirementsSatisfied' => $plugins->isRequirementsSatisfied($plugin), - 'requirements' => $plugin->getRequirements(), - 'unsatisfiedRequirements' => $plugins->getUnsatisfiedRequirements($plugin), - ]; - } } diff --git a/app/Providers/PluginServiceProvider.php b/app/Providers/PluginServiceProvider.php index 36226cd5..ff170c4e 100644 --- a/app/Providers/PluginServiceProvider.php +++ b/app/Providers/PluginServiceProvider.php @@ -11,109 +11,13 @@ use Illuminate\Support\ServiceProvider; class PluginServiceProvider extends ServiceProvider { - /** - * Register any application services. - * - * @return void - */ public function register() { $this->app->singleton('plugins', PluginManager::class); } - /** - * Bootstrap any application services. - * - * @return void - */ public function boot(PluginManager $plugins) { - // Disable plugins which has unsatisfied dependencies - $this->disableRequirementsUnsatisfiedPlugins($plugins); - - // Store paths of class files of plugins - $src_paths = []; - - $loader = $this->app->make('translation.loader'); - // Make view instead of view.finder since the finder is defined as not a singleton - $finder = $this->app->make('view'); - - foreach ($plugins->getPlugins() as $plugin) { - if ($plugin->isEnabled()) { - $src_paths[$plugin->getNamespace()] = $plugin->getPath().'/src'; - // Add paths of views - $finder->addNamespace($plugin->getNamespace(), $plugin->getPath().'/views'); - } - - // Always add paths of translation files for namespace hints - $loader->addNamespace($plugin->getNamespace(), $plugin->getPath().'/lang'); - } - - $this->registerPluginCallbackListener(); - $this->registerClassAutoloader($src_paths); - - // Register plugin's own composer autoloader - foreach ($plugins->getEnabledComposerAutoloaders() as $autoloader) { - require $autoloader; - } - - $bootstrappers = $plugins->getEnabledBootstrappers(); - - foreach ($bootstrappers as $file) { - $bootstrapper = require $file; - // Call closure using service container - $this->app->call($bootstrapper); - } - } - - protected function disableRequirementsUnsatisfiedPlugins(PluginManager $manager) - { - foreach ($manager->getEnabledPlugins() as $plugin) { - if (! $manager->isRequirementsSatisfied($plugin)) { - $manager->disable($plugin->name); - } - } - } - - protected function registerPluginCallbackListener() - { - Event::listen([ - Events\PluginWasEnabled::class, - Events\PluginWasDeleted::class, - Events\PluginWasDisabled::class, - ], function ($event) { - // Call callback functions of plugin - if (file_exists($filename = $event->plugin->getPath().'/callbacks.php')) { - $callbacks = require $filename; - - $callback = Arr::get($callbacks, get_class($event)); - - return $callback ? app()->call($callback, [$event->plugin]) : null; - } - }); - } - - /** - * Register class autoloader for plugins. - * - * @return void - */ - protected function registerClassAutoloader($paths) - { - spl_autoload_register(function ($class) use ($paths) { - // Traverse in registered plugin paths - foreach ((array) array_keys($paths) as $namespace) { - if ($namespace != '' && mb_strpos($class, $namespace) === 0) { - // Parse real file path - $path = $paths[$namespace].Str::replaceFirst($namespace, '', $class).'.php'; - $path = str_replace('\\', '/', $path); - - if (file_exists($path)) { - // Include class file if it exists - include $path; - } - } - } - }); + $plugins->boot(); } } diff --git a/app/Services/PluginManager.php b/app/Services/PluginManager.php index e01f3661..5a7df0ec 100644 --- a/app/Services/PluginManager.php +++ b/app/Services/PluginManager.php @@ -66,15 +66,20 @@ class PluginManager } /** - * Boot all enabled plugins. + * Get all installed plugins. + * + * @return Collection */ - public function boot() + public function all() { - if ($this->booted) { - return; + if (filled($this->plugins)) { + return $this->plugins; } - $this->enabled = collect(json_decode($this->option->get('plugins_enabled', '[]'), true)); + $this->enabled = collect(json_decode($this->option->get('plugins_enabled', '[]'), true)) + ->mapWithKeys(function ($item) { + return [$item['name'] => ['version' => $item['version']]]; + }); $plugins = collect(); collect($this->filesystem->directories($this->getPluginsDir())) @@ -100,18 +105,31 @@ class PluginManager if ($this->getUnsatisfied($plugin)->isNotEmpty()) { $this->disable($plugin); } - if ($this->enabled->contains('name', $name)) { + if ($this->enabled->has($name)) { $plugin->setEnabled(true); if (Comparator::notEqualTo( $manifest['version'], - $this->enabled->firstWhere('name', $name)['version'] + $this->enabled->get($name)['version'] )) { $this->dispatcher->dispatch(new Events\PluginVersionChanged($plugin)); } } }); - $enabled = $plugins->filter(function ($plugin) { + $this->plugins = $plugins; + return $plugins; + } + + /** + * Boot all enabled plugins. + */ + public function boot() + { + if ($this->booted) { + return; + } + + $enabled = $this->all()->filter(function ($plugin) { return $plugin->isEnabled(); }); @@ -217,6 +235,14 @@ class PluginManager }); } + /** + * @return Plugin|null + */ + public function get(string $name) + { + return $this->all()->get($name); + } + /** * @return Collection */ @@ -313,23 +339,13 @@ class PluginManager */ public function enable($name) { - if (is_null($this->enabled)) { - $this->convertPluginRecord(); - } - - if (! $this->isEnabled($name)) { - $plugin = $this->getPlugin($name); - - $this->enabled->push([ - 'name' => $name, - 'version' => $plugin->getVersion(), - ]); + $plugin = $this->get($name); + if (! $plugin->isEnabled($name)) { + $this->enabled->put($name, ['version' => $plugin->version]); $this->saveEnabled(); $plugin->setEnabled(true); - $this->copyPluginAssets($plugin); - $this->dispatcher->dispatch(new Events\PluginWasEnabled($plugin)); } } @@ -365,7 +381,7 @@ class PluginManager * * @param string $name */ - public function uninstall($name) + public function delete($name) { $plugin = $this->getPlugin($name); @@ -387,10 +403,6 @@ class PluginManager */ public function getEnabledPlugins() { - if (is_null($this->enabled)) { - $this->convertPluginRecord(); - } - return $this->getPlugins()->only($this->getEnabled()); } @@ -472,7 +484,9 @@ class PluginManager */ protected function saveEnabled() { - $this->option->set('plugins_enabled', $this->enabled->values()->toJson()); + $this->option->set('plugins_enabled', $this->enabled->map(function ($info, $name) { + // + })->toJson()); } /** @@ -504,10 +518,10 @@ class PluginManager return (! Semver::satisfies($version, $constraint)) ? [$name => compact('version', 'constraint')] : []; - } elseif (! $this->enabled->contains('name', $name)) { + } elseif (! $this->enabled->has($name)) { return [$name => ['version' => null, 'constraint' => $constraint]]; } else { - $version = $this->enabled->firstWhere('name', $name)['version']; + $version = $this->enabled->get($name)['version']; return (! Semver::satisfies($version, $constraint)) ? [$name => compact('version', 'constraint')] : []; @@ -618,40 +632,4 @@ class PluginManager $dir.'/lang' ); } - - /** - * Convert `plugins_enabled` field for backward compatibility. - * - * @return $this - */ - protected function convertPluginRecord() - { - $list = collect(json_decode($this->option->get('plugins_enabled'), true)); - $this->enabled = $list->map(function ($item) { - if (is_string($item)) { - $plugin = $this->getPlugin($item); - - // If we cannot read the package of that plugin, just return it as-is. - if (is_null($plugin)) { - return $item; - } - - return [ - 'name' => $item, - 'version' => $plugin->getVersion(), - ]; - } else { - $plugin = $this->getPlugin($item['name']); - if (! empty($plugin)) { - $item['version'] = $plugin->getVersion(); - } - - return $item; - } - }); - - $this->saveEnabled(); - - return $this; - } } diff --git a/app/helpers.php b/app/helpers.php index 38329fc4..a53f972b 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -25,9 +25,9 @@ if (! function_exists('webpack_assets')) { } if (! function_exists('plugin')) { - function plugin(string $id) + function plugin(string $name) { - return app('plugins')->getPlugin($id); + return app('plugins')->get($name); } } diff --git a/resources/assets/src/components/mixins/enablePlugin.ts b/resources/assets/src/components/mixins/enablePlugin.ts index 66bcc803..37f336ad 100644 --- a/resources/assets/src/components/mixins/enablePlugin.ts +++ b/resources/assets/src/components/mixins/enablePlugin.ts @@ -4,13 +4,13 @@ export default Vue.extend({ data: () => ({ plugins: [] }), methods: { async enablePlugin({ - name, dependencies: { requirements }, originalIndex, + name, dependencies: { all }, originalIndex, }: { name: string, - dependencies: { requirements: string[] }, + dependencies: { all: { [name: string]: string } }, originalIndex: number }) { - if (requirements.length === 0) { + if (Object.keys(all).length === 0) { try { await this.$confirm( this.$t('admin.noDependenciesNotice'), diff --git a/resources/assets/src/views/admin/Plugins.vue b/resources/assets/src/views/admin/Plugins.vue index 577e92d5..8acfc9f1 100644 --- a/resources/assets/src/views/admin/Plugins.vue +++ b/resources/assets/src/views/admin/Plugins.vue @@ -54,16 +54,16 @@
- {{ dep }}: {{ semver }} + {{ name }}: {{ constraint }}
diff --git a/resources/assets/tests/views/admin/Plugins.test.ts b/resources/assets/tests/views/admin/Plugins.test.ts index 22ae6880..8f29b86c 100644 --- a/resources/assets/tests/views/admin/Plugins.test.ts +++ b/resources/assets/tests/views/admin/Plugins.test.ts @@ -7,11 +7,11 @@ jest.mock('@/scripts/notify') test('render dependencies', async () => { Vue.prototype.$http.get.mockResolvedValue([ - { name: 'a', dependencies: { requirements: [] } }, + { name: 'a', dependencies: { all: {}, unsatisfied: {} } }, { name: 'b', dependencies: { - requirements: { a: '^1.0.0', c: '^2.0.0' }, unsatisfiedRequirements: { c: {} }, + all: { a: '^1.0.0', c: '^2.0.0' }, unsatisfied: { c: '' }, }, }, ]) @@ -26,13 +26,13 @@ test('render dependencies', async () => { test('render operation buttons', async () => { Vue.prototype.$http.get.mockResolvedValue([ { - name: 'a', dependencies: { requirements: [] }, enabled: true, config: true, + name: 'a', dependencies: { all: {}, unsatisfied: {} }, enabled: true, config: true, }, { - name: 'b', dependencies: { requirements: [] }, enabled: true, config: false, + name: 'b', dependencies: { all: {}, unsatisfied: {} }, enabled: true, config: false, }, { - name: 'c', dependencies: { requirements: [] }, enabled: false, + name: 'c', dependencies: { all: {}, unsatisfied: {} }, enabled: false, }, ]) const wrapper = mount(Plugins) @@ -49,10 +49,10 @@ test('render operation buttons', async () => { test('enable plugin', async () => { Vue.prototype.$http.get.mockResolvedValue([ { - name: 'a', dependencies: { requirements: [] }, enabled: false, + name: 'a', dependencies: { all: {}, unsatisfied: {} }, enabled: false, }, { - name: 'b', dependencies: { requirements: {} }, enabled: false, + name: 'b', dependencies: { all: {}, unsatisfied: {} }, enabled: false, }, ]) Vue.prototype.$http.post @@ -101,7 +101,7 @@ test('enable plugin', async () => { test('disable plugin', async () => { Vue.prototype.$http.get.mockResolvedValue([ { - name: 'a', dependencies: { requirements: [] }, enabled: true, config: false, + name: 'a', dependencies: { all: {}, unsatisfied: {} }, enabled: true, config: false, }, ]) Vue.prototype.$http.post @@ -126,7 +126,7 @@ test('disable plugin', async () => { test('delete plugin', async () => { Vue.prototype.$http.get.mockResolvedValue([ { - name: 'a', dependencies: { requirements: [] }, enabled: false, + name: 'a', dependencies: { all: {}, unsatisfied: {} }, enabled: false, }, ]) Vue.prototype.$http.post diff --git a/tests/PluginControllerTest.php b/tests/PluginControllerTest.php index dd4a7de5..f4057ddd 100644 --- a/tests/PluginControllerTest.php +++ b/tests/PluginControllerTest.php @@ -2,6 +2,8 @@ namespace Tests; +use App\Services\Plugin; +use App\Services\PluginManager; use Illuminate\Support\Facades\File; use Tests\Concerns\GeneratesFakePlugins; use Illuminate\Foundation\Testing\DatabaseTransactions; @@ -14,10 +16,6 @@ class PluginControllerTest extends TestCase protected function setUp(): void { parent::setUp(); - - $this->generateFakePlugin(['name' => 'fake-plugin-for-test', 'version' => '1.1.4']); - $this->generateFakePlugin(['name' => 'fake-plugin-with-config-view', 'version' => '5.1.4', 'config' => 'config.blade.php']); - $this->actAs('superAdmin'); } @@ -29,24 +27,114 @@ class PluginControllerTest extends TestCase public function testConfig() { + option(['plugins_enabled' => json_encode([ + ['name' => 'fake3', 'version' => '0.0.0'], + ['name' => 'fake4', 'version' => '0.0.0'], + ])]); + $this->mock(PluginManager::class, function ($mock) { + $mock->shouldReceive('get') + ->with('fake1') + ->once() + ->andReturn(null); + + $mock->shouldReceive('get') + ->with('fake2') + ->once() + ->andReturn(new Plugin('', [])); + + $mock->shouldReceive('get') + ->with('fake3') + ->once() + ->andReturn(new Plugin('', [])); + + $plugin = new Plugin(resource_path(''), ['config' => 'common/favicon.blade.php']); + $plugin->setEnabled(true); + $mock->shouldReceive('get') + ->with('fake4') + ->once() + ->andReturn($plugin); + }); + + // No such plugin. + $this->get('/admin/plugins/config/fake1') + ->assertNotFound(); + // Plugin is disabled - $this->get('/admin/plugins/config/fake-plugin-with-config-view') + $this->get('/admin/plugins/config/fake2') ->assertNotFound(); // Plugin is enabled but it doesn't have config view - plugin('fake-plugin-for-test')->setEnabled(true); - $this->get('/admin/plugins/config/avatar-api') + $this->get('/admin/plugins/config/fake3') ->assertSee(trans('admin.plugins.operations.no-config-notice')) ->assertNotFound(); // Plugin has config view - plugin('fake-plugin-with-config-view')->setEnabled(true); - $this->get('/admin/plugins/config/fake-plugin-with-config-view') + $this->get('/admin/plugins/config/fake4') ->assertSuccessful(); + + option(['plugins_enabled' => '[]']); } public function testManage() { + $this->mock(PluginManager::class, function ($mock) { + $mock->shouldReceive('get') + ->with('nope') + ->once() + ->andReturn(null); + + $mock->shouldReceive('get') + ->with('fake1') + ->once() + ->andReturn(new Plugin('', [])); + + $mock->shouldReceive('get') + ->with('fake2') + ->once() + ->andReturn(new Plugin('', ['name' => 'fake2'])); + $mock->shouldReceive('getUnsatisfied') + ->withArgs(function ($plugin) { + $this->assertEquals('fake2', $plugin->name); + return true; + }) + ->once() + ->andReturn(collect([ + 'dep' => ['version' => '0.0.0', 'constraint' => '^6.6.6'], + 'whatever' => ['version' => null, 'constraint' => '^1.2.3'], + ])); + + $mock->shouldReceive('get') + ->with('fake3') + ->once() + ->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([])); + + $mock->shouldReceive('get') + ->with('fake4') + ->once() + ->andReturn(new Plugin('', ['name' => 'fake4', 'title' => 'Fake'])); + $mock->shouldReceive('disable') + ->with('fake4') + ->once(); + + $mock->shouldReceive('get') + ->with('fake5') + ->once() + ->andReturn(new Plugin('', ['name' => 'fake5', 'title' => 'Fake'])); + $mock->shouldReceive('delete') + ->with('fake5') + ->once(); + }); + // An not-existed plugin $this->postJson('/admin/plugins/manage', ['name' => 'nope']) ->assertJson([ @@ -55,21 +143,15 @@ class PluginControllerTest extends TestCase ]); // Invalid action - $this->postJson('/admin/plugins/manage', ['name' => 'fake-plugin-for-test']) + $this->postJson('/admin/plugins/manage', ['name' => 'fake1']) ->assertJson([ 'code' => 1, 'message' => trans('admin.invalid-action'), ]); // Enable a plugin with unsatisfied dependencies - app('plugins')->getPlugin('fake-plugin-for-test')->setRequirements([ - 'blessing-skin-server' => '^3.4.0 || ^4.0.0', - 'fake-plugin-with-config-view' => '^6.6.6', - 'whatever' => '^1.0.0', - ]); - app('plugins')->enable('fake-plugin-with-config-view'); $this->postJson('/admin/plugins/manage', [ - 'name' => 'fake-plugin-for-test', + 'name' => 'fake2', 'action' => 'enable', ])->assertJson([ 'code' => 1, @@ -77,7 +159,7 @@ class PluginControllerTest extends TestCase 'data' => [ 'reason' => [ trans('admin.plugins.operations.unsatisfied.version', [ - 'name' => 'fake-plugin-with-config-view', + 'name' => 'dep', 'constraint' => '^6.6.6', ]), trans('admin.plugins.operations.unsatisfied.disabled', [ @@ -88,43 +170,57 @@ class PluginControllerTest extends TestCase ]); // Enable a plugin - app('plugins')->getPlugin('fake-plugin-for-test')->setRequirements([]); $this->postJson('/admin/plugins/manage', [ - 'name' => 'fake-plugin-for-test', + 'name' => 'fake3', 'action' => 'enable', ])->assertJson([ 'code' => 0, 'message' => trans( 'admin.plugins.operations.enabled', - ['plugin' => plugin('fake-plugin-for-test')->title] + ['plugin' => 'Fake'] ), ]); // Disable a plugin $this->postJson('/admin/plugins/manage', [ - 'name' => 'fake-plugin-for-test', + 'name' => 'fake4', 'action' => 'disable', ])->assertJson([ 'code' => 0, 'message' => trans( 'admin.plugins.operations.disabled', - ['plugin' => plugin('fake-plugin-for-test')->title] + ['plugin' => 'Fake'] ), ]); // Delete a plugin $this->postJson('/admin/plugins/manage', [ - 'name' => 'fake-plugin-for-test', + 'name' => 'fake5', 'action' => 'delete', ])->assertJson([ 'code' => 0, 'message' => trans('admin.plugins.operations.deleted'), ]); - $this->assertFalse(file_exists(base_path('plugins/fake-plugin-for-test/'))); } public function testGetPluginData() { + $this->mock(PluginManager::class, function ($mock) { + $mock->shouldReceive('all') + ->once() + ->andReturn(collect([new Plugin('', [ + 'name' => 'a', + 'version' => '0.0.0', + 'title' => '' + ])])); + $mock->shouldReceive('getUnsatisfied') + ->withArgs(function ($plugin) { + $this->assertEquals('a', $plugin->name); + return true; + }) + ->once() + ->andReturn(collect(['b' => null])); + }); $this->getJson('/admin/plugins/data') ->assertJsonStructure([ [ @@ -140,13 +236,4 @@ class PluginControllerTest extends TestCase ], ]); } - - protected function tearDown(): void - { - // Clean fake plugins - File::deleteDirectory(config('plugins.directory').DIRECTORY_SEPARATOR.'fake-plugin-for-test'); - File::deleteDirectory(config('plugins.directory').DIRECTORY_SEPARATOR.'fake-plugin-with-config-view'); - - parent::tearDown(); - } } diff --git a/tests/ServicesTest/PluginManagerTest.php b/tests/ServicesTest/PluginManagerTest.php index d1526c03..c79c6da0 100644 --- a/tests/ServicesTest/PluginManagerTest.php +++ b/tests/ServicesTest/PluginManagerTest.php @@ -23,7 +23,6 @@ class PluginManagerTest extends TestCase public function testPreventBootingAgain() { - // TODO: modify asserting 0 times here $this->mock(Filesystem::class, function ($mock) { $mock->shouldReceive('directories')->times(1); }); @@ -367,7 +366,7 @@ class PluginManagerTest extends TestCase $reflection = new ReflectionClass($manager); $property = $reflection->getProperty('enabled'); $property->setAccessible(true); - $property->setValue($manager, collect([['name' => 'another-plugin', 'version' => '1.2.3']])); + $property->setValue($manager, collect(['another-plugin' => ['version' => '1.2.3']])); $info = $manager->getUnsatisfied($plugin)->get('another-plugin'); $this->assertEquals('1.2.3', $info['version']); $this->assertEquals('0.0.*', $info['constraint']);