core/drupalci.yml | 10 +- .../Annotation/DrupalAspectsOfCKEditor5Plugin.php | 9 + .../src/Plugin/CKEditor5PluginDefinition.php | 67 ++++- .../src/Plugin/CKEditor5PluginManager.php | 2 + .../src/Kernel/CKEditor5PluginManagerTest.php | 323 +++++++++++++++++++-- core/package.json | 2 +- 6 files changed, 383 insertions(+), 30 deletions(-) diff --git a/core/drupalci.yml b/core/drupalci.yml index 94aa2ad22f..6b4e9a767b 100644 --- a/core/drupalci.yml +++ b/core/drupalci.yml @@ -15,30 +15,30 @@ build: # deprecated code. run_tests.phpunit: types: 'PHPUnit-Unit' - testgroups: '--all' + testgroups: '--module ckeditor5' suppress-deprecations: false halt-on-fail: false run_tests.kernel: types: 'PHPUnit-Kernel' - testgroups: '--all' + testgroups: '--module ckeditor5' suppress-deprecations: false halt-on-fail: false run_tests.build: # Limit concurrency due to disk space concerns. concurrency: 15 types: 'PHPUnit-Build' - testgroups: '--all' + testgroups: '--module ckeditor5' suppress-deprecations: false halt-on-fail: false run_tests.functional: types: 'PHPUnit-Functional' - testgroups: '--all' + testgroups: '--module ckeditor5' suppress-deprecations: false halt-on-fail: false run_tests.javascript: concurrency: 15 types: 'PHPUnit-FunctionalJavascript' - testgroups: '--all' + testgroups: '--module ckeditor5' suppress-deprecations: false halt-on-fail: false # Run nightwatch testing. diff --git a/core/modules/ckeditor5/src/Annotation/DrupalAspectsOfCKEditor5Plugin.php b/core/modules/ckeditor5/src/Annotation/DrupalAspectsOfCKEditor5Plugin.php index a8feeb33a2..fff4a59aa8 100644 --- a/core/modules/ckeditor5/src/Annotation/DrupalAspectsOfCKEditor5Plugin.php +++ b/core/modules/ckeditor5/src/Annotation/DrupalAspectsOfCKEditor5Plugin.php @@ -42,6 +42,15 @@ class DrupalAspectsOfCKEditor5Plugin extends Plugin { */ public $class = CKEditor5PluginDefault::class; + /** + * The CKEditor 5 plugin deriver class. + * + * This property is optional and it does not need to be declared. + * + * @var string|null + */ + public $deriver = NULL; + /** * The library this plugin requires. * diff --git a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginDefinition.php b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginDefinition.php index 1f6e96e64e..b129696bb9 100644 --- a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginDefinition.php +++ b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginDefinition.php @@ -6,6 +6,7 @@ use Drupal\ckeditor5\HTMLRestrictions; use Drupal\Component\Assertion\Inspector; +use Drupal\Component\Plugin\Definition\DerivablePluginDefinitionInterface; use Drupal\Component\Plugin\Definition\PluginDefinition; use Drupal\Component\Plugin\Definition\PluginDefinitionInterface; use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException; @@ -16,7 +17,7 @@ /** * Provides an implementation of a CKEditor 5 plugin definition. */ -final class CKEditor5PluginDefinition extends PluginDefinition implements PluginDefinitionInterface { +final class CKEditor5PluginDefinition extends PluginDefinition implements PluginDefinitionInterface, DerivablePluginDefinitionInterface { use SchemaCheckTrait; @@ -34,6 +35,15 @@ final class CKEditor5PluginDefinition extends PluginDefinition implements Plugin */ private $drupal; + /** + * Discovered base plugin IDs. + * + * @var string[] + * + * @internal + */ + protected static $basePluginIds = []; + /** * CKEditor5PluginDefinition constructor. * @@ -51,11 +61,36 @@ public function __construct(array $definition) { } $this->provider = $definition['provider']; - static::validateCKEditor5Aspects($id, $definition); - $this->ckeditor5 = $definition['ckeditor5']; + try { + static::validateCKEditor5Aspects($id, $definition); + $this->ckeditor5 = $definition['ckeditor5']; + + $this->validateDrupalAspects($id, $definition); + $this->drupal = $definition['drupal']; + } + catch (InvalidPluginDefinitionException $e) { + // If this exception is thrown for a CKEditor 5 plugin definition whose ID + // matches a previously seen base plugin ID, it means the deriver did not + // generate a valid plugin definition. Re-throw the exception, but tweak + // the language for DX: clarify it is for a derived plugin definition. + if (!empty(static::$basePluginIds) && preg_match('/^(' . implode('|', static::$basePluginIds) . ').*/', $this->id)) { + throw new InvalidPluginDefinitionException($e->getPluginId(), str_replace('plugin definition', 'derived plugin definition', $e->getMessage())); + } + + // Allow an invalid plugin definition only if it's yet to be derived. + if (isset($definition['drupal']['deriver']) && class_exists($definition['drupal']['deriver'])) { + // Pass on all information from the base definition, without validation; + // it will be validated later. + $this->ckeditor5 = $definition['ckeditor5'] ?? []; + $this->drupal = $definition['drupal']; + // Track which IDs should be considered base plugin IDs. + static::$basePluginIds[] = $this->id; + return; + } - $this->validateDrupalAspects($id, $definition); - $this->drupal = $definition['drupal']; + // Otherwise, the exception was appropriate: re-throw it. + throw $e; + } } /** @@ -304,6 +339,28 @@ public function setClass($class) { return $this; } + /** + * {@inheritdoc} + * + * @see \Drupal\ckeditor5\Annotation\DrupalAspectsOfCKEditor5Plugin::$deriver + */ + public function getDeriver() { + // TRICKY: this is the only key that is allowed to not be set, because it is + // possible that this plugin definition is a partial/incomplete one, and the + // default from the annotation is only applied at the time that + // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::__construct() + // @see \Drupal\ckeditor5\Annotation\CKEditor5Plugin::__construct() + return $this->drupal['deriver'] ?? NULL; + } + + /** + * {@inheritdoc} + */ + public function setDeriver($deriver) { + $this->drupal['deriver'] = $deriver; + return $this; + } + /** * Whether this plugin is configurable by the user. * diff --git a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php index 068647a0bc..65b20f8adc 100644 --- a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php +++ b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php @@ -13,6 +13,7 @@ use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Plugin\DefaultPluginManager; use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery; +use Drupal\Core\Plugin\Discovery\ContainerDerivativeDiscoveryDecorator; use Drupal\Core\Plugin\Discovery\YamlDiscoveryDecorator; use Drupal\editor\EditorInterface; use Drupal\filter\FilterPluginCollection; @@ -61,6 +62,7 @@ protected function getDiscovery() { // supports top-level properties. // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::label() $discovery = new AnnotationBridgeDecorator($discovery, $this->pluginDefinitionAnnotationName); + $discovery = new ContainerDerivativeDiscoveryDecorator($discovery); $this->discovery = $discovery; } return $this->discovery; diff --git a/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php b/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php index aa5d266166..d223064ccc 100644 --- a/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php +++ b/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php @@ -3,8 +3,12 @@ namespace Drupal\Tests\ckeditor5\Kernel; use Composer\Autoload\ClassLoader; +use Drupal\ckeditor5\Annotation\CKEditor5AspectsOfCKEditor5Plugin; +use Drupal\ckeditor5\Annotation\DrupalAspectsOfCKEditor5Plugin; use Drupal\ckeditor5\HTMLRestrictions; use Drupal\ckeditor5\Plugin\CKEditor5Plugin\Heading; +use Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition; +use Drupal\Component\DependencyInjection\ContainerInterface; use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\editor\Entity\Editor; @@ -78,38 +82,44 @@ protected function setUp(): void { } /** - * @covers \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::__construct() - * @dataProvider providerTestInvalidPluginDefinitions + * Mocks a module providing a CKEditor 5 plugin in VFS. + * + * @param string $module_name + * The name of the module. + * @param string $yaml + * The YAML to be stored in the *.ckeditor5.yml file. + * @param array $additional_files + * The additional files to create. + * + * @return \Drupal\Component\DependencyInjection\ContainerInterface + * The container that has the VFS-mocked CKEditor 5 plugin-providing module + * installed in it; this container must be used to simulate this module + * being installed. */ - public function testInvalidPluginDefinitions(string $yaml, ?string $expected_message, array $additional_files = []): void { - if ($expected_message) { - $this->expectException(InvalidPluginDefinitionException::class); - $this->expectExceptionMessage($expected_message); - } - + private function mockModuleInVfs(string $module_name, string $yaml, array $additional_files = []): ContainerInterface { $site_directory = ltrim(parse_url($this->siteDirectory)['path'], '/'); vfsStream::create([ 'modules' => [ - 'ckeditor5_invalid_plugin' => [ - 'ckeditor5_invalid_plugin.info.yml' => << [ + "$module_name.info.yml" => << $yaml, + "$module_name.ckeditor5.yml" => $yaml, ] + $additional_files, ], ], $this->vfsRoot->getChild($site_directory)); if (!empty($additional_files)) { $additional_class_loader = new ClassLoader(); - $additional_class_loader->addPsr4("Drupal\\ckeditor5_invalid_plugin\\Plugin\\CKEditor5Plugin\\", vfsStream::url("root/$site_directory/modules/ckeditor5_invalid_plugin/src/Plugin/CKEditor5Plugin")); + $additional_class_loader->addPsr4("Drupal\\$module_name\\Plugin\\CKEditor5Plugin\\", vfsStream::url("root/$site_directory/modules/$module_name/src/Plugin/CKEditor5Plugin")); $additional_class_loader->register(TRUE); } $config_sync = \Drupal::service('config.storage'); $config_data = $this->config('core.extension')->get(); - $config_data['module']['ckeditor5_invalid_plugin'] = 1; + $config_data['module'][$module_name] = 1; $config_sync->write('core.extension', $config_data); // Construct a new container for testing a plugin definition in isolation, @@ -123,9 +133,9 @@ public function testInvalidPluginDefinitions(string $yaml, ?string $expected_mes $container = new ContainerBuilder(new FrozenParameterBag([ 'app.root' => $root, 'container.modules' => [ - 'ckeditor5_invalid_plugin' => [ + $module_name => [ 'type' => 'module', - 'pathname' => 'modules/ckeditor5_invalid_plugin/ckeditor5_invalid_plugin.info.yml', + 'pathname' => "modules/$module_name/$module_name.info.yml", 'filename' => NULL, ] + $this->container->getParameter('container.modules'), ], @@ -146,14 +156,28 @@ public function testInvalidPluginDefinitions(string $yaml, ?string $expected_mes // only work-around possible is to manipulate the config schema definition // cache. // @todo Remove this in https://www.drupal.org/project/drupal/issues/2961541. - if (isset($additional_files['config']['schema']['ckeditor5_invalid_plugin.schema.yml'])) { - $cache = \Drupal::service('cache.discovery')->get('typed_config_definitions'); + if (isset($additional_files['config']['schema']["$module_name.schema.yml"])) { + $cache = \Drupal::service('cache.discovery') + ->get('typed_config_definitions'); $typed_config_definitions = $cache->data; - $typed_config_definitions += Yaml::parse($additional_files['config']['schema']['ckeditor5_invalid_plugin.schema.yml']); + $typed_config_definitions += Yaml::parse($additional_files['config']['schema']["$module_name.schema.yml"]); \Drupal::service('config.typed')->clearCachedDefinitions(); \Drupal::service('cache.discovery')->set('typed_config_definitions', $typed_config_definitions, $cache->expire, $cache->tags); } + return $container; + } + + /** + * @covers \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::__construct() + * @dataProvider providerTestInvalidPluginDefinitions + */ + public function testInvalidPluginDefinitions(string $yaml, ?string $expected_message, array $additional_files = []): void { + if ($expected_message) { + $this->expectException(InvalidPluginDefinitionException::class); + $this->expectExceptionMessage($expected_message); + } + $container = $this->mockModuleInVfs('ckeditor5_invalid_plugin', $yaml, $additional_files); $container->get('plugin.manager.ckeditor5.plugin')->getDefinitions(); } @@ -1519,4 +1543,265 @@ public function testExternalLinkAutomaticLinkDecoratorDisallowed(): void { $this->manager->getDefinitions(); } + /** + * @covers ::getDiscovery + * @dataProvider providerTestDerivedPluginDefinitions + */ + public function testDerivedPluginDefinitions(string $yaml, ?string $expected_message, array $additional_files = [], ?array $expected_derived_plugin_definitions = NULL): void { + if ($expected_message) { + $this->expectException(InvalidPluginDefinitionException::class); + $this->expectExceptionMessage($expected_message); + } + $container = $this->mockModuleInVfs('ckeditor5_derived_plugin', $yaml, $additional_files); + + $actual_definitions = $container->get('plugin.manager.ckeditor5.plugin')->getDefinitions(); + $this->assertEquals($expected_derived_plugin_definitions, $actual_definitions); + } + + /** + * Data provider. + * + * @return \Generator + * Test scenarios. + */ + public function providerTestDerivedPluginDefinitions(): \Generator { + // Defaults inherited from CKEditor5AspectsOfCKEditor5Plugin. + $ckeditor5_aspects_defaults = get_class_vars(CKEditor5AspectsOfCKEditor5Plugin::class); + // Defaults inherited from DrupalAspectsOfCKEditor5Plugin. + $drupal_aspects_defaults = get_class_vars(DrupalAspectsOfCKEditor5Plugin::class); + + $simple_deriver_additional_files = [ + 'src' => [ + 'Plugin' => [ + 'CKEditor5Plugin' => [ + 'SimpleDeriver.php' => <<<'PHP' +toArray(); + $definition['id'] .= "_$derivative"; + $definition['drupal']['label'] = sprintf("Foo %s", $derivative); + $this->derivatives[$definition['id']] = new CKEditor5PluginDefinition($definition); + } + return $this->derivatives; + } +} +PHP, + ], + ], + ], + ]; + + yield 'INVALID: simple deriver but without `drupal.elements` in the base definition and it not getting set by the deriver' => [ + << [ + << [ + << [ + << new CKEditor5PluginDefinition([ + 'provider' => 'ckeditor5_derived_plugin', + 'id' => 'ckeditor5_derived_plugin_foo_bar', + 'ckeditor5' => ['plugins' => []] + $ckeditor5_aspects_defaults, + 'drupal' => [ + 'label' => 'Foo bar', + 'elements' => FALSE, + 'deriver' => 'Drupal\ckeditor5_derived_plugin\Plugin\CKEditor5Plugin\SimpleDeriver', + ] + $drupal_aspects_defaults, + ]), + 'ckeditor5_derived_plugin_foo:ckeditor5_derived_plugin_foo_baz' => new CKEditor5PluginDefinition([ + 'provider' => 'ckeditor5_derived_plugin', + 'id' => 'ckeditor5_derived_plugin_foo_baz', + 'ckeditor5' => ['plugins' => []] + $ckeditor5_aspects_defaults, + 'drupal' => [ + 'label' => 'Foo baz', + 'elements' => FALSE, + 'deriver' => 'Drupal\ckeditor5_derived_plugin\Plugin\CKEditor5Plugin\SimpleDeriver', + ] + $drupal_aspects_defaults, + ]), + ], + ]; + + yield 'VALID: minimal base plugin definition, maximal deriver' => [ + << [ + 'Plugin' => [ + 'CKEditor5Plugin' => [ + 'MaximalDeriver.php' => <<<'PHP' +toArray(); + $definition['id'] .= "_$derivative"; + $definition['drupal']['label'] = sprintf("Foo %s", $derivative); + $definition['drupal']['elements'] = FALSE; + $definition['ckeditor5']['plugins'] = []; + $this->derivatives[$definition['id']] = new CKEditor5PluginDefinition($definition); + } + return $this->derivatives; + } +} +PHP, + ], + ], + ], + ], + [ + 'ckeditor5_derived_plugin_foo:ckeditor5_derived_plugin_foo_A' => new CKEditor5PluginDefinition([ + 'provider' => 'ckeditor5_derived_plugin', + 'id' => 'ckeditor5_derived_plugin_foo_A', + 'ckeditor5' => ['plugins' => []], + 'drupal' => [ + 'label' => 'Foo A', + 'elements' => FALSE, + 'deriver' => 'Drupal\ckeditor5_derived_plugin\Plugin\CKEditor5Plugin\MaximalDeriver', + ] + $drupal_aspects_defaults, + ]), + 'ckeditor5_derived_plugin_foo:ckeditor5_derived_plugin_foo_B' => new CKEditor5PluginDefinition([ + 'provider' => 'ckeditor5_derived_plugin', + 'id' => 'ckeditor5_derived_plugin_foo_B', + 'ckeditor5' => ['plugins' => []], + 'drupal' => [ + 'label' => 'Foo B', + 'elements' => FALSE, + 'deriver' => 'Drupal\ckeditor5_derived_plugin\Plugin\CKEditor5Plugin\MaximalDeriver', + ] + $drupal_aspects_defaults, + ]), + ], + ]; + + yield 'VALID: container-dependent deriver' => [ + << [ + 'schema' => [ + 'ckeditor5_derived_plugin.schema.yml' => << [ + 'Plugin' => [ + 'CKEditor5Plugin' => [ + 'ContainerDependentDeriver.php' => <<<'PHP' +authenticationCollector = $authentication_collector; + } + public static function create(ContainerInterface $container, $base_plugin_id) { + assert($base_plugin_id === 'ckeditor5_derived_plugin_foo'); + return new static($container->get('authentication_collector')); + } + public function getDerivativeDefinitions($base_plugin_definition) { + assert($base_plugin_definition instanceof CKEditor5PluginDefinition); + $authentication_providers = array_keys($this->authenticationCollector->getSortedProviders()); + foreach ($authentication_providers as $id) { + $definition = $base_plugin_definition->toArray(); + $definition['id'] .= "_$id"; + $definition['drupal']['label'] = sprintf("Foo %s", $id); + $this->derivatives[$definition['id']] = new CKEditor5PluginDefinition($definition); + } + return $this->derivatives; + } +} +PHP, + ], + ], + ], + ], + [ + 'ckeditor5_derived_plugin_foo:ckeditor5_derived_plugin_foo_cookie' => new CKEditor5PluginDefinition([ + 'provider' => 'ckeditor5_derived_plugin', + 'id' => 'ckeditor5_derived_plugin_foo_cookie', + 'ckeditor5' => ['plugins' => []] + $ckeditor5_aspects_defaults, + 'drupal' => [ + 'label' => 'Foo cookie', + 'elements' => FALSE, + 'deriver' => 'Drupal\ckeditor5_derived_plugin\Plugin\CKEditor5Plugin\ContainerDependentDeriver', + ] + $drupal_aspects_defaults, + ]), + ], + ]; + } + } diff --git a/core/package.json b/core/package.json index c14f58a08c..cc55f438eb 100644 --- a/core/package.json +++ b/core/package.json @@ -18,7 +18,7 @@ "lint:css": "stylelint \"**/*.css\"", "lint:css-checkstyle": "stylelint \"**/*.css\" --custom-formatter ./node_modules/stylelint-checkstyle-formatter/index.js", "lint:yaml": "node ./node_modules/eslint/bin/eslint.js --ext .yml .", - "test:nightwatch": "node -r dotenv-safe/config ./node_modules/.bin/nightwatch --config ./tests/Drupal/Nightwatch/nightwatch.conf.js", + "test:nightwatch": "node -r dotenv-safe/config ./node_modules/.bin/nightwatch --config ./tests/Drupal/Nightwatch/nightwatch.conf.js --tag ckeditor5", "prettier": "prettier --write \"./**/*.js\"", "spellcheck": "cspell", "spellcheck:make-drupal-dict": "rm -f misc/cspell/dictionary.txt && touch misc/cspell/dictionary.txt && yarn -s spellcheck:core --unique --wordsOnly | tr '[:upper:]' '[:lower:]' | tr -d \\\\\\\\ | LC_ALL=C sort -u -o misc/cspell/dictionary.txt",