From 43ce451db27c7efc1d3187c59463a6a32aa47c91 Mon Sep 17 00:00:00 2001 From: Dave Long <dave@longwaveconsulting.com> Date: Wed, 15 May 2024 16:45:49 +0100 Subject: [PATCH] Issue #3447210 by alexpott, jnicola: Move RecipeDiscovery into RecipeConfigurator (cherry picked from commit 2e3e874662237ad38bd2006b3c8126a458d7cb20) --- core/lib/Drupal/Core/Recipe/Recipe.php | 37 ++++-------- .../Drupal/Core/Recipe/RecipeConfigurator.php | 48 +++++++++++++-- .../Drupal/Core/Recipe/RecipeDiscovery.php | 58 ------------------- .../Core/Recipe/RecipeConfiguratorTest.php | 40 +++++++++++-- .../Core/Recipe/RecipeDiscoveryTest.php | 48 --------------- 5 files changed, 92 insertions(+), 139 deletions(-) delete mode 100644 core/lib/Drupal/Core/Recipe/RecipeDiscovery.php delete mode 100644 core/tests/Drupal/KernelTests/Core/Recipe/RecipeDiscoveryTest.php diff --git a/core/lib/Drupal/Core/Recipe/Recipe.php b/core/lib/Drupal/Core/Recipe/Recipe.php index 4da07d2743e5..0312de933701 100644 --- a/core/lib/Drupal/Core/Recipe/Recipe.php +++ b/core/lib/Drupal/Core/Recipe/Recipe.php @@ -57,8 +57,7 @@ public function __construct( public static function createFromDirectory(string $path): static { $recipe_data = self::parse($path . '/recipe.yml'); - $recipe_discovery = static::getRecipeDiscovery(dirname($path)); - $recipes = new RecipeConfigurator(is_array($recipe_data['recipes']) ? $recipe_data['recipes'] : [], $recipe_discovery); + $recipes = new RecipeConfigurator(is_array($recipe_data['recipes']) ? $recipe_data['recipes'] : [], dirname($path)); $install = new InstallConfigurator($recipe_data['install'], \Drupal::service('extension.list.module'), \Drupal::service('extension.list.theme')); $config = new ConfigConfigurator($recipe_data['config'], $path, \Drupal::service('config.storage')); $content = new Finder($path . '/content'); @@ -90,7 +89,7 @@ private static function parse(string $file): array { // recipes. // @see ::validateRecipeExists() // @see ::validateConfigActions() - $discovery = self::getRecipeDiscovery(dirname($file, 2)); + $include_path = dirname($file, 2); $constraints = new Collection([ 'name' => new Required([ @@ -136,7 +135,7 @@ private static function parse(string $file): array { ), new Callback( callback: self::validateRecipeExists(...), - payload: $discovery, + payload: $include_path, ), ]), ]), @@ -177,7 +176,7 @@ private static function parse(string $file): array { new NotBlank(), new Callback( callback: self::validateConfigActions(...), - payload: $discovery, + payload: $include_path, ), ]), ]), @@ -232,33 +231,21 @@ private static function validateExtensionIsAvailable(string $value, ExecutionCon * The machine name of the recipe to look for. * @param \Symfony\Component\Validator\Context\ExecutionContextInterface $context * The validator execution context. - * @param \Drupal\Core\Recipe\RecipeDiscovery $discovery - * A discovery object to find other recipes. + * @param string $include_path + * The recipe's include path. */ - private static function validateRecipeExists(string $name, ExecutionContextInterface $context, RecipeDiscovery $discovery): void { + private static function validateRecipeExists(string $name, ExecutionContextInterface $context, string $include_path): void { if (empty($name)) { return; } try { - $discovery->getRecipe($name); + RecipeConfigurator::getIncludedRecipe($include_path, $name); } catch (UnknownRecipeException) { $context->addViolation('The %name recipe does not exist.', ['%name' => $name]); } } - /** - * Gets the recipe discovery object for a recipe. - * - * @param string $recipeDirectory - * The directory the contains the recipe. - * - * @return \Drupal\Core\Recipe\RecipeDiscovery - */ - private static function getRecipeDiscovery(string $recipeDirectory): RecipeDiscovery { - return new RecipeDiscovery($recipeDirectory); - } - /** * Validates that the corresponding extension is enabled for a config action. * @@ -266,10 +253,10 @@ private static function getRecipeDiscovery(string $recipeDirectory): RecipeDisco * The config action; not used. * @param \Symfony\Component\Validator\Context\ExecutionContextInterface $context * The validator execution context. - * @param \Drupal\Core\Recipe\RecipeDiscovery $discovery - * A discovery object to find other recipes. + * @param string $include_path + * The recipe's include path. */ - private static function validateConfigActions(mixed $value, ExecutionContextInterface $context, RecipeDiscovery $discovery): void { + private static function validateConfigActions(mixed $value, ExecutionContextInterface $context, string $include_path): void { $config_name = str_replace(['[config][actions]', '[', ']'], '', $context->getPropertyPath()); [$config_provider] = explode('.', $config_name); if ($config_provider === 'core') { @@ -279,7 +266,7 @@ private static function validateConfigActions(mixed $value, ExecutionContextInte $recipe_being_validated = $context->getRoot(); assert(is_array($recipe_being_validated)); - $configurator = new RecipeConfigurator($recipe_being_validated['recipes'] ?? [], $discovery); + $configurator = new RecipeConfigurator($recipe_being_validated['recipes'] ?? [], $include_path); // The config provider must either be an already-installed module or theme, // or an extension being installed by this recipe or a recipe it depends on. diff --git a/core/lib/Drupal/Core/Recipe/RecipeConfigurator.php b/core/lib/Drupal/Core/Recipe/RecipeConfigurator.php index 927bcfc17fff..8e1402af2d46 100644 --- a/core/lib/Drupal/Core/Recipe/RecipeConfigurator.php +++ b/core/lib/Drupal/Core/Recipe/RecipeConfigurator.php @@ -10,17 +10,57 @@ */ final class RecipeConfigurator { + /** + * @var \Drupal\Core\Recipe\Recipe[] + */ public readonly array $recipes; /** * @param string[] $recipes * A list of recipes for a recipe to apply. The recipes will be applied in * the order listed. - * @param \Drupal\Core\Recipe\RecipeDiscovery $recipeDiscovery - * Recipe discovery. + * @param string $include_path + * The recipe's include path. */ - public function __construct(array $recipes, RecipeDiscovery $recipeDiscovery) { - $this->recipes = array_map([$recipeDiscovery, 'getRecipe'], $recipes); + public function __construct(array $recipes, string $include_path) { + $this->recipes = array_map(fn(string $name) => static::getIncludedRecipe($include_path, $name), $recipes); + } + + /** + * Gets an included recipe object. + * + * @param string $include_path + * The recipe's include path. + * @param string $name + * The machine name of the recipe to get. + * + * @return \Drupal\Core\Recipe\Recipe + * The recipe object. + * + * @throws \Drupal\Core\Recipe\UnknownRecipeException + * Thrown when the recipe cannot be found. + */ + public static function getIncludedRecipe(string $include_path, string $name): Recipe { + // In order to allow recipes to include core provided recipes, $name can be + // a Drupal root relative path to a recipe folder. For example, a recipe can + // include the core provided 'article_tags' recipe by listing the recipe as + // 'core/recipes/article_tags'. It is strongly recommended not to rely on + // relative paths for including recipes. Required recipes should be put in + // the same parent directory as the recipe being applied. Note, only linux + // style directory separators are supported. PHP on Windows can resolve the + // mix of directory separators. + if (str_contains($name, '/')) { + $path = \Drupal::root() . "/$name/recipe.yml"; + } + else { + $path = $include_path . "/$name/recipe.yml"; + } + + if (file_exists($path)) { + return Recipe::createFromDirectory(dirname($path)); + } + $search_path = dirname($path, 2); + throw new UnknownRecipeException($name, $search_path, sprintf("Can not find the %s recipe, search path: %s", $name, $search_path)); } /** diff --git a/core/lib/Drupal/Core/Recipe/RecipeDiscovery.php b/core/lib/Drupal/Core/Recipe/RecipeDiscovery.php deleted file mode 100644 index e27bcee39012..000000000000 --- a/core/lib/Drupal/Core/Recipe/RecipeDiscovery.php +++ /dev/null @@ -1,58 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\Core\Recipe; - -/** - * @internal - * This API is experimental. - */ -final class RecipeDiscovery { - - /** - * Constructs a recipe discovery object. - * - * @param string $path - * The path will be searched folders containing a recipe.yml. There will be - * no traversal further into the directory structure. - */ - public function __construct(protected string $path) { - } - - /** - * Gets a recipe object. - * - * @param string $name - * The machine name of the recipe to find. - * - * @return \Drupal\Core\Recipe\Recipe - * The recipe object. - * - * @throws \Drupal\Core\Recipe\UnknownRecipeException - * Thrown when the recipe cannot be found. - */ - public function getRecipe(string $name): Recipe { - // In order to allow recipes to include core provided recipes, $name can be - // a Drupal root relative path to a recipe folder. For example, a recipe can - // include the core provided 'article_tags' recipe by listing the recipe as - // 'core/recipes/article_tags'. It is strongly recommended not to rely on - // relative paths for including recipes. Required recipes should be put in - // the same parent directory as the recipe being applied. Note, only linux - // style directory separators are supported. PHP on Windows can resolve the - // mix of directory separators. - if (str_contains($name, '/')) { - $path = \Drupal::root() . "/$name/recipe.yml"; - } - else { - $path = $this->path . "/$name/recipe.yml"; - } - - if (file_exists($path)) { - return Recipe::createFromDirectory(dirname($path)); - } - $search_path = dirname($path, 2); - throw new UnknownRecipeException($name, $search_path, sprintf("Can not find the %s recipe, search path: %s", $name, $search_path)); - } - -} diff --git a/core/tests/Drupal/KernelTests/Core/Recipe/RecipeConfiguratorTest.php b/core/tests/Drupal/KernelTests/Core/Recipe/RecipeConfiguratorTest.php index 640cc182de87..e772dd023a9d 100644 --- a/core/tests/Drupal/KernelTests/Core/Recipe/RecipeConfiguratorTest.php +++ b/core/tests/Drupal/KernelTests/Core/Recipe/RecipeConfiguratorTest.php @@ -6,20 +6,19 @@ use Drupal\Core\Recipe\Recipe; use Drupal\Core\Recipe\RecipeConfigurator; -use Drupal\Core\Recipe\RecipeDiscovery; +use Drupal\Core\Recipe\UnknownRecipeException; use Drupal\KernelTests\KernelTestBase; /** - * @covers \Drupal\Core\Recipe\RecipeConfigurator + * @coversDefaultClass \Drupal\Core\Recipe\RecipeConfigurator * @group Recipe */ class RecipeConfiguratorTest extends KernelTestBase { public function testRecipeConfigurator(): void { - $discovery = new RecipeDiscovery('core/tests/fixtures/recipes'); $recipe_configurator = new RecipeConfigurator( ['install_two_modules', 'install_node_with_config', 'recipe_include'], - $discovery + 'core/tests/fixtures/recipes' ); // Private method "listAllRecipes". $reflection = new \ReflectionMethod('\Drupal\Core\Recipe\RecipeConfigurator', 'listAllRecipes'); @@ -50,4 +49,37 @@ public function testRecipeConfigurator(): void { $this->assertEquals(1, array_count_values($recipe_extensions)['field']); } + /** + * Tests that RecipeConfigurator can load recipes. + * + * @testWith ["install_two_modules", "Install two modules"] + * ["recipe_include", "Recipe include"] + * + * @covers ::getIncludedRecipe + */ + public function testIncludedRecipeLoader(string $recipe, string $name): void { + $recipe = RecipeConfigurator::getIncludedRecipe('core/tests/fixtures/recipes', $recipe); + $this->assertSame($name, $recipe->name); + } + + /** + * Tests exception thrown when RecipeConfigurator cannot find a recipe. + * + * @testWith ["no_recipe"] + * ["does_not_exist"] + * + * @covers ::getIncludedRecipe + */ + public function testIncludedRecipeLoaderException(string $recipe): void { + try { + RecipeConfigurator::getIncludedRecipe('core/tests/fixtures/recipes', $recipe); + $this->fail('Expected exception not thrown'); + } + catch (UnknownRecipeException $e) { + $this->assertSame($recipe, $e->recipe); + $this->assertSame('core/tests/fixtures/recipes', $e->searchPath); + $this->assertSame('Can not find the ' . $recipe . ' recipe, search path: ' . $e->searchPath, $e->getMessage()); + } + } + } diff --git a/core/tests/Drupal/KernelTests/Core/Recipe/RecipeDiscoveryTest.php b/core/tests/Drupal/KernelTests/Core/Recipe/RecipeDiscoveryTest.php deleted file mode 100644 index 165fb07de236..000000000000 --- a/core/tests/Drupal/KernelTests/Core/Recipe/RecipeDiscoveryTest.php +++ /dev/null @@ -1,48 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\KernelTests\Core\Recipe; - -use Drupal\Core\Recipe\RecipeDiscovery; -use Drupal\Core\Recipe\UnknownRecipeException; -use Drupal\KernelTests\KernelTestBase; - -/** - * @coversDefaultClass \Drupal\Core\Recipe\RecipeDiscovery - * @group Recipe - */ -class RecipeDiscoveryTest extends KernelTestBase { - - /** - * Tests that recipe discovery can find recipes. - * - * @testWith ["install_two_modules", "Install two modules"] - * ["recipe_include", "Recipe include"] - */ - public function testRecipeDiscovery(string $recipe, string $name): void { - $discovery = new RecipeDiscovery('core/tests/fixtures/recipes'); - $recipe = $discovery->getRecipe($recipe); - $this->assertSame($name, $recipe->name); - } - - /** - * Tests the exception thrown when recipe discovery cannot find a recipe. - * - * @testWith ["no_recipe"] - * ["does_not_exist"] - */ - public function testRecipeDiscoveryException(string $recipe): void { - $discovery = new RecipeDiscovery('core/tests/fixtures/recipes'); - try { - $discovery->getRecipe($recipe); - $this->fail('Expected exception not thrown'); - } - catch (UnknownRecipeException $e) { - $this->assertSame($recipe, $e->recipe); - $this->assertSame('core/tests/fixtures/recipes', $e->searchPath); - $this->assertSame('Can not find the ' . $recipe . ' recipe, search path: ' . $e->searchPath, $e->getMessage()); - } - } - -} -- GitLab