diff --git a/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php b/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php index 4ada9bad9c8bd8e08f848a2c6d2ae9ed554a7ade..db545fc71628f09156f115c21f0445e87ccce808 100644 --- a/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php +++ b/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php @@ -96,7 +96,7 @@ class SuccessfulUpdateTest extends UpdaterFormTestBase { // Ensure that a list of pending database updates is visible, along with a // short explanation, in the warning messages. $warning_messages = $assert_session->elementExists('xpath', '//div[@data-drupal-messages]//div[@aria-label="Warning message"]'); - $this->assertStringContainsString('Possible database updates have been detected in the following extensions.<ul><li>System</li><li>Automatic Updates Theme With Updates</li></ul>', $warning_messages->getHtml()); + $this->assertStringContainsString('Database updates have been detected in the following extensions.<ul><li>System</li><li>Automatic Updates Theme With Updates</li></ul>', $warning_messages->getHtml()); $page->pressButton('Continue'); $this->checkForMetaRefresh(); diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php index 1df91013c3b01fcb612bfae46df96dd246bbc5d7..60c6dbea82fffe68fb156a7b1299f4b0c8ee681a 100644 --- a/package_manager/src/Validator/StagedDBUpdateValidator.php +++ b/package_manager/src/Validator/StagedDBUpdateValidator.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\package_manager\Validator; +use Drupal\Component\Assertion\Inspector; use Drupal\Core\Extension\Extension; use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\Extension\ThemeExtensionList; @@ -54,7 +55,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface { $extensions_with_updates = $this->getExtensionsWithDatabaseUpdates($stage_dir); if ($extensions_with_updates) { $extensions_with_updates = array_map($this->t(...), $extensions_with_updates); - $event->addWarning($extensions_with_updates, $this->t('Possible database updates have been detected in the following extensions.')); + $event->addWarning($extensions_with_updates, $this->t('Database updates have been detected in the following extensions.')); } } @@ -90,14 +91,14 @@ class StagedDBUpdateValidator implements EventSubscriberInterface { $stage_dir .= DIRECTORY_SEPARATOR . $web_root; } - $active_hashes = $this->getHashes($active_dir, $extension); - $staged_hashes = $this->getHashes($stage_dir, $extension); + $active_functions = $this->getUpdateFunctions($active_dir, $extension); + $staged_functions = $this->getUpdateFunctions($stage_dir, $extension); - return $active_hashes !== $staged_hashes; + return (bool) array_diff($staged_functions, $active_functions); } /** - * Returns hashes of the .install and .post-update.php files for a module. + * Returns a list of all update functions for a module. * * @param string $root_dir * The root directory of the Drupal code base. @@ -105,25 +106,72 @@ class StagedDBUpdateValidator implements EventSubscriberInterface { * The module to check. * * @return string[] - * The hashes of the module's .install and .post_update.php files, in that - * order, if they exist. The array will be keyed by file extension. + * The names of the update functions in the module's .install and + * .post_update.php files. */ - protected function getHashes(string $root_dir, Extension $extension): array { + protected function getUpdateFunctions(string $root_dir, Extension $extension): array { + $name = $extension->getName(); + $path = implode(DIRECTORY_SEPARATOR, [ $root_dir, $extension->getPath(), - $extension->getName(), + $name, ]); - $hashes = []; + $function_names = []; - foreach (['.install', '.post_update.php'] as $suffix) { + $patterns = [ + '.install' => '/^' . $name . '_update_[0-9]+$/i', + '.post_update.php' => '/^' . $name . '_post_update_.+$/i', + ]; + foreach ($patterns as $suffix => $pattern) { $file = $path . $suffix; - if (file_exists($file)) { - $hashes[$suffix] = hash_file('sha256', $file); + if (!file_exists($file)) { + continue; + } + // Parse the file and scan for named functions which match the pattern. + $code = file_get_contents($file); + $tokens = token_get_all($code); + + for ($i = 0; $i < count($tokens); $i++) { + $chunk = array_slice($tokens, $i, 3); + if ($this->tokensMatchFunctionNamePattern($chunk, $pattern)) { + $function_names[] = $chunk[2][1]; + } } } - return $hashes; + return $function_names; + } + + /** + * Determines if a set of tokens contain a function name matching a pattern. + * + * @param array[] $tokens + * A set of three tokens, part of a stream returned by token_get_all(). + * @param string $pattern + * If the tokens declare a named function, a regular expression to test the + * function name against. + * + * @return bool + * TRUE if the given tokens declare a function whose name matches the given + * pattern; FALSE otherwise. + * + * @see token_get_all() + */ + private function tokensMatchFunctionNamePattern(array $tokens, string $pattern): bool { + if (count($tokens) !== 3 || !Inspector::assertAllStrictArrays($tokens)) { + return FALSE; + } + // A named function declaration will always be a T_FUNCTION (the word + // `function`), followed by T_WHITESPACE (or the code would be syntactically + // invalid), followed by a T_STRING (the function name). This will ignore + // anonymous functions, but match class methods (although class methods are + // highly unlikely to match the naming patterns of update hooks). + $names = array_map('token_name', array_column($tokens, 0)); + if ($names === ['T_FUNCTION', 'T_WHITESPACE', 'T_STRING']) { + return (bool) preg_match($pattern, $tokens[2][1]); + } + return FALSE; } /** @@ -142,7 +190,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface { * The path of the stage directory. * * @return \Drupal\Core\StringTranslation\TranslatableMarkup[] - * The names of the extensions that have possible database updates. + * The names of the extensions that have database updates. */ public function getExtensionsWithDatabaseUpdates(string $stage_dir): array { $extensions_with_updates = []; diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php index 052cb321fbf6b9ea333d184959390f1204c48a75..0e1dbc483ced10f1bbe05e596b914d397bff9e6d 100644 --- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php +++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php @@ -14,25 +14,6 @@ use Drupal\package_manager\ValidationResult; */ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase { - /** - * The extensions that will be used in this test. - * - * System and Stark are installed, so they are used to test what happens when - * database updates are detected in installed extensions. Views and Olivero - * are not installed by this test, so they are used to test what happens when - * uninstalled extensions have database updates. - * - * @var string[] - * - * @see ::setUp() - */ - private $extensions = [ - 'system' => 'core/modules/system', - 'views' => 'core/modules/views', - 'stark' => 'core/themes/stark', - 'olivero' => 'core/themes/olivero', - ]; - /** * {@inheritdoc} */ @@ -47,99 +28,169 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase { // files in the active directory. $active_dir = $this->container->get(PathLocator::class)->getProjectRoot(); - foreach ($this->extensions as $extension_name => $extension_path) { + // System and Stark are installed, so they are used to test what happens + // when database updates are detected in installed extensions. Views and + // Olivero are not installed, so they are used to test what happens when + // non-installed extensions have database updates. + $extensions = [ + 'core/modules/system', + 'core/themes/stark', + 'core/modules/views', + 'core/themes/olivero', + ]; + foreach ($extensions as $extension_path) { $extension_path = $active_dir . '/' . $extension_path; mkdir($extension_path, 0777, TRUE); - - foreach ($this->providerSuffixes() as [$suffix]) { - touch("$extension_path/$extension_name.$suffix"); + $extension_name = basename($extension_path); + + // Ensure each extension has a .install and a .post_update.php file with + // an empty update function in it. + foreach (['install', 'post_update.php'] as $suffix) { + $function_name = match ($suffix) { + 'install' => $extension_name . '_update_1000', + 'post_update.php' => $extension_name . '_post_update_test', + }; + file_put_contents("$extension_path/$extension_name.$suffix", "<?php\nfunction $function_name() {}"); } } } /** - * Data provider for several test methods. + * Data provider for ::testStagedDatabaseUpdates(). * - * @return \string[][] + * @return array[] * The test cases. */ - public function providerSuffixes(): array { + public function providerStagedDatabaseUpdate(): array { + $summary = t('Database updates have been detected in the following extensions.'); + return [ - 'hook_update_N' => ['install'], - 'hook_post_update_NAME' => ['post_update.php'], + 'schema update in installed module' => [ + 'core/modules/system', + 'install', + [ + ValidationResult::createWarning([ + t('System'), + ], $summary), + ], + ], + 'post-update in installed module' => [ + 'core/modules/system', + 'post_update.php', + [ + ValidationResult::createWarning([ + t('System'), + ], $summary), + ], + ], + 'schema update in installed theme' => [ + 'core/themes/stark', + 'install', + [ + ValidationResult::createWarning([ + t('Stark'), + ], $summary), + ], + ], + 'post-update in installed theme' => [ + 'core/themes/stark', + 'post_update.php', + [ + ValidationResult::createWarning([ + t('Stark'), + ], $summary), + ], + ], + // The validator should ignore changes in any extensions that aren't + // installed. + 'schema update in non-installed module' => [ + 'core/modules/views', + 'install', + [], + ], + 'post-update in non-installed module' => [ + 'core/modules/views', + 'post_update.php', + [], + ], + 'schema update in non-installed theme' => [ + 'core/themes/olivero', + 'install', + [], + ], + 'post-update in non-installed theme' => [ + 'core/themes/olivero', + 'post_update.php', + [], + ], ]; } /** - * Tests that no errors are raised if the stage has no DB updates. - */ - public function testNoUpdates(): void { - $stage = $this->createStage(); - $stage->create(); - $this->assertStatusCheckResults([], $stage); - } - - /** - * Tests that a warning is raised if DB update files are removed in the stage. + * Tests validation of staged database updates. * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). + * @param string $extension_dir + * The directory of the extension that should have database updates, + * relative to the stage directory. + * @param string $file_extension + * The extension of the update file, without the leading period. Must be + * either `install` or `post_update.php`. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. * - * @dataProvider providerSuffixes + * @dataProvider providerStagedDatabaseUpdate */ - public function testFileDeleted(string $suffix): void { - $stage = $this->createStage(); - $stage->create(); - - $stage_dir = $stage->getStageDirectory(); - foreach ($this->extensions as $name => $path) { - unlink("$stage_dir/$path/$name.$suffix"); - } - - $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.')); - $this->assertStatusCheckResults([$result], $stage); - } + public function testStagedDatabaseUpdate(string $extension_dir, string $file_extension, array $expected_results): void { + $extension_name = basename($extension_dir); + $relative_file_path = $extension_dir . '/' . $extension_name . '.' . $file_extension; - /** - * Tests that a warning is raised if DB update files are changed in the stage. - * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). - * - * @dataProvider providerSuffixes - */ - public function testFileChanged(string $suffix): void { $stage = $this->createStage(); $stage->create(); + // Nothing has been changed in the stage, so ensure the validator doesn't + // detect any changes. + $this->assertStatusCheckResults([], $stage); - $stage_dir = $stage->getStageDirectory(); - foreach ($this->extensions as $name => $path) { - file_put_contents("$stage_dir/$path/$name.$suffix", $this->randomString()); - } - - $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.')); - $this->assertStatusCheckResults([$result], $stage); - } - - /** - * Tests that a warning is raised if DB update files are added in the stage. - * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). - * - * @dataProvider providerSuffixes - */ - public function testFileAdded(string $suffix): void { - $stage = $this->createStage(); - $stage->create(); + $staged_update_file = $stage->getStageDirectory() . '/' . $relative_file_path; + $this->assertFileIsWritable($staged_update_file); + + // Now add a "real" update function -- either a schema update or a + // post-update, depending on what $file_extension is -- and ensure that the + // validator detects it. + $update_function_name = match ($file_extension) { + 'install' => $extension_name . '_update_1001', + 'post_update.php' => $extension_name . '_post_update_' . $this->randomMachineName(), + }; + file_put_contents($staged_update_file, "function $update_function_name() {}\n", FILE_APPEND); + $this->assertStatusCheckResults($expected_results, $stage); + + // Add a bunch of functions which are named similarly to real schema update + // and post-update functions, but not quite right, to ensure they are + // ignored by the validator. Also throw an anonymous function in there to + // ensure those are ignored as well. + $code = <<<END +<?php +function {$extension_name}_update() { \$foo = function () {}; } +function {$extension_name}_update_string_123() {} +function {$extension_name}_update__123() {} +function ($extension_name}__post_update_test() {} +function ($extension_name}_post_update() {} +END; + file_put_contents($staged_update_file, $code); + $this->assertStatusCheckResults([], $stage); - $active_dir = $this->container->get(PathLocator::class)->getProjectRoot(); - foreach ($this->extensions as $name => $path) { - unlink("$active_dir/$path/$name.$suffix"); - } + // If the update file is deleted from the stage, the validator should not + // detect any database updates. + unlink($staged_update_file); + $this->assertStatusCheckResults([], $stage); - $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.')); - $this->assertStatusCheckResults([$result], $stage); + // If the update file doesn't exist in the active directory, but does exist + // in the stage with a legitimate schema update or post-update function, the + // validator should detect it. + $project_root = $this->container->get(PathLocator::class) + ->getProjectRoot(); + unlink($project_root . '/' . $relative_file_path); + file_put_contents($staged_update_file, "<?php\nfunction $update_function_name() {}"); + $this->assertStatusCheckResults($expected_results, $stage); } /** diff --git a/src/Validator/StagedDatabaseUpdateValidator.php b/src/Validator/StagedDatabaseUpdateValidator.php index a2998417dd55e42bada1155c4dabfb999160652c..e04da6c289907277f7cc42b911be7f2064df30f6 100644 --- a/src/Validator/StagedDatabaseUpdateValidator.php +++ b/src/Validator/StagedDatabaseUpdateValidator.php @@ -46,7 +46,7 @@ final class StagedDatabaseUpdateValidator implements EventSubscriberInterface { $invalid_extensions = $this->stagedDBUpdateValidator->getExtensionsWithDatabaseUpdates($stage->getStageDirectory()); if ($invalid_extensions) { $invalid_extensions = array_map($this->t(...), $invalid_extensions); - $event->addError($invalid_extensions, $this->t('The update cannot proceed because possible database updates have been detected in the following extensions.')); + $event->addError($invalid_extensions, $this->t('The update cannot proceed because database updates have been detected in the following extensions.')); } } diff --git a/tests/src/Functional/StagedDatabaseUpdateTest.php b/tests/src/Functional/StagedDatabaseUpdateTest.php index 4d5fcbc5e0c32fd44c0a43f5da275a6228f521f2..e34c9f53e7b6718ffb829ad3b40041305ed2f2a2 100644 --- a/tests/src/Functional/StagedDatabaseUpdateTest.php +++ b/tests/src/Functional/StagedDatabaseUpdateTest.php @@ -83,7 +83,7 @@ class StagedDatabaseUpdateTest extends UpdaterFormTestBase { // Ensure that a list of pending database updates is visible, along with a // short explanation, in the warning messages. - $possible_update_message = 'Possible database updates have been detected in the following extensions.<ul><li>System</li><li>Automatic Updates Theme With Updates</li></ul>'; + $possible_update_message = 'Database updates have been detected in the following extensions.<ul><li>System</li><li>Automatic Updates Theme With Updates</li></ul>'; $warning_messages = $assert_session->elementExists('css', 'div[data-drupal-messages] div[aria-label="Warning message"]'); $this->assertStringContainsString($possible_update_message, $warning_messages->getHtml()); if ($maintenance_mode_on === TRUE) { diff --git a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php index 7ca9e175cc967188d00e919f45d98e5cc00cf155..b435fb2badc9ecd02b7ad9d68bbebc6dc4bba00e 100644 --- a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php +++ b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php @@ -6,7 +6,6 @@ namespace Drupal\Tests\automatic_updates\Kernel\StatusCheck; use Drupal\Core\Logger\RfcLogLevel; use Drupal\package_manager\Event\PreApplyEvent; -use Drupal\package_manager\PathLocator; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; use ColinODell\PsrTestLogger\TestLogger; @@ -23,150 +22,25 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase { protected static $modules = ['automatic_updates']; /** - * The extensions that will be used in this test. - * - * System and Stark are installed, so they are used to test what happens when - * database updates are detected in installed extensions. Views and Olivero - * are not installed by this test, so they are used to test what happens when - * uninstalled extensions have database updates. - * - * @var string[] - * - * @see ::setUp() + * Tests that unattended updates are stopped by staged database updates. */ - private $extensions = [ - 'system' => 'core/modules/system', - 'views' => 'core/modules/views', - 'stark' => 'core/themes/stark', - 'olivero' => 'core/themes/olivero', - ]; - - /** - * The test logger to collected messages logged by the cron update stage. - * - * @var \Psr\Log\Test\TestLogger - */ - private $logger; - - /** - * {@inheritdoc} - */ - protected function setUp(): void { - parent::setUp(); - - $this->container->get('theme_installer')->install(['stark']); - $this->assertFalse($this->container->get('module_handler')->moduleExists('views')); - $this->assertFalse($this->container->get('theme_handler')->themeExists('olivero')); - - // Ensure that all the extensions we're testing with have database update - // files in the active directory. - $active_dir = $this->container->get(PathLocator::class)->getProjectRoot(); - - foreach ($this->extensions as $extension_name => $extension_path) { - $extension_path = $active_dir . '/' . $extension_path; - mkdir($extension_path, 0777, TRUE); - - foreach ($this->providerSuffixes() as [$suffix]) { - touch("$extension_path/$extension_name.$suffix"); - } - } - - $this->logger = new TestLogger(); + public function testStagedDatabaseUpdateExists(): void { + $logger = new TestLogger(); $this->container->get('logger.channel.automatic_updates') - ->addLogger($this->logger); - } - - /** - * Data provider for several test methods. - * - * @return \string[][] - * The test cases. - */ - public function providerSuffixes(): array { - return [ - 'hook_update_N' => ['install'], - 'hook_post_update_NAME' => ['post_update.php'], - ]; - } + ->addLogger($logger); - /** - * Tests that no errors are raised if the stage has no DB updates. - */ - public function testNoUpdates(): void { - $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1'); - $this->container->get('cron')->run(); - $this->assertFalse($this->logger->hasRecords((string) RfcLogLevel::ERROR)); - } - - /** - * Tests that an error is raised if DB update files are removed in the stage. - * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). - * - * @dataProvider providerSuffixes - */ - public function testFileDeleted(string $suffix): void { - $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1'); - $listener = function (PreApplyEvent $event) use ($suffix): void { - $stage_dir = $event->stage->getStageDirectory(); - foreach ($this->extensions as $name => $path) { - unlink("$stage_dir/$path/$name.$suffix"); - } - }; - $this->addEventTestListener($listener); - - $this->container->get('cron')->run(); - $expected_message = "The update cannot proceed because possible database updates have been detected in the following extensions.\nSystem\nStark\n"; - $this->assertTrue($this->logger->hasRecord($expected_message, (string) RfcLogLevel::ERROR)); - } - - /** - * Tests that an error is raised if DB update files are changed in the stage. - * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). - * - * @dataProvider providerSuffixes - */ - public function testFileChanged(string $suffix): void { - $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1'); - $listener = function (PreApplyEvent $event) use ($suffix): void { - $stage_dir = $event->stage->getStageDirectory(); - foreach ($this->extensions as $name => $path) { - file_put_contents("$stage_dir/$path/$name.$suffix", $this->randomString()); - } - }; - $this->addEventTestListener($listener); - - $this->container->get('cron')->run(); - $expected_message = "The update cannot proceed because possible database updates have been detected in the following extensions.\nSystem\nStark\n"; - $this->assertTrue($this->logger->hasRecord($expected_message, (string) RfcLogLevel::ERROR)); - } - - /** - * Tests that an error is raised if DB update files are added in the stage. - * - * @param string $suffix - * The update file suffix to test (one of `install` or `post_update.php`). - * - * @dataProvider providerSuffixes - */ - public function testFileAdded(string $suffix): void { $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1'); - $listener = function () use ($suffix): void { - $active_dir = $this->container->get(PathLocator::class) - ->getProjectRoot(); - foreach ($this->extensions as $name => $path) { - unlink("$active_dir/$path/$name.$suffix"); - } + $listener = function (PreApplyEvent $event): void { + $dir = $event->stage->getStageDirectory() . '/core/modules/system'; + mkdir($dir, 0777, TRUE); + file_put_contents($dir . '/system.install', "<?php\nfunction system_update_10101010() {}"); }; $this->addEventTestListener($listener); $this->container->get('cron')->run(); - $expected_message = "The update cannot proceed because possible database updates have been detected in the following extensions.\nSystem\nStark\n"; - $this->assertTrue($this->logger->hasRecord($expected_message, (string) RfcLogLevel::ERROR)); + $expected_message = "The update cannot proceed because database updates have been detected in the following extensions.\nSystem\n"; + $this->assertTrue($logger->hasRecord($expected_message, (string) RfcLogLevel::ERROR)); } }