From 0011d522c191c02b48906bbb0750791727d6d09f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Wed, 21 Jun 2023 13:50:26 -0400
Subject: [PATCH 01/30] Use primitive static analysis
---
.../src/Validator/StagedDBUpdateValidator.php | 28 +++++++++++++------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 1df91013c3..1ec5b3ff88 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -90,8 +90,8 @@ 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_hashes = $this->getUpdateFunctions($active_dir, $extension);
+ $staged_hashes = $this->getUpdateFunctions($stage_dir, $extension);
return $active_hashes !== $staged_hashes;
}
@@ -108,22 +108,34 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* 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.
*/
- 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 = [];
+ $functions = [];
- foreach (['.install', '.post_update.php'] as $suffix) {
+ $patterns = [
+ '.install' => '/^\s*function\s+' . $name . '_update_[0-9]+\s*\(/',
+ '.post_update.php' => '/^\s*function\s+' . $name . '_post_update_.+\s*\(/',
+ ];
+ foreach ($patterns as $suffix => $pattern) {
$file = $path . $suffix;
if (file_exists($file)) {
- $hashes[$suffix] = hash_file('sha256', $file);
+ $code = file_get_contents($file);
+
+ $matches = [];
+ assert(preg_match($pattern, $code, $matches) !== FALSE);
+ assert(natcasesort($matches[0]));
+
+ $functions[$suffix] = $matches[0];
}
}
- return $hashes;
+ return $functions;
}
/**
--
GitLab
From f0ae625edb9933457d2dd56d34ff3caa3c15af35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Wed, 21 Jun 2023 15:35:08 -0400
Subject: [PATCH 02/30] Better null handling
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 1ec5b3ff88..1433a0659c 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -129,10 +129,10 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$code = file_get_contents($file);
$matches = [];
- assert(preg_match($pattern, $code, $matches) !== FALSE);
- assert(natcasesort($matches[0]));
-
- $functions[$suffix] = $matches[0];
+ if (preg_match($pattern, $code, $matches)) {
+ assert(natcasesort($matches[0]));
+ }
+ $functions[$suffix] = $matches[0] ?? [];
}
}
return $functions;
--
GitLab
From e64201acd450e07bbac76704d4d9e66fb6d3fda6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Wed, 21 Jun 2023 16:16:45 -0400
Subject: [PATCH 03/30] Fix final failures
---
.../src/Validator/StagedDBUpdateValidator.php | 11 ++++++-----
.../tests/src/Kernel/StagedDBUpdateValidatorTest.php | 12 +++++++++++-
.../StagedDatabaseUpdateValidatorTest.php | 7 ++++++-
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 1433a0659c..72dcdf91cf 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -97,16 +97,17 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
}
/**
- * 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.
* @param \Drupal\Core\Extension\Extension $extension
* 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.
+ * @return array<string, string[]>
+ * The names of the update functions in the module's .install and
+ * .post_update.php files, in that order, if they exist. The array will be
+ * keyed by file extension.
*/
protected function getUpdateFunctions(string $root_dir, Extension $extension): array {
$name = $extension->getName();
@@ -129,7 +130,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$code = file_get_contents($file);
$matches = [];
- if (preg_match($pattern, $code, $matches)) {
+ if (preg_match_all($pattern, $code, $matches, PREG_PATTERN_ORDER)) {
assert(natcasesort($matches[0]));
}
$functions[$suffix] = $matches[0] ?? [];
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 052cb321fb..c2f5ecae54 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -114,7 +114,17 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$stage_dir = $stage->getStageDirectory();
foreach ($this->extensions as $name => $path) {
- file_put_contents("$stage_dir/$path/$name.$suffix", $this->randomString());
+ $function_name = match ($suffix) {
+ 'install' => $name . '_update_' . PHP_INT_MAX,
+ 'post_update.php' => $name . '_post_update_test_package_manager',
+ };
+ // If the file already exists, add this fake, empty update hook to it,
+ // along with an extra function with a random name, to ensure that we
+ // are only detecting functions that are named properly. Also add extra
+ // whitespace to ensure that we can find update functions even in oddly
+ // formatted files.
+ $extra_function = $name . '_' . $this->randomMachineName();
+ file_put_contents("$stage_dir/$path/$name.$suffix", "\n function $function_name( ) {}\n\nfunction $extra_function ()", FILE_APPEND);
}
$result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
diff --git a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
index 7ca9e175cc..f38cc0d8ab 100644
--- a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
+++ b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
@@ -134,7 +134,12 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
$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());
+ $function_name = match ($suffix) {
+ 'install' => $name . '_update_' . PHP_INT_MAX,
+ 'post_update.php' => $name . '_post_update_test_package_manager',
+ };
+ // If the file already exists, add this fake, empty update hook to it.
+ file_put_contents("$stage_dir/$path/$name.$suffix", "\nfunction $function_name() {}", FILE_APPEND);
}
};
$this->addEventTestListener($listener);
--
GitLab
From 0024908ab0d3e057e5001718aad522a25e129e04 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Wed, 21 Jun 2023 16:23:20 -0400
Subject: [PATCH 04/30] Case insensitive
---
.../src/Validator/StagedDBUpdateValidator.php | 4 ++--
.../tests/src/Kernel/StagedDBUpdateValidatorTest.php | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 72dcdf91cf..770d88e59b 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -120,8 +120,8 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$functions = [];
$patterns = [
- '.install' => '/^\s*function\s+' . $name . '_update_[0-9]+\s*\(/',
- '.post_update.php' => '/^\s*function\s+' . $name . '_post_update_.+\s*\(/',
+ '.install' => '/^\s*function\s+' . $name . '_update_[0-9]+\s*\(/i',
+ '.post_update.php' => '/^\s*function\s+' . $name . '_post_update_.+\s*\(/i',
];
foreach ($patterns as $suffix => $pattern) {
$file = $path . $suffix;
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index c2f5ecae54..85086ced53 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -118,13 +118,13 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
'install' => $name . '_update_' . PHP_INT_MAX,
'post_update.php' => $name . '_post_update_test_package_manager',
};
- // If the file already exists, add this fake, empty update hook to it,
- // along with an extra function with a random name, to ensure that we
- // are only detecting functions that are named properly. Also add extra
- // whitespace to ensure that we can find update functions even in oddly
- // formatted files.
+ // If the file already exists, add a fake, empty update hook to it, along
+ // with an extra function with a random name, to ensure that we are only
+ // detecting functions that are named properly. Also add extra whitespace
+ // and strange casing to ensure that we can even find update functions in
+ // oddly formatted files.
$extra_function = $name . '_' . $this->randomMachineName();
- file_put_contents("$stage_dir/$path/$name.$suffix", "\n function $function_name( ) {}\n\nfunction $extra_function ()", FILE_APPEND);
+ file_put_contents("$stage_dir/$path/$name.$suffix", "\n Function $function_name( ) {}\n\nfunction $extra_function ()", FILE_APPEND);
}
$result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
--
GitLab
From a87b9c111e0264b05bd6c37cff0708a02bc766bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 09:43:43 -0400
Subject: [PATCH 05/30] See if token_get_all works as expected
---
.../src/Validator/StagedDBUpdateValidator.php | 30 ++++++++++++++-----
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 770d88e59b..d3b6b62e10 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -120,20 +120,36 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$functions = [];
$patterns = [
- '.install' => '/^\s*function\s+' . $name . '_update_[0-9]+\s*\(/i',
- '.post_update.php' => '/^\s*function\s+' . $name . '_post_update_.+\s*\(/i',
+ '.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)) {
$code = file_get_contents($file);
-
- $matches = [];
- if (preg_match_all($pattern, $code, $matches, PREG_PATTERN_ORDER)) {
- assert(natcasesort($matches[0]));
+ $tokens = token_get_all($code);
+
+ // Scan for a specific sequence: T_FUNCTION, T_WHITESPACE, T_STRING --
+ // this indicates a function declaration.
+ for ($i = 0; $i < count($tokens); $i++) {
+ if (is_array($tokens[$i]) && token_name($tokens[$i][0]) === 'T_FUNCTION') {
+ // We found a function! The next token MUST be whitespace for this
+ // to be valid PHP code, so skip over it.
+ $i += 2;
+ assert(token_name($tokens[$i][0]) === 'T_STRING');
+
+ // If the function name matches the pattern, add it to the list.
+ $name = $tokens[$i][1];
+ if (preg_match($pattern, $name)) {
+ $functions[$suffix][] = $name;
+ }
+ }
+ }
+ // Sort the list of functions to ensure they are compared consistently.
+ if (array_key_exists($suffix, $functions)) {
+ natcasesort($functions[$suffix]);
}
- $functions[$suffix] = $matches[0] ?? [];
}
}
return $functions;
--
GitLab
From 4574378bfbdd385dde5dcff4699b30f441cdc91d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 10:24:30 -0400
Subject: [PATCH 06/30] Add a helper to the test for stronger coverage
---
.../Kernel/StagedDBUpdateValidatorTest.php | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 85086ced53..48b84f6d24 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -57,6 +57,44 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
}
}
+ /**
+ * Adds database update and post-update functions to a file.
+ *
+ * @param string $extension_dir
+ * The full path of the extension to which the updates should be added.
+ * @param string $file_extension
+ * The extension of the update file. Can be either `.install` or
+ * `.post_update.php`.
+ */
+ private function addUpdatesToFile(string $extension_dir, string $file_extension): void {
+ static $update_number = 1001;
+
+ $this->assertDirectoryIsWritable($extension_dir);
+ $extension_name = basename($extension_dir);
+ $file = $extension_dir . '/' . $extension_name . '.' . $file_extension;
+
+ // Ensure the file exists and has the PHP opening tag, or we won't be able
+ // to parse it.
+ if (!file_exists($file)) {
+ file_put_contents($file, "<?php\n");
+ }
+
+ // Add two functions to the file: an empty update hook with a legitimate
+ // name -- the given $file_extension determines whether it's a database
+ // update or post-update hook -- and a randomly-named helper function, to
+ // ensure we are only considering "real" update hooks.
+ $random = $this->randomMachineName();
+ $update_function_name = match ($file_extension) {
+ '.install' => sprintf('%s_update_%d', $extension_name, $update_number++),
+ '.post_update.php' => sprintf('%s_post_update_%s', $extension_name, $random),
+ };
+ $code = <<<END
+function $update_function_name() {}
+function _{$extension_name}_{$random}() {}
+END;
+ file_put_contents($file, $code, FILE_APPEND);
+ }
+
/**
* Data provider for several test methods.
*
--
GitLab
From 54fbf787f5608750b9cdba8760fa15f0e909f67e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 10:41:06 -0400
Subject: [PATCH 07/30] Use a data provider
---
.../Kernel/StagedDBUpdateValidatorTest.php | 135 +++++++++++-------
1 file changed, 82 insertions(+), 53 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 48b84f6d24..92bf6e8127 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -52,7 +52,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
mkdir($extension_path, 0777, TRUE);
foreach ($this->providerSuffixes() as [$suffix]) {
- touch("$extension_path/$extension_name.$suffix");
+ file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
}
}
}
@@ -69,15 +69,9 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
private function addUpdatesToFile(string $extension_dir, string $file_extension): void {
static $update_number = 1001;
- $this->assertDirectoryIsWritable($extension_dir);
$extension_name = basename($extension_dir);
- $file = $extension_dir . '/' . $extension_name . '.' . $file_extension;
-
- // Ensure the file exists and has the PHP opening tag, or we won't be able
- // to parse it.
- if (!file_exists($file)) {
- file_put_contents($file, "<?php\n");
- }
+ $file = $extension_dir . '/' . $extension_name . $file_extension;
+ $this->assertFileIsWritable($file);
// Add two functions to the file: an empty update hook with a legitimate
// name -- the given $file_extension determines whether it's a database
@@ -117,73 +111,108 @@ END;
$this->assertStatusCheckResults([], $stage);
}
- /**
- * Tests that a warning 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 {
- $stage = $this->createStage();
- $stage->create();
+ public function providerStagedDatabaseUpdate(): array {
+ $summary = t('Possible database updates have been detected in the following extensions.');
- $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);
+ return [
+ 'schema update in installed module' => [
+ 'system',
+ '.install',
+ [
+ ValidationResult::createWarning([
+ t('System'),
+ ], $summary)
+ ],
+ ],
+ 'post-update in installed module' => [
+ 'system',
+ '.post_update.php',
+ [
+ ValidationResult::createWarning([
+ t('System'),
+ ], $summary)
+ ],
+ ],
+ 'schema update in installed theme' => [
+ 'stark',
+ '.install',
+ [
+ ValidationResult::createWarning([
+ t('Stark'),
+ ], $summary)
+ ],
+ ],
+ 'post-update in installed theme' => [
+ 'stark',
+ '.post_update.php',
+ [
+ ValidationResult::createWarning([
+ t('Stark'),
+ ], $summary)
+ ],
+ ],
+ 'schema update in non-installed module' => [
+ 'views',
+ '.install',
+ [],
+ ],
+ 'post-update in non-installed module' => [
+ 'views',
+ '.post_update.php',
+ [],
+ ],
+ 'schema update in non-installed theme' => [
+ 'olivero',
+ '.install',
+ [],
+ ],
+ 'post-update in non-installed theme' => [
+ 'olivero',
+ '.post_update.php',
+ [],
+ ],
+ ];
}
/**
- * Tests that a warning is raised if DB update files are changed 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_name
+ * The name of the extension that should have database updates. Must be
+ * a key in $this->extensions.
+ * @param string $file_extension
+ * The extension of the update file, including 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 testFileChanged(string $suffix): void {
+ public function testStagedDatabaseUpdate(string $extension_name, string $file_extension, array $expected_results): void {
+ $this->assertArrayHasKey($extension_name, $this->extensions);
+
$stage = $this->createStage();
$stage->create();
- $stage_dir = $stage->getStageDirectory();
- foreach ($this->extensions as $name => $path) {
- $function_name = match ($suffix) {
- 'install' => $name . '_update_' . PHP_INT_MAX,
- 'post_update.php' => $name . '_post_update_test_package_manager',
- };
- // If the file already exists, add a fake, empty update hook to it, along
- // with an extra function with a random name, to ensure that we are only
- // detecting functions that are named properly. Also add extra whitespace
- // and strange casing to ensure that we can even find update functions in
- // oddly formatted files.
- $extra_function = $name . '_' . $this->randomMachineName();
- file_put_contents("$stage_dir/$path/$name.$suffix", "\n Function $function_name( ) {}\n\nfunction $extra_function ()", FILE_APPEND);
- }
-
- $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
- $this->assertStatusCheckResults([$result], $stage);
+ $this->addUpdatesToFile($stage->getStageDirectory() . '/' . $this->extensions[$extension_name], $file_extension);
+ $this->assertStatusCheckResults($expected_results, $stage);
}
/**
- * Tests that a warning is raised if DB update files are added in the stage.
+ * Tests that a warning 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 testFileAdded(string $suffix): void {
+ public function testFileDeleted(string $suffix): void {
$stage = $this->createStage();
$stage->create();
- $active_dir = $this->container->get(PathLocator::class)->getProjectRoot();
+ $stage_dir = $stage->getStageDirectory();
foreach ($this->extensions as $name => $path) {
- unlink("$active_dir/$path/$name.$suffix");
+ unlink("$stage_dir/$path/$name.$suffix");
}
$result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
--
GitLab
From 23589150a32ed20d5371dad38403a0034fa7e393 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 11:26:06 -0400
Subject: [PATCH 08/30] Make test more correct
---
.../Kernel/StagedDBUpdateValidatorTest.php | 72 +++++++------------
1 file changed, 27 insertions(+), 45 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 92bf6e8127..c67d576744 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -57,38 +57,6 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
}
}
- /**
- * Adds database update and post-update functions to a file.
- *
- * @param string $extension_dir
- * The full path of the extension to which the updates should be added.
- * @param string $file_extension
- * The extension of the update file. Can be either `.install` or
- * `.post_update.php`.
- */
- private function addUpdatesToFile(string $extension_dir, string $file_extension): void {
- static $update_number = 1001;
-
- $extension_name = basename($extension_dir);
- $file = $extension_dir . '/' . $extension_name . $file_extension;
- $this->assertFileIsWritable($file);
-
- // Add two functions to the file: an empty update hook with a legitimate
- // name -- the given $file_extension determines whether it's a database
- // update or post-update hook -- and a randomly-named helper function, to
- // ensure we are only considering "real" update hooks.
- $random = $this->randomMachineName();
- $update_function_name = match ($file_extension) {
- '.install' => sprintf('%s_update_%d', $extension_name, $update_number++),
- '.post_update.php' => sprintf('%s_post_update_%s', $extension_name, $random),
- };
- $code = <<<END
-function $update_function_name() {}
-function _{$extension_name}_{$random}() {}
-END;
- file_put_contents($file, $code, FILE_APPEND);
- }
-
/**
* Data provider for several test methods.
*
@@ -117,7 +85,7 @@ END;
return [
'schema update in installed module' => [
'system',
- '.install',
+ 'install',
[
ValidationResult::createWarning([
t('System'),
@@ -126,7 +94,7 @@ END;
],
'post-update in installed module' => [
'system',
- '.post_update.php',
+ 'post_update.php',
[
ValidationResult::createWarning([
t('System'),
@@ -135,7 +103,7 @@ END;
],
'schema update in installed theme' => [
'stark',
- '.install',
+ 'install',
[
ValidationResult::createWarning([
t('Stark'),
@@ -144,7 +112,7 @@ END;
],
'post-update in installed theme' => [
'stark',
- '.post_update.php',
+ 'post_update.php',
[
ValidationResult::createWarning([
t('Stark'),
@@ -153,22 +121,22 @@ END;
],
'schema update in non-installed module' => [
'views',
- '.install',
+ 'install',
[],
],
'post-update in non-installed module' => [
'views',
- '.post_update.php',
+ 'post_update.php',
[],
],
'schema update in non-installed theme' => [
'olivero',
- '.install',
+ 'install',
[],
],
'post-update in non-installed theme' => [
'olivero',
- '.post_update.php',
+ 'post_update.php',
[],
],
];
@@ -181,20 +149,34 @@ END;
* The name of the extension that should have database updates. Must be
* a key in $this->extensions.
* @param string $file_extension
- * The extension of the update file, including the leading period. Must be
- * either `.install` or `.post_update.php`.
+ * 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 providerStagedDatabaseUpdate
*/
public function testStagedDatabaseUpdate(string $extension_name, string $file_extension, array $expected_results): void {
- $this->assertArrayHasKey($extension_name, $this->extensions);
-
$stage = $this->createStage();
$stage->create();
- $this->addUpdatesToFile($stage->getStageDirectory() . '/' . $this->extensions[$extension_name], $file_extension);
+ $file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $this->extensions[$extension_name], $extension_name, $file_extension);
+ $this->assertFileIsWritable($file);
+
+ // Add a helper function with a random name, to ensure that it's ignored by
+ // the validator.
+ $function_name = sprintf('_%s_%s', $extension_name, $this->randomMachineName());
+ file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
+ $this->assertStatusCheckResults([], $stage);
+
+ // 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.
+ $function_name = match ($file_extension) {
+ 'install' => $extension_name . '_update_1001',
+ 'post_update.php' => $extension_name . '_post_update_' . $this->randomMachineName(),
+ };
+ file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
$this->assertStatusCheckResults($expected_results, $stage);
}
--
GitLab
From f4cdb4ec5a14e67943c43f828b68c4b348aa47d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 11:32:22 -0400
Subject: [PATCH 09/30] Improve coverage again
---
.../Kernel/StagedDBUpdateValidatorTest.php | 48 ++++++-------------
1 file changed, 15 insertions(+), 33 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index c67d576744..c578b8d271 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -51,25 +51,12 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$extension_path = $active_dir . '/' . $extension_path;
mkdir($extension_path, 0777, TRUE);
- foreach ($this->providerSuffixes() as [$suffix]) {
+ foreach (['install', 'post_update.php'] as $suffix) {
file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
}
}
}
- /**
- * 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'],
- ];
- }
-
/**
* Tests that no errors are raised if the stage has no DB updates.
*/
@@ -79,6 +66,12 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$this->assertStatusCheckResults([], $stage);
}
+ /**
+ * Data provider for ::testStagedDatabaseUpdates().
+ *
+ * @return array[]
+ * The test cases.
+ */
public function providerStagedDatabaseUpdate(): array {
$summary = t('Possible database updates have been detected in the following extensions.');
@@ -178,27 +171,16 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
};
file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
$this->assertStatusCheckResults($expected_results, $stage);
- }
- /**
- * Tests that a warning 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 {
- $stage = $this->createStage();
- $stage->create();
-
- $stage_dir = $stage->getStageDirectory();
- foreach ($this->extensions as $name => $path) {
- unlink("$stage_dir/$path/$name.$suffix");
- }
+ // If previous updates are removed, the validator should not detect any
+ // database updates.
+ file_put_contents($file, "<?php\nfunction previous_updates_removed() {}");
+ $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 is deleted from the stage, the validator should not
+ // detect any database updates.
+ unlink($file);
+ $this->assertStatusCheckResults([], $stage);
}
/**
--
GitLab
From 91cfd93f4e197f90bc4b5249b71e4a9b8f609237 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 11:39:31 -0400
Subject: [PATCH 10/30] More simplification
---
.../Kernel/StagedDBUpdateValidatorTest.php | 74 ++++++++-----------
1 file changed, 32 insertions(+), 42 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index c578b8d271..0788110fc9 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,9 +28,20 @@ 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);
+ $extension_name = basename($extension_path);
foreach (['install', 'post_update.php'] as $suffix) {
file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
@@ -57,15 +49,6 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
}
}
- /**
- * 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);
- }
-
/**
* Data provider for ::testStagedDatabaseUpdates().
*
@@ -77,7 +60,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
return [
'schema update in installed module' => [
- 'system',
+ 'core/modules/system',
'install',
[
ValidationResult::createWarning([
@@ -86,7 +69,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
],
],
'post-update in installed module' => [
- 'system',
+ 'core/modules/system',
'post_update.php',
[
ValidationResult::createWarning([
@@ -95,7 +78,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
],
],
'schema update in installed theme' => [
- 'stark',
+ 'core/themes/stark',
'install',
[
ValidationResult::createWarning([
@@ -104,7 +87,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
],
],
'post-update in installed theme' => [
- 'stark',
+ 'core/themes/stark',
'post_update.php',
[
ValidationResult::createWarning([
@@ -112,23 +95,25 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
], $summary)
],
],
+ // The validator should ignore changes in any extensions that aren't
+ // installed.
'schema update in non-installed module' => [
- 'views',
+ 'core/modules/views',
'install',
[],
],
'post-update in non-installed module' => [
- 'views',
+ 'core/modules/views',
'post_update.php',
[],
],
'schema update in non-installed theme' => [
- 'olivero',
+ 'core/themes/olivero',
'install',
[],
],
'post-update in non-installed theme' => [
- 'olivero',
+ 'core/themes/olivero',
'post_update.php',
[],
],
@@ -138,9 +123,9 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
/**
* Tests validation of staged database updates.
*
- * @param string $extension_name
- * The name of the extension that should have database updates. Must be
- * a key in $this->extensions.
+ * @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`.
@@ -149,11 +134,16 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
*
* @dataProvider providerStagedDatabaseUpdate
*/
- public function testStagedDatabaseUpdate(string $extension_name, string $file_extension, array $expected_results): void {
+ public function testStagedDatabaseUpdate(string $extension_dir, string $file_extension, array $expected_results): void {
$stage = $this->createStage();
$stage->create();
- $file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $this->extensions[$extension_name], $extension_name, $file_extension);
+ // Nothing has been changed in the stage, so ensure the validator doesn't
+ // detect any changes.
+ $this->assertStatusCheckResults([], $stage);
+
+ $extension_name = basename($extension_dir);
+ $file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $extension_dir, $extension_name, $file_extension);
$this->assertFileIsWritable($file);
// Add a helper function with a random name, to ensure that it's ignored by
--
GitLab
From 56b67a4a2dae96bc648066eb24c38ae2d21eba1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 11:50:07 -0400
Subject: [PATCH 11/30] argh coding standards
---
.../src/Validator/StagedDBUpdateValidator.php | 2 +-
.../tests/src/Kernel/StagedDBUpdateValidatorTest.php | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index d3b6b62e10..7d6c0b2e27 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -137,8 +137,8 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
// We found a function! The next token MUST be whitespace for this
// to be valid PHP code, so skip over it.
$i += 2;
+ // We should be looking at a string identifier (the function name).
assert(token_name($tokens[$i][0]) === 'T_STRING');
-
// If the function name matches the pattern, add it to the list.
$name = $tokens[$i][1];
if (preg_match($pattern, $name)) {
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 0788110fc9..e845a467dc 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -65,7 +65,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
[
ValidationResult::createWarning([
t('System'),
- ], $summary)
+ ], $summary),
],
],
'post-update in installed module' => [
@@ -74,7 +74,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
[
ValidationResult::createWarning([
t('System'),
- ], $summary)
+ ], $summary),
],
],
'schema update in installed theme' => [
@@ -83,7 +83,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
[
ValidationResult::createWarning([
t('Stark'),
- ], $summary)
+ ], $summary),
],
],
'post-update in installed theme' => [
@@ -92,7 +92,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
[
ValidationResult::createWarning([
t('Stark'),
- ], $summary)
+ ], $summary),
],
],
// The validator should ignore changes in any extensions that aren't
@@ -148,7 +148,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
// Add a helper function with a random name, to ensure that it's ignored by
// the validator.
- $function_name = sprintf('_%s_%s', $extension_name, $this->randomMachineName());
+ $function_name = '_' . $extension_name . '_' . $this->randomMachineName();
file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
$this->assertStatusCheckResults([], $stage);
--
GitLab
From 61894b190245bff98f9ebad7ce017698fb34d55a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 12:51:10 -0400
Subject: [PATCH 12/30] Fix parser trouble and handle anonymous functions
---
.../src/Validator/StagedDBUpdateValidator.php | 59 ++++++++++++-------
.../Kernel/StagedDBUpdateValidatorTest.php | 6 +-
2 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 7d6c0b2e27..2d490d401c 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -119,6 +119,23 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
]);
$functions = [];
+ // A named function declaration will always be a T_FUNCTION (the word
+ // `function`), followed by T_WHITESPACE (or the code would be syntactially
+ // 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).
+ $is_named_function = function (array $tokens): bool {
+ return (
+ count($tokens) === 3 &&
+ is_array($tokens[0]) &&
+ is_array($tokens[1]) &&
+ is_array($tokens[2]) &&
+ token_name($tokens[0][0]) === 'T_FUNCTION' &&
+ token_name($tokens[1][0]) === 'T_WHITESPACE' &&
+ token_name($tokens[2][0]) === 'T_STRING'
+ );
+ };
+
$patterns = [
'.install' => '/^' . $name . '_update_[0-9]+$/i',
'.post_update.php' => '/^' . $name . '_post_update_.+$/i',
@@ -126,30 +143,28 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
foreach ($patterns as $suffix => $pattern) {
$file = $path . $suffix;
- if (file_exists($file)) {
- $code = file_get_contents($file);
- $tokens = token_get_all($code);
-
- // Scan for a specific sequence: T_FUNCTION, T_WHITESPACE, T_STRING --
- // this indicates a function declaration.
- for ($i = 0; $i < count($tokens); $i++) {
- if (is_array($tokens[$i]) && token_name($tokens[$i][0]) === 'T_FUNCTION') {
- // We found a function! The next token MUST be whitespace for this
- // to be valid PHP code, so skip over it.
- $i += 2;
- // We should be looking at a string identifier (the function name).
- assert(token_name($tokens[$i][0]) === 'T_STRING');
- // If the function name matches the pattern, add it to the list.
- $name = $tokens[$i][1];
- if (preg_match($pattern, $name)) {
- $functions[$suffix][] = $name;
- }
+ 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 ($is_named_function($chunk)) {
+ $name = $chunk[2][1];
+ if (preg_match($pattern, $name)) {
+ $functions[$suffix][] = $name;
}
+ // We already know what the next three tokens are, so we can skip
+ // over them.
+ $i += 3;
}
- // Sort the list of functions to ensure they are compared consistently.
- if (array_key_exists($suffix, $functions)) {
- natcasesort($functions[$suffix]);
- }
+ }
+ // Sort the list of functions to ensure they are compared consistently.
+ if (array_key_exists($suffix, $functions)) {
+ natcasesort($functions[$suffix]);
}
}
return $functions;
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index e845a467dc..f32e7c5166 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -146,10 +146,10 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $extension_dir, $extension_name, $file_extension);
$this->assertFileIsWritable($file);
- // Add a helper function with a random name, to ensure that it's ignored by
- // the validator.
+ // Add a helper function with a random name, as well as an anonymous
+ // function, to ensure they are ignored by the validator.
$function_name = '_' . $extension_name . '_' . $this->randomMachineName();
- file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
+ file_put_contents($file, "function $function_name() { \$foo = function () {}; }\n", FILE_APPEND);
$this->assertStatusCheckResults([], $stage);
// Now add a "real" update function -- either a schema update or a
--
GitLab
From 0ade1cad0f48ad68921f39d28deaac1433a5a006 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 13:16:00 -0400
Subject: [PATCH 13/30] Partial fixes
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 2 +-
.../StatusCheck/StagedDatabaseUpdateValidatorTest.php | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 2d490d401c..b0cf86d46e 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -120,7 +120,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$functions = [];
// A named function declaration will always be a T_FUNCTION (the word
- // `function`), followed by T_WHITESPACE (or the code would be syntactially
+ // `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).
diff --git a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
index f38cc0d8ab..16932593fc 100644
--- a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
+++ b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
@@ -67,7 +67,7 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
mkdir($extension_path, 0777, TRUE);
foreach ($this->providerSuffixes() as [$suffix]) {
- touch("$extension_path/$extension_name.$suffix");
+ file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
}
}
@@ -99,7 +99,7 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
}
/**
- * Tests that an error is raised if DB update files are removed in the stage.
+ * Tests that no 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`).
@@ -117,8 +117,7 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
$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));
+ $this->assertFalse($this->logger->hasRecords(RfcLogLevel::ERROR));
}
/**
--
GitLab
From 2fb93a0b6b6b6acf9ec8a3e3539c877e87fbdbc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 13:26:53 -0400
Subject: [PATCH 14/30] Hollow out StagedDatabaseUpdateValidatorTest
---
.../StagedDatabaseUpdateValidatorTest.php | 150 ++----------------
1 file changed, 10 insertions(+), 140 deletions(-)
diff --git a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
index 16932593fc..09d72af5f1 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,154 +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]) {
- file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
- }
- }
-
- $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 no 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();
- $this->assertFalse($this->logger->hasRecords(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) {
- $function_name = match ($suffix) {
- 'install' => $name . '_update_' . PHP_INT_MAX,
- 'post_update.php' => $name . '_post_update_test_package_manager',
- };
- // If the file already exists, add this fake, empty update hook to it.
- file_put_contents("$stage_dir/$path/$name.$suffix", "\nfunction $function_name() {}", FILE_APPEND);
- }
- };
- $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 possible database updates have been detected in the following extensions.\nSystem\n";
+ $this->assertTrue($logger->hasRecord($expected_message, (string) RfcLogLevel::ERROR));
}
}
--
GitLab
From 0ce9d19be99bbe5480b537ab4e6a18e9f80dc213 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 13:31:35 -0400
Subject: [PATCH 15/30] Use Inspector in is_named_function
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index b0cf86d46e..a2b34acba2 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;
@@ -127,9 +128,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$is_named_function = function (array $tokens): bool {
return (
count($tokens) === 3 &&
- is_array($tokens[0]) &&
- is_array($tokens[1]) &&
- is_array($tokens[2]) &&
+ Inspector::assertAllStrictArrays($tokens) &&
token_name($tokens[0][0]) === 'T_FUNCTION' &&
token_name($tokens[1][0]) === 'T_WHITESPACE' &&
token_name($tokens[2][0]) === 'T_STRING'
--
GitLab
From 7a561318d42555012921f6befb3e92b7910c2d81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 14:06:45 -0400
Subject: [PATCH 16/30] Stoopid coding standards on 10.0.x
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index a2b34acba2..9c3bcde492 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -105,7 +105,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* @param \Drupal\Core\Extension\Extension $extension
* The module to check.
*
- * @return array<string, string[]>
+ * @return array<string,string[]>
* The names of the update functions in the module's .install and
* .post_update.php files, in that order, if they exist. The array will be
* keyed by file extension.
--
GitLab
From c9dcfaf5e4f20a5b29bfe2029a03b201c2a636de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 14:10:22 -0400
Subject: [PATCH 17/30] Boy I sure do hate these coding standards
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 9c3bcde492..03d4140359 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -105,7 +105,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* @param \Drupal\Core\Extension\Extension $extension
* The module to check.
*
- * @return array<string,string[]>
+ * @return array[]
* The names of the update functions in the module's .install and
* .post_update.php files, in that order, if they exist. The array will be
* keyed by file extension.
--
GitLab
From 1c41508902939abb4e479d866e9c833cab9a70b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 14:44:32 -0400
Subject: [PATCH 18/30] A few easy suggestions
---
.../src/Validator/StagedDBUpdateValidator.php | 57 +++++++++++--------
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 03d4140359..610ed8107a 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -91,10 +91,10 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$stage_dir .= DIRECTORY_SEPARATOR . $web_root;
}
- $active_hashes = $this->getUpdateFunctions($active_dir, $extension);
- $staged_hashes = $this->getUpdateFunctions($stage_dir, $extension);
+ $active_functions = $this->getUpdateFunctions($active_dir, $extension);
+ $staged_functions = $this->getUpdateFunctions($stage_dir, $extension);
- return $active_hashes !== $staged_hashes;
+ return $active_functions !== $staged_functions;
}
/**
@@ -120,21 +120,6 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
]);
$functions = [];
- // 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).
- $is_named_function = function (array $tokens): bool {
- return (
- count($tokens) === 3 &&
- Inspector::assertAllStrictArrays($tokens) &&
- token_name($tokens[0][0]) === 'T_FUNCTION' &&
- token_name($tokens[1][0]) === 'T_WHITESPACE' &&
- token_name($tokens[2][0]) === 'T_STRING'
- );
- };
-
$patterns = [
'.install' => '/^' . $name . '_update_[0-9]+$/i',
'.post_update.php' => '/^' . $name . '_post_update_.+$/i',
@@ -151,11 +136,8 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
for ($i = 0; $i < count($tokens); $i++) {
$chunk = array_slice($tokens, $i, 3);
- if ($is_named_function($chunk)) {
- $name = $chunk[2][1];
- if (preg_match($pattern, $name)) {
- $functions[$suffix][] = $name;
- }
+ if ($this->tokensMatchFunctionNamePattern($chunk, $pattern)) {
+ $functions[$suffix][] = $tokens[2][1];
// We already know what the next three tokens are, so we can skip
// over them.
$i += 3;
@@ -169,6 +151,35 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
return $functions;
}
+ /**
+ * Determines if a set of tokens contain a function name matching a pattern.
+ *
+ * @param array[] $tokens
+ * A set of three tokens.
+ * @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.
+ */
+ 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 preg_match($pattern, $tokens[2][1]);
+ }
+ return FALSE;
+ }
+
/**
* {@inheritdoc}
*/
--
GitLab
From 80d6a35c52fbaa4e561c32ca7b62df18a967f273 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 16:07:13 -0400
Subject: [PATCH 19/30] Improve test checking
---
.../src/Validator/StagedDBUpdateValidator.php | 2 +-
.../src/Kernel/StagedDBUpdateValidatorTest.php | 16 ++++++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 610ed8107a..36deea7c54 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -175,7 +175,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
// 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 preg_match($pattern, $tokens[2][1]);
+ return (bool) preg_match($pattern, $tokens[2][1]);
}
return FALSE;
}
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index f32e7c5166..b27b5ce44e 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -146,10 +146,18 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $extension_dir, $extension_name, $file_extension);
$this->assertFileIsWritable($file);
- // Add a helper function with a random name, as well as an anonymous
- // function, to ensure they are ignored by the validator.
- $function_name = '_' . $extension_name . '_' . $this->randomMachineName();
- file_put_contents($file, "function $function_name() { \$foo = function () {}; }\n", FILE_APPEND);
+ // 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
+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($file, $code, FILE_APPEND);
$this->assertStatusCheckResults([], $stage);
// Now add a "real" update function -- either a schema update or a
--
GitLab
From ad7f5862431069e870ec000ba90af71fb40bb0da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 16:17:18 -0400
Subject: [PATCH 20/30] Round out the coverage
---
.../Kernel/StagedDBUpdateValidatorTest.php | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index b27b5ce44e..2ccc0b94cd 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -135,16 +135,17 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
* @dataProvider providerStagedDatabaseUpdate
*/
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;
+
$stage = $this->createStage();
$stage->create();
-
// Nothing has been changed in the stage, so ensure the validator doesn't
// detect any changes.
$this->assertStatusCheckResults([], $stage);
- $extension_name = basename($extension_dir);
- $file = sprintf('%s/%s/%s.%s', $stage->getStageDirectory(), $extension_dir, $extension_name, $file_extension);
- $this->assertFileIsWritable($file);
+ $staged_update_file = $stage->getStageDirectory() . '/' . $relative_file_path;
+ $this->assertFileIsWritable($staged_update_file);
// 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
@@ -157,28 +158,37 @@ function {$extension_name}_update__123() {}
function ($extension_name}__post_update_test() {}
function ($extension_name}_post_update() {}
END;
- file_put_contents($file, $code, FILE_APPEND);
+ file_put_contents($staged_update_file, $code, FILE_APPEND);
$this->assertStatusCheckResults([], $stage);
// 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.
- $function_name = match ($file_extension) {
+ $update_function_name = match ($file_extension) {
'install' => $extension_name . '_update_1001',
'post_update.php' => $extension_name . '_post_update_' . $this->randomMachineName(),
};
- file_put_contents($file, "function $function_name() {}\n", FILE_APPEND);
+ file_put_contents($staged_update_file, "function $update_function_name() {}\n", FILE_APPEND);
$this->assertStatusCheckResults($expected_results, $stage);
// If previous updates are removed, the validator should not detect any
// database updates.
- file_put_contents($file, "<?php\nfunction previous_updates_removed() {}");
+ file_put_contents($staged_update_file, "<?php\nfunction previous_updates_removed() {}");
$this->assertStatusCheckResults([], $stage);
// If the update file is deleted from the stage, the validator should not
// detect any database updates.
- unlink($file);
+ unlink($staged_update_file);
$this->assertStatusCheckResults([], $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);
}
/**
--
GitLab
From 9d48640448553a3ec30a84db1b5071304e75f462 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 16:22:24 -0400
Subject: [PATCH 21/30] Use a regular assert for count
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 36deea7c54..847aa4f9c4 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -165,7 +165,9 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* pattern; FALSE otherwise.
*/
private function tokensMatchFunctionNamePattern(array $tokens, string $pattern): bool {
- if (count($tokens) !== 3 || !Inspector::assertAllStrictArrays($tokens)) {
+ assert(count($tokens) === 3);
+
+ if (!Inspector::assertAllStrictArrays($tokens)) {
return FALSE;
}
// A named function declaration will always be a T_FUNCTION (the word
--
GitLab
From 4ae9dd240517d086efc877ad8c826373ef32e4f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 22 Jun 2023 16:50:40 -0400
Subject: [PATCH 22/30] Revert "Use a regular assert for count"
This reverts commit 9d48640448553a3ec30a84db1b5071304e75f462.
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 847aa4f9c4..36deea7c54 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -165,9 +165,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* pattern; FALSE otherwise.
*/
private function tokensMatchFunctionNamePattern(array $tokens, string $pattern): bool {
- assert(count($tokens) === 3);
-
- if (!Inspector::assertAllStrictArrays($tokens)) {
+ if (count($tokens) !== 3 || !Inspector::assertAllStrictArrays($tokens)) {
return FALSE;
}
// A named function declaration will always be a T_FUNCTION (the word
--
GitLab
From 4f9e7951ca04a8f8645d404253db5782ded3b3f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 10:58:33 -0400
Subject: [PATCH 23/30] Minor fixes, still needing test coverage
---
.../src/Validator/StagedDBUpdateValidator.php | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 36deea7c54..4cc4ea8ad4 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -118,7 +118,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$extension->getPath(),
$name,
]);
- $functions = [];
+ $function_names = [];
$patterns = [
'.install' => '/^' . $name . '_update_[0-9]+$/i',
@@ -137,18 +137,18 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
for ($i = 0; $i < count($tokens); $i++) {
$chunk = array_slice($tokens, $i, 3);
if ($this->tokensMatchFunctionNamePattern($chunk, $pattern)) {
- $functions[$suffix][] = $tokens[2][1];
+ $function_names[$suffix][] = $chunk[2][1];
// We already know what the next three tokens are, so we can skip
// over them.
$i += 3;
}
}
// Sort the list of functions to ensure they are compared consistently.
- if (array_key_exists($suffix, $functions)) {
- natcasesort($functions[$suffix]);
+ if (array_key_exists($suffix, $function_names)) {
+ natcasesort($function_names[$suffix]);
}
}
- return $functions;
+ return $function_names;
}
/**
--
GitLab
From fe6c9155656bb2612c399a4aab32f4e6af8575e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 11:15:05 -0400
Subject: [PATCH 24/30] Try to fill in missing test gaps, and switch to
array_diff for accuracy
---
.../src/Validator/StagedDBUpdateValidator.php | 13 ++++---------
.../src/Kernel/StagedDBUpdateValidatorTest.php | 8 +++++++-
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 4cc4ea8ad4..76014e449a 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -94,7 +94,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$active_functions = $this->getUpdateFunctions($active_dir, $extension);
$staged_functions = $this->getUpdateFunctions($stage_dir, $extension);
- return $active_functions !== $staged_functions;
+ return (bool) array_diff($staged_functions, $active_functions);
}
/**
@@ -105,10 +105,9 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* @param \Drupal\Core\Extension\Extension $extension
* The module to check.
*
- * @return array[]
+ * @return string[]
* The names of the update functions in the module's .install and
- * .post_update.php files, in that order, if they exist. The array will be
- * keyed by file extension.
+ * .post_update.php files.
*/
protected function getUpdateFunctions(string $root_dir, Extension $extension): array {
$name = $extension->getName();
@@ -137,16 +136,12 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
for ($i = 0; $i < count($tokens); $i++) {
$chunk = array_slice($tokens, $i, 3);
if ($this->tokensMatchFunctionNamePattern($chunk, $pattern)) {
- $function_names[$suffix][] = $chunk[2][1];
+ $function_names[] = $chunk[2][1];
// We already know what the next three tokens are, so we can skip
// over them.
$i += 3;
}
}
- // Sort the list of functions to ensure they are compared consistently.
- if (array_key_exists($suffix, $function_names)) {
- natcasesort($function_names[$suffix]);
- }
}
return $function_names;
}
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 2ccc0b94cd..75f76ed3c3 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -43,8 +43,14 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
mkdir($extension_path, 0777, TRUE);
$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) {
- file_put_contents("$extension_path/$extension_name.$suffix", "<?php\n");
+ $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() {}");
}
}
}
--
GitLab
From 75b9b3eb16b9fd6a042fad63b87e3b71d5b10b21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 11:17:01 -0400
Subject: [PATCH 25/30] Remove +3 jump
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 3 ---
1 file changed, 3 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 76014e449a..551ba1af3d 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -137,9 +137,6 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
$chunk = array_slice($tokens, $i, 3);
if ($this->tokensMatchFunctionNamePattern($chunk, $pattern)) {
$function_names[] = $chunk[2][1];
- // We already know what the next three tokens are, so we can skip
- // over them.
- $i += 3;
}
}
}
--
GitLab
From d89f9e35b2e819631a7ebefd6e196613abb395c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 12:39:20 -0400
Subject: [PATCH 26/30] Fix feedback
---
.../Kernel/StagedDBUpdateValidatorTest.php | 28 ++++++++-----------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
index 75f76ed3c3..a021618e3a 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -153,33 +153,29 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
$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, FILE_APPEND);
- $this->assertStatusCheckResults([], $stage);
-
- // 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);
-
- // If previous updates are removed, the validator should not detect any
- // database updates.
- file_put_contents($staged_update_file, "<?php\nfunction previous_updates_removed() {}");
+ file_put_contents($staged_update_file, $code);
$this->assertStatusCheckResults([], $stage);
// If the update file is deleted from the stage, the validator should not
--
GitLab
From a5281d69b7833c103f38c6468836fe2018833018 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 13:28:35 -0400
Subject: [PATCH 27/30] Mention token_get_all in comment
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index 551ba1af3d..d626e5e29f 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -147,7 +147,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* Determines if a set of tokens contain a function name matching a pattern.
*
* @param array[] $tokens
- * A set of three 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.
@@ -155,6 +155,8 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
* @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)) {
--
GitLab
From 3d0c6b589302bd0ffb2d5aa6845b181c53f8c520 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 13:54:13 -0400
Subject: [PATCH 28/30] Remove the word "possible" -- these are definitely
database updates
---
package_manager/src/Validator/StagedDBUpdateValidator.php | 4 ++--
.../tests/src/Kernel/StagedDBUpdateValidatorTest.php | 2 +-
src/Validator/StagedDatabaseUpdateValidator.php | 2 +-
.../Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/package_manager/src/Validator/StagedDBUpdateValidator.php b/package_manager/src/Validator/StagedDBUpdateValidator.php
index d626e5e29f..60c6dbea82 100644
--- a/package_manager/src/Validator/StagedDBUpdateValidator.php
+++ b/package_manager/src/Validator/StagedDBUpdateValidator.php
@@ -55,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.'));
}
}
@@ -190,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 a021618e3a..0e1dbc483c 100644
--- a/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
+++ b/package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php
@@ -62,7 +62,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
* The test cases.
*/
public function providerStagedDatabaseUpdate(): array {
- $summary = t('Possible database updates have been detected in the following extensions.');
+ $summary = t('Database updates have been detected in the following extensions.');
return [
'schema update in installed module' => [
diff --git a/src/Validator/StagedDatabaseUpdateValidator.php b/src/Validator/StagedDatabaseUpdateValidator.php
index a2998417dd..e04da6c289 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/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
index 09d72af5f1..b435fb2bad 100644
--- a/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
+++ b/tests/src/Kernel/StatusCheck/StagedDatabaseUpdateValidatorTest.php
@@ -39,7 +39,7 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
$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\n";
+ $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));
}
--
GitLab
From 96d5c98cf8971a8c4537e923e57e2d3afcc3a43b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 14:29:50 -0400
Subject: [PATCH 29/30] Fix one wrong assertion
---
tests/src/Functional/StagedDatabaseUpdateTest.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/src/Functional/StagedDatabaseUpdateTest.php b/tests/src/Functional/StagedDatabaseUpdateTest.php
index 4d5fcbc5e0..e34c9f53e7 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) {
--
GitLab
From ed58fe18e653f590014f72a5bd9e452b47413a2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 23 Jun 2023 14:59:25 -0400
Subject: [PATCH 30/30] one more dammit
---
.../tests/src/Functional/SuccessfulUpdateTest.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php b/automatic_updates_extensions/tests/src/Functional/SuccessfulUpdateTest.php
index 4ada9bad9c..db545fc716 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();
--
GitLab