From 1b4c4238a71decb5efc72281780f965d9cf99dcd Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Tue, 12 May 2020 23:53:01 +0100 Subject: [PATCH] Issue #3123537 by tedbow, dww, DamienMcKenna, xjm, Berdir, tim.plunkett, pingwin4eg: /admin/reports/status should not WSOD if it finds an extension missing core_version_requirement in its info.yml file --- .../Core/Extension/InfoParserDynamic.php | 2 +- .../src/Controller/SystemController.php | 8 +++ .../system/src/Form/ModulesListForm.php | 8 +++ .../Form/ModulesListFormWebTest.php | 57 ++++++++++++++++++- .../src/Functional/Theme/ThemeUiTest.php | 56 ++++++++++++++++++ .../UpdateSystem/UpdateScriptTest.php | 22 +++++++ .../Core/Extension/InfoParserUnitTest.php | 32 ++++++++++- 7 files changed, 179 insertions(+), 6 deletions(-) diff --git a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php index 01c8430d0235..46e248497f63 100644 --- a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php +++ b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php @@ -68,7 +68,7 @@ public function parse($filename) { // easier for contrib to use test modules. $parsed_info['core_version_requirement'] = \Drupal::VERSION; } - else { + elseif (!isset($parsed_info['core'])) { // Non-core extensions must specify core compatibility. throw new InfoParserException("The 'core_version_requirement' key must be present in " . $filename); } diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index 5942ff3ce0fd..a04724fe804d 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -209,11 +209,19 @@ public function themesPage() { $theme_groups = ['installed' => [], 'uninstalled' => []]; $admin_theme = $config->get('admin'); $admin_theme_options = []; + $incompatible_installed = FALSE; foreach ($themes as &$theme) { if (!empty($theme->info['hidden'])) { continue; } + if (!$incompatible_installed && $theme->info['core_incompatible'] && $theme->status) { + $incompatible_installed = TRUE; + $this->messenger()->addWarning($this->t( + 'There are errors with some installed themes. Visit the <a href=":link">status report page</a> for more information.', + [':link' => Url::fromRoute('system.status')->toString()] + )); + } $theme->is_default = ($theme->getName() == $theme_default); $theme->is_admin = ($theme->getName() == $admin_theme || ($theme->is_default && empty($admin_theme))); $theme->is_experimental = isset($theme->info['experimental']) && $theme->info['experimental']; diff --git a/core/modules/system/src/Form/ModulesListForm.php b/core/modules/system/src/Form/ModulesListForm.php index d0c0a6fbb2a6..80178fdd7b89 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -171,12 +171,20 @@ public function buildForm(array $form, FormStateInterface $form_state) { // Iterate over each of the modules. $form['modules']['#tree'] = TRUE; + $incompatible_installed = FALSE; foreach ($modules as $filename => $module) { if (empty($module->info['hidden'])) { $package = $module->info['package']; $form['modules'][$package][$filename] = $this->buildRow($modules, $module, $distribution); $form['modules'][$package][$filename]['#parents'] = ['modules', $filename]; } + if (!$incompatible_installed && $module->status && $module->info['core_incompatible']) { + $incompatible_installed = TRUE; + $this->messenger()->addWarning($this->t( + 'There are errors with some installed modules. Visit the <a href=":link">status report page</a> for more information.', + [':link' => Url::fromRoute('system.status')->toString()] + )); + } } // Add a wrapper around every package. diff --git a/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php b/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php index 7ce0b2737038..7a248e319335 100644 --- a/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php +++ b/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\system\Functional\Form; +use Drupal\Core\Serialization\Yaml; use Drupal\Tests\BrowserTestBase; /** @@ -73,9 +74,7 @@ public function testModulesListFormWithInvalidInfoFile() { type: module core: 9.x BROKEN, - // Checking for 'core_version_requirement' is done before checking - // for a valid 'core' value. - 'expected_error' => "The 'core_version_requirement' key must be present in $file_path", + 'expected_error' => "'core: 9.x' is not supported. Use 'core_version_requirement' to specify core compatibility. Only 'core: 8.x' is supported to provide backwards compatibility for Drupal 8 when needed in $file_path", ], [ 'yml' => <<<BROKEN @@ -123,4 +122,56 @@ public function testRequiredByThemeMessage() { $this->assertSession()->pageTextContains('Test Theme Depending on Modules (Theme) (Disabled)'); } + /** + * Tests that incompatible modules message is shown. + */ + public function testInstalledIncompatibleModule() { + $incompatible_modules_message = 'There are errors with some installed modules. Visit the status report page for more information.'; + $path = \Drupal::getContainer()->getParameter('site.path') . "/modules/changing_module"; + mkdir($path, 0777, TRUE); + $file_path = "$path/changing_module.info.yml"; + $info = [ + 'name' => 'Module that changes', + 'type' => 'module', + ]; + $compatible_info = $info + ['core_version_requirement' => '*']; + + file_put_contents($file_path, Yaml::encode($compatible_info)); + $edit = ['modules[changing_module][enable]' => 'changing_module']; + $this->drupalGet('admin/modules'); + $this->drupalPostForm('admin/modules', $edit, t('Install')); + $this->assertText('Module Module that changes has been enabled.'); + + $incompatible_updates = [ + [ + 'core_version_requirement' => '^1', + ], + [ + 'core' => '8.x', + ], + ]; + foreach ($incompatible_updates as $incompatible_update) { + $incompatible_info = $info + $incompatible_update; + file_put_contents($file_path, Yaml::encode($incompatible_info)); + $this->drupalGet('admin/modules'); + $this->assertText($incompatible_modules_message); + + file_put_contents($file_path, Yaml::encode($compatible_info)); + $this->drupalGet('admin/modules'); + $this->assertNoText($incompatible_modules_message); + } + // Uninstall the module and ensure that incompatible modules message is not + // displayed for modules that are not installed. + $edit = ['uninstall[changing_module]' => 'changing_module']; + $this->drupalPostForm('admin/modules/uninstall', $edit, t('Uninstall')); + $this->drupalPostForm(NULL, NULL, t('Uninstall')); + $this->assertText('The selected modules have been uninstalled.'); + foreach ($incompatible_updates as $incompatible_update) { + $incompatible_info = $info + $incompatible_update; + file_put_contents($file_path, Yaml::encode($incompatible_info)); + $this->drupalGet('admin/modules'); + $this->assertNoText($incompatible_modules_message); + } + } + } diff --git a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php index 228e6c0111dd..8e8aead9f747 100644 --- a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php +++ b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\system\Functional\Theme; +use Drupal\Core\Serialization\Yaml; use Drupal\Tests\BrowserTestBase; /** @@ -318,4 +319,59 @@ public function testInstallModuleWithIncompatibleDependencies() { $this->assertStringContainsString('This theme requires the listed modules to operate correctly.', $theme_container->getText()); } + /** + * Tests that incompatible themes message is shown. + */ + public function testInstalledIncompatibleTheme() { + $page = $this->getSession()->getPage(); + $assert_session = $this->assertSession(); + $incompatitable_themes_message = 'There are errors with some installed themes. Visit the status report page for more information.'; + $path = \Drupal::getContainer()->getParameter('site.path') . "/themes/changing_theme"; + mkdir($path, 0777, TRUE); + $file_path = "$path/changing_theme.info.yml"; + $theme_name = 'Theme that changes'; + $info = [ + 'name' => $theme_name, + 'type' => 'theme', + 'base theme' => FALSE, + ]; + + $compatible_info = $info + ['core_version_requirement' => '*']; + + file_put_contents($file_path, Yaml::encode($compatible_info)); + $this->drupalGet('admin/appearance'); + $this->assertNoText($incompatitable_themes_message); + $page->clickLink("Install $theme_name theme"); + $assert_session->addressEquals('admin/appearance'); + $assert_session->pageTextContains("The $theme_name theme has been installed"); + + $incompatible_updates = [ + [ + 'core_version_requirement' => '^1', + ], + [ + 'core' => '8.x', + ], + ]; + foreach ($incompatible_updates as $incompatible_update) { + $incompatible_info = $info + $incompatible_update; + file_put_contents($file_path, Yaml::encode($incompatible_info)); + $this->drupalGet('admin/appearance'); + $this->assertText($incompatitable_themes_message); + + file_put_contents($file_path, Yaml::encode($compatible_info)); + $this->drupalGet('admin/appearance'); + $this->assertNoText($incompatitable_themes_message); + } + // Uninstall the theme and ensure that incompatible themes message is not + // displayed for themes that are not installed. + $this->uninstallTheme($theme_name); + foreach ($incompatible_updates as $incompatible_update) { + $incompatible_info = $info + $incompatible_update; + file_put_contents($file_path, Yaml::encode($incompatible_info)); + $this->drupalGet('admin/appearance'); + $this->assertNoText($incompatitable_themes_message); + } + } + } diff --git a/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php index cd0197d95990..b1860a305e8b 100644 --- a/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php +++ b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php @@ -323,6 +323,28 @@ public function providerExtensionCompatibilityChange() { ], 'The following theme is installed, but it is incompatible with PHP ' . phpversion() . ":", ], + 'module: core_version_requirement key missing' => [ + [ + 'core_version_requirement' => '^8 || ^9', + 'type' => 'module', + ], + [ + 'core' => '8.x', + 'type' => 'module', + ], + $incompatible_module_message, + ], + 'theme: core_version_requirement key missing' => [ + [ + 'core_version_requirement' => '^8 || ^9', + 'type' => 'theme', + ], + [ + 'core' => '8.x', + 'type' => 'theme', + ], + $incompatible_theme_message, + ], ]; } diff --git a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php index b5559fb263ad..4f8213ab3de4 100644 --- a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php @@ -377,7 +377,7 @@ public function testCoreWithoutCoreVersionRequirement($core) { "core_without_core_version_requirement-$core-duplicate.info.txt" => $core_without_core_version_requirement, ], ]); - $exception_message = "The 'core_version_requirement' key must be present in vfs://modules/fixtures/core_without_core_version_requirement-$core"; + $exception_message = "'core: $core' is not supported. Use 'core_version_requirement' to specify core compatibility. Only 'core: 8.x' is supported to provide backwards compatibility for Drupal 8 when needed in vfs://modules/fixtures/core_without_core_version_requirement-$core"; // Set the expected exception for the 2nd call to parse(). $this->expectException('\Drupal\Core\Extension\InfoParserException'); $this->expectExceptionMessage("$exception_message-duplicate.info.txt"); @@ -397,7 +397,6 @@ public function testCoreWithoutCoreVersionRequirement($core) { public function providerCoreWithoutCoreVersionRequirement() { return [ '7.x' => ['7.x'], - '8.x' => ['8.x'], '9.x' => ['9.x'], '10.x' => ['10.x'], ]; @@ -663,4 +662,33 @@ public function testUnparsableCoreVersionRequirement() { $this->infoParser->parse(vfsStream::url('modules/fixtures/unparsable_core_version_requirement.info.txt')); } + /** + * Tests an info file with 'core: 8.x' but without 'core_version_requirement'. + * + * @covers ::parse + */ + public function testCore8xNoCoreVersionRequirement() { + $info = <<<INFO +package: Core +core: 8.x +version: VERSION +type: module +name: Module for That +dependencies: + - field +INFO; + + vfsStream::setup('modules'); + foreach (['1', '2'] as $file_delta) { + $filename = "core_version_requirement-$file_delta.info.txt"; + vfsStream::create([ + 'fixtures' => [ + $filename => $info, + ], + ]); + $info_values = $this->infoParser->parse(vfsStream::url("modules/fixtures/$filename")); + $this->assertSame(TRUE, $info_values['core_incompatible'], "Expected 'core_incompatible's for file: $filename"); + } + } + } -- GitLab