diff --git a/core/modules/block/block.module b/core/modules/block/block.module index 5b44e1be7f0b5285191c866b5a486bec79b9c420..c88ac33a14ac932b6d4b0f24a43f9059b1fa4596 100644 --- a/core/modules/block/block.module +++ b/core/modules/block/block.module @@ -128,11 +128,12 @@ function block_theme_initialize($theme) { $default_theme_blocks = \Drupal::entityTypeManager()->getStorage('block')->loadByProperties(['theme' => $default_theme]); foreach ($default_theme_blocks as $default_theme_block_id => $default_theme_block) { if (str_starts_with($default_theme_block_id, $default_theme . '_')) { - $id = str_replace($default_theme, $theme, $default_theme_block_id); + $id = str_replace($default_theme . '_', '', $default_theme_block_id); } else { - $id = $theme . '_' . $default_theme_block_id; + $id = $default_theme_block_id; } + $id = \Drupal::service('block.repository')->getUniqueMachineName($id, $theme); $block = $default_theme_block->createDuplicateBlock($id, $theme); // If the region isn't supported by the theme, assign the block to the // theme's default region. diff --git a/core/modules/block/src/BlockForm.php b/core/modules/block/src/BlockForm.php index 4d27062930cfef4be38d871d9d0bb2d1b848342f..8c7bedc934edad472f40b01edadb87f6a87044b9 100644 --- a/core/modules/block/src/BlockForm.php +++ b/core/modules/block/src/BlockForm.php @@ -73,6 +73,13 @@ class BlockForm extends EntityForm { */ protected $pluginFormFactory; + /** + * The block repository service. + * + * @var \Drupal\block\BlockRepositoryInterface + */ + protected $blockRepository; + /** * Constructs a BlockForm object. * @@ -88,14 +95,21 @@ class BlockForm extends EntityForm { * The theme handler. * @param \Drupal\Core\Plugin\PluginFormFactoryInterface $plugin_form_manager * The plugin form manager. + * @param \Drupal\block\BlockRepositoryInterface|null $block_repository + * The block repository service. */ - public function __construct(EntityTypeManagerInterface $entity_type_manager, ExecutableManagerInterface $manager, ContextRepositoryInterface $context_repository, LanguageManagerInterface $language, ThemeHandlerInterface $theme_handler, PluginFormFactoryInterface $plugin_form_manager) { + public function __construct(EntityTypeManagerInterface $entity_type_manager, ExecutableManagerInterface $manager, ContextRepositoryInterface $context_repository, LanguageManagerInterface $language, ThemeHandlerInterface $theme_handler, PluginFormFactoryInterface $plugin_form_manager, ?BlockRepositoryInterface $block_repository = NULL) { + if ($block_repository === NULL) { + @trigger_error('Calling ' . __METHOD__ . ' without the $block_repository argument is deprecated in drupal:10.2.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3333575', E_USER_DEPRECATED); + $block_repository = \Drupal::service('block.repository'); + } $this->storage = $entity_type_manager->getStorage('block'); $this->manager = $manager; $this->contextRepository = $context_repository; $this->language = $language; $this->themeHandler = $theme_handler; $this->pluginFormFactory = $plugin_form_manager; + $this->blockRepository = $block_repository; } /** @@ -108,7 +122,8 @@ public static function create(ContainerInterface $container) { $container->get('context.repository'), $container->get('language_manager'), $container->get('theme_handler'), - $container->get('plugin_form.factory') + $container->get('plugin_form.factory'), + $container->get('block.repository') ); } @@ -380,7 +395,7 @@ protected function submitVisibility(array $form, FormStateInterface $form_state) } /** - * Generates a unique machine name for a block. + * Generates a unique machine name for a block based on a suggested string. * * @param \Drupal\block\BlockInterface $block * The block entity. @@ -390,28 +405,7 @@ protected function submitVisibility(array $form, FormStateInterface $form_state) */ public function getUniqueMachineName(BlockInterface $block) { $suggestion = $block->getPlugin()->getMachineNameSuggestion(); - if ($block->getTheme()) { - $suggestion = $block->getTheme() . '_' . $suggestion; - } - - // Get all the blocks which starts with the suggested machine name. - $query = $this->storage->getQuery(); - $query->condition('id', $suggestion, 'CONTAINS'); - $block_ids = $query->execute(); - - $block_ids = array_map(function ($block_id) { - $parts = explode('.', $block_id); - return end($parts); - }, $block_ids); - - // Iterate through potential IDs until we get a new one. E.g. - // 'plugin', 'plugin_2', 'plugin_3', etc. - $count = 1; - $machine_default = $suggestion; - while (in_array($machine_default, $block_ids)) { - $machine_default = $suggestion . '_' . ++$count; - } - return $machine_default; + return $this->blockRepository->getUniqueMachineName($suggestion, $block->getTheme()); } /** diff --git a/core/modules/block/src/BlockRepository.php b/core/modules/block/src/BlockRepository.php index 4103270d55b5db592ef530849df24c120a4083e2..3a5a00d84694139d3fb962f525bca2e51c58cc70 100644 --- a/core/modules/block/src/BlockRepository.php +++ b/core/modules/block/src/BlockRepository.php @@ -83,4 +83,32 @@ public function getVisibleBlocksPerRegion(array &$cacheable_metadata = []) { return $assignments; } + /** + * {@inheritdoc} + */ + public function getUniqueMachineName(string $suggestion, string $theme = NULL): string { + if ($theme) { + $suggestion = $theme . '_' . $suggestion; + } + // Get all the block machine names that begin with the suggested string. + $query = $this->blockStorage->getQuery(); + $query->accessCheck(FALSE); + $query->condition('id', $suggestion, 'CONTAINS'); + $block_ids = $query->execute(); + + $block_ids = array_map(function ($block_id) { + $parts = explode('.', $block_id); + return end($parts); + }, $block_ids); + + // Iterate through potential IDs until we get a new one. E.g. + // For example, 'plugin', 'plugin_2', 'plugin_3', etc. + $count = 1; + $machine_default = $suggestion; + while (in_array($machine_default, $block_ids)) { + $machine_default = $suggestion . '_' . ++$count; + } + return $machine_default; + } + } diff --git a/core/modules/block/src/BlockRepositoryInterface.php b/core/modules/block/src/BlockRepositoryInterface.php index 9c4f871a7ee906e1d5f9e757c3534c2bcb0df4df..09f91168406b5f6a5d8801fb1050254cf8667471 100644 --- a/core/modules/block/src/BlockRepositoryInterface.php +++ b/core/modules/block/src/BlockRepositoryInterface.php @@ -31,4 +31,17 @@ interface BlockRepositoryInterface { */ public function getVisibleBlocksPerRegion(array &$cacheable_metadata = []); + /** + * Based on a suggested string generates a unique machine name for a block. + * + * @param string $suggestion + * The suggested block ID. + * @param string $theme + * The machine name of the theme. + * + * @return string + * Returns the unique name. + */ + public function getUniqueMachineName(string $suggestion, string $theme): string; + } diff --git a/core/modules/block/tests/src/Kernel/NewDefaultThemeBlocksTest.php b/core/modules/block/tests/src/Kernel/NewDefaultThemeBlocksTest.php index 06bf3544e4dbbe7fe72a8d53684520431a48a857..388d0ac8be4c40eefeaecf5a10106eea15f0f9ef 100644 --- a/core/modules/block/tests/src/Kernel/NewDefaultThemeBlocksTest.php +++ b/core/modules/block/tests/src/Kernel/NewDefaultThemeBlocksTest.php @@ -23,14 +23,36 @@ class NewDefaultThemeBlocksTest extends KernelTestBase { 'user', ]; + /** + * The theme installer service. + * + * @var \Drupal\Core\Extension\ThemeInstallerInterface + */ + protected $themeInstaller; + + /** + * The default theme. + */ + protected $defaultTheme; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + + $this->installConfig(['system']); + /** @var \Drupal\Core\Extension\ThemeInstallerInterface $themeInstaller */ + $this->themeInstaller = $this->container->get('theme_installer'); + $this->defaultTheme = $this->config('system.theme')->get('default'); + } + /** * Check the blocks are correctly copied by block_themes_installed(). */ public function testNewDefaultThemeBlocks() { - $this->installConfig(['system']); - /** @var \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer */ - $theme_installer = $this->container->get('theme_installer'); - $default_theme = $this->config('system.theme')->get('default'); + $default_theme = $this->defaultTheme; + $theme_installer = $this->themeInstaller; $theme_installer->install([$default_theme]); // Add two instances of the user login block. @@ -61,25 +83,96 @@ public function testNewDefaultThemeBlocks() { // Ensure that the new default theme has the same blocks as the previous // default theme. $default_block_names = $block_storage->getQuery() + ->accessCheck(FALSE) + ->condition('theme', $default_theme) + ->execute(); + $new_blocks = $block_storage->getQuery() + ->accessCheck(FALSE) + ->condition('theme', $new_theme) + ->execute(); + $this->assertSameSize($default_block_names, $new_blocks); + + foreach ($default_block_names as $default_block_name) { + // Remove the matching block from the list of blocks in the new theme. + // For example, if the old theme has block.block.stark_admin, + // unset block.block.olivero_admin. + unset($new_blocks[str_replace($default_theme . '_', $new_theme . '_', $default_block_name)]); + } + $this->assertEmpty($new_blocks); + + // Install a hidden base theme and ensure blocks are not copied. + $base_theme = 'test_basetheme'; + $theme_installer->install([$base_theme]); + $new_blocks = $block_storage->getQuery() + ->accessCheck(FALSE) + ->condition('theme', $base_theme) + ->execute(); + $this->assertEmpty($new_blocks); + } + + /** + * Checks that a theme block is still created when same ID exists. + */ + public function testBlockCollision() { + $default_theme = $this->defaultTheme; + $theme_installer = $this->themeInstaller; + $theme_installer->install([$default_theme]); + + // Add two instances of the user login block with machine + // names that will collide. + $this->placeBlock('user_login_block', [ + 'id' => 'user_login_block', + ]); + $this->placeBlock('user_login_block', [ + 'id' => $default_theme . '_user_login_block', + ]); + + // Add an instance of a different block. + $this->placeBlock('system_powered_by_block', [ + 'id' => $default_theme . '_' . strtolower($this->randomMachineName(8)), + ]); + + // Install a different theme that does not have blocks. + $new_theme = 'test_theme'; + // The new theme is different from the previous default theme. + $this->assertNotEquals($new_theme, $default_theme); + + $theme_installer->install([$new_theme]); + $this->config('system.theme') + ->set('default', $new_theme) + ->save(); + + $block_storage = $this->container->get('entity_type.manager')->getStorage('block'); + + // Ensure that the new default theme has the same blocks as the previous + // default theme. + $default_block_names = $block_storage->getQuery() + ->accessCheck(FALSE) ->condition('theme', $default_theme) ->execute(); $new_blocks = $block_storage->getQuery() + ->accessCheck(FALSE) ->condition('theme', $new_theme) ->execute(); $this->assertSameSize($default_block_names, $new_blocks); foreach ($default_block_names as $default_block_name) { // Remove the matching block from the list of blocks in the new theme. - // E.g., if the old theme has block.block.stark_admin, + // For example, if the old theme has block.block.stark_admin, // unset block.block.olivero_admin. unset($new_blocks[str_replace($default_theme . '_', $new_theme . '_', $default_block_name)]); } + // The test_theme_user_login_block machine name is already in use, so therefore + // \Drupal\block\BlockRepository::getUniqueMachineName + // appends a counter. + unset($new_blocks[$new_theme . '_user_login_block_2']); $this->assertEmpty($new_blocks); // Install a hidden base theme and ensure blocks are not copied. $base_theme = 'test_basetheme'; $theme_installer->install([$base_theme]); $new_blocks = $block_storage->getQuery() + ->accessCheck(FALSE) ->condition('theme', $base_theme) ->execute(); // Installing a hidden base theme does not copy the blocks from the default diff --git a/core/modules/block/tests/src/Unit/BlockFormTest.php b/core/modules/block/tests/src/Unit/BlockFormTest.php index fdaee38e1ede03223fa4832e440097ed7377a3e5..9c0047155f892a7fe1928ea042c047ffb718ea36 100644 --- a/core/modules/block/tests/src/Unit/BlockFormTest.php +++ b/core/modules/block/tests/src/Unit/BlockFormTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\block\Unit; use Drupal\block\BlockForm; +use Drupal\block\BlockRepository; use Drupal\block\Entity\Block; use Drupal\Core\Block\BlockBase; use Drupal\Core\Plugin\PluginFormFactoryInterface; @@ -35,7 +36,6 @@ class BlockFormTest extends UnitTestCase { */ protected $language; - /** * The theme handler. * @@ -43,6 +43,13 @@ class BlockFormTest extends UnitTestCase { */ protected $themeHandler; + /** + * The theme manager service. + * + * @var \Drupal\Core\Theme\ThemeManagerInterface|\PHPUnit\Framework\MockObject\MockObject + */ + protected $themeManager; + /** * The entity type manager. * @@ -50,6 +57,13 @@ class BlockFormTest extends UnitTestCase { */ protected $entityTypeManager; + /** + * The mocked context handler. + * + * @var \Drupal\Core\Plugin\Context\ContextHandlerInterface|\PHPUnit\Framework\MockObject\MockObject + */ + protected $contextHandler; + /** * The mocked context repository. * @@ -64,6 +78,13 @@ class BlockFormTest extends UnitTestCase { */ protected $pluginFormFactory; + /** + * The block repository. + * + * @var \Drupal\block\BlockRepositoryInterface + */ + protected $blockRepository; + /** * {@inheritdoc} */ @@ -82,6 +103,10 @@ protected function setUp(): void { ->willReturn($this->storage); $this->pluginFormFactory = $this->prophesize(PluginFormFactoryInterface::class); + + $this->themeManager = $this->createMock('\Drupal\Core\Theme\ThemeManagerInterface'); + $this->contextHandler = $this->createMock('Drupal\Core\Plugin\Context\ContextHandlerInterface'); + $this->blockRepository = new BlockRepository($this->entityTypeManager, $this->themeManager, $this->contextHandler); } /** @@ -136,10 +161,10 @@ public function testGetUniqueMachineName() { ->method('getQuery') ->willReturn($query); - $block_form_controller = new BlockForm($this->entityTypeManager, $this->conditionManager, $this->contextRepository, $this->language, $this->themeHandler, $this->pluginFormFactory->reveal()); + $block_form_controller = new BlockForm($this->entityTypeManager, $this->conditionManager, $this->contextRepository, $this->language, $this->themeHandler, $this->pluginFormFactory->reveal(), $this->blockRepository); - // Ensure that the block with just one other instance gets the next available - // name suggestion. + // Ensure that the block with just one other instance gets + // the next available name suggestion. $this->assertEquals('test_2', $block_form_controller->getUniqueMachineName($blocks['test'])); // Ensure that the block with already three instances (_0, _1, _2) gets the