Issue #3280168 by kunal.sachdev: StagedDatabaseUpdateValidator should check themes also

automatic_updates.services.yml

@@ -108,6 +108,7 @@ services:
     arguments:
       - '@package_manager.path_locator'
       - '@extension.list.module'
+      - '@extension.list.theme'
       - '@string_translation'
     tags:
       - { name: event_subscriber }

src/Validator/StagedDatabaseUpdateValidator.php

@@ -6,6 +6,7 @@ use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\Updater;
 use Drupal\Core\Extension\Extension;
 use Drupal\Core\Extension\ModuleExtensionList;
+use Drupal\Core\Extension\ThemeExtensionList;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\StringTranslation\TranslationInterface;
 use Drupal\package_manager\Event\PreApplyEvent;
@@ -37,6 +38,13 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface {
    */
   protected $moduleList;
 
+  /**
+   * The theme list service.
+   *
+   * @var \Drupal\Core\Extension\ThemeExtensionList
+   */
+  protected $themeList;
+
   /**
    * Constructs a StagedDatabaseUpdateValidator object.
    *
@@ -44,12 +52,15 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface {
    *   The path locator service.
    * @param \Drupal\Core\Extension\ModuleExtensionList $module_list
    *   The module list service.
+   * @param \Drupal\Core\Extension\ThemeExtensionList $theme_list
+   *   The theme list service.
    * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
    *   The string translation service.
    */
-  public function __construct(PathLocator $path_locator, ModuleExtensionList $module_list, TranslationInterface $translation) {
+  public function __construct(PathLocator $path_locator, ModuleExtensionList $module_list, ThemeExtensionList $theme_list, TranslationInterface $translation) {
     $this->pathLocator = $path_locator;
     $this->moduleList = $module_list;
+    $this->themeList = $theme_list;
     $this->setStringTranslation($translation);
   }
 
@@ -65,19 +76,23 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface {
       return;
     }
 
-    $invalid_modules = [];
+    $invalid_extensions = [];
     // Although \Drupal\automatic_updates\Validator\StagedProjectsValidator
     // should prevent non-core modules from being added, updated, or removed in
     // the staging area, we check all installed modules so as not to rely on the
     // presence of StagedProjectsValidator.
-    foreach ($this->moduleList->getAllInstalledInfo() as $name => $info) {
-      if ($this->hasStagedUpdates($stage, $this->moduleList->get($name))) {
-        $invalid_modules[] = $info['name'];
+    foreach ($this->moduleList->getAllInstalledInfo() as $module_name => $module_info) {
+      if ($this->hasStagedUpdates($stage, $this->moduleList->get($module_name))) {
+        $invalid_extensions[] = $module_info['name'];
       }
     }
-
-    if ($invalid_modules) {
-      $event->addError($invalid_modules, $this->t('The update cannot proceed because possible database updates have been detected in the following modules.'));
+    foreach ($this->themeList->getAllInstalledInfo() as $theme_name => $theme_info) {
+      if ($this->hasStagedUpdates($stage, $this->themeList->get($theme_name))) {
+        $invalid_extensions[] = $theme_info['name'];
+      }
+    }
+    if ($invalid_extensions) {
+      $event->addError($invalid_extensions, $this->t('The update cannot proceed because possible database updates have been detected in the following extensions.'));
     }
   }
 
@@ -126,18 +141,18 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface {
    *
    * @param string $root_dir
    *   The root directory of the Drupal code base.
-   * @param \Drupal\Core\Extension\Extension $module
+   * @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.
    */
-  protected function getHashes(string $root_dir, Extension $module): array {
+  protected function getHashes(string $root_dir, Extension $extension): array {
     $path = implode(DIRECTORY_SEPARATOR, [
       $root_dir,
-      $module->getPath(),
-      $module->getName(),
+      $extension->getPath(),
+      $extension->getName(),
     ]);
     $hashes = [];
 

tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php

@@ -49,12 +49,19 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
     $virtual_active_dir = $this->container->get('package_manager.path_locator')
       ->getProjectRoot();
 
-    // Copy the .install and .post_update.php files from every installed module,
-    // in the *actual* Drupal code base that is running this test, into the
-    // virtual project (i.e., the active directory).
-    $module_list = $this->container->get('module_handler')->getModuleList();
-    foreach ($module_list as $name => $module) {
-      $path = $module->getPath();
+    // Copy the .install and .post_update.php files from all extensions used in
+    // this test class, in the *actual* Drupal code base that is running this
+    // test, into the virtual project (i.e., the active directory).
+    $module_list = $this->container->get('extension.list.module');
+    $extensions = [];
+    $extensions['system'] = $module_list->get('system');
+    $extensions['views'] = $module_list->get('views');
+    $extensions['package_manager_bypass'] = $module_list->get('package_manager_bypass');
+    $theme_list = $this->container->get('extension.list.theme');
+    $extensions['automatic_updates_theme'] = $theme_list->get('automatic_updates_theme');
+    $extensions['automatic_updates_theme_with_updates'] = $theme_list->get('automatic_updates_theme_with_updates');
+    foreach ($extensions as $name => $extension) {
+      $path = $extension->getPath();
       @mkdir("$virtual_active_dir/$path", 0777, TRUE);
 
       foreach (static::SUFFIXES as $suffix) {
@@ -68,18 +75,23 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
    * Tests that no errors are raised if staged files have no DB updates.
    */
   public function testNoUpdates(): void {
-    // Since we're testing with a modified version of Views, it should not be
-    // installed.
+    // Since we're testing with a modified version of 'views' and
+    // 'automatic_updates_theme_with_updates', these should not be installed.
     $this->assertFalse($this->container->get('module_handler')->moduleExists('views'));
+    $this->assertFalse($this->container->get('theme_handler')->themeExists('automatic_updates_theme_with_updates'));
 
-    // Create bogus staged versions of Views' .install and .post_update.php
-    // files. Since it's not installed, the changes should not raise any
-    // validation errors.
+    // Create bogus staged versions of Views' and
+    // Automatic Updates Theme with Updates .install and .post_update.php files.
+    // Since these extensions are not installed, the changes should not raise
+    // any validation errors.
     $updater = $this->container->get('automatic_updates.cron_updater');
-    $module_dir = $updater->getStageDirectory() . '/core/modules/views';
-    mkdir($module_dir, 0777, TRUE);
+    $module_list = $this->container->get('extension.list.module')->getList();
+    $theme_list = $this->container->get('extension.list.theme')->getList();
+    $module_dir = $updater->getStageDirectory() . '/' . $module_list['views']->getPath();
+    $theme_dir = $updater->getStageDirectory() . '/' . $theme_list['automatic_updates_theme_with_updates']->getPath();
     foreach (static::SUFFIXES as $suffix) {
       file_put_contents("$module_dir/views.$suffix", $this->randomString());
+      file_put_contents("$theme_dir/automatic_updates_theme_with_updates.$suffix", $this->randomString());
     }
 
     $updater->apply();
@@ -112,19 +124,25 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
    * @dataProvider providerFileChanged
    */
   public function testFileChanged(string $suffix, bool $delete): void {
-    /** @var \Drupal\Tests\automatic_updates\Kernel\ReadinessValidation\TestCronUpdater $updater */
+    /** @var \Drupal\automatic_updates\CronUpdater $updater */
     $updater = $this->container->get('automatic_updates.cron_updater');
-
-    $file = $updater->getStageDirectory() . "/core/modules/system/system.$suffix";
+    $theme_installer = $this->container->get('theme_installer');
+    $theme_installer->install(['automatic_updates_theme_with_updates']);
+    $theme = $this->container->get('theme_handler')
+      ->getTheme('automatic_updates_theme_with_updates');
+    $module_file = $updater->getStageDirectory() . "/core/modules/system/system.$suffix";
+    $theme_file = $updater->getStageDirectory() . "/{$theme->getPath()}/automatic_updates_theme_with_updates.$suffix";
     if ($delete) {
-      unlink($file);
+      unlink($module_file);
+      unlink($theme_file);
     }
     else {
-      file_put_contents($file, $this->randomString());
+      file_put_contents($module_file, $this->randomString());
+      file_put_contents($theme_file, $this->randomString());
     }
 
     $expected_results = [
-      ValidationResult::createError(['System'], t('The update cannot proceed because possible database updates have been detected in the following modules.')),
+      ValidationResult::createError(['System', 'Automatic Updates Theme With Updates'], t('The update cannot proceed because possible database updates have been detected in the following extensions.')),
     ];
 
     try {
@@ -142,21 +160,28 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
   public function testUpdatesAddedInStage(): void {
     $module = $this->container->get('module_handler')
       ->getModule('package_manager_bypass');
+    $theme_installer = $this->container->get('theme_installer');
+    $theme_installer->install(['automatic_updates_theme']);
+    $theme = $this->container->get('theme_handler')
+      ->getTheme('automatic_updates_theme');
 
-    /** @var \Drupal\Tests\automatic_updates\Kernel\ReadinessValidation\TestCronUpdater $updater */
+    /** @var \Drupal\automatic_updates\CronUpdater $updater */
     $updater = $this->container->get('automatic_updates.cron_updater');
 
     foreach (static::SUFFIXES as $suffix) {
-      $file = sprintf('%s/%s/%s.%s', $updater->getStageDirectory(), $module->getPath(), $module->getName(), $suffix);
-      // The file we're creating shouldn't already exist in the staging area
+      $module_file = sprintf('%s/%s/%s.%s', $updater->getStageDirectory(), $module->getPath(), $module->getName(), $suffix);
+      $theme_file = sprintf('%s/%s/%s.%s', $updater->getStageDirectory(), $theme->getPath(), $theme->getName(), $suffix);
+      // The files we're creating shouldn't already exist in the staging area
       // unless it's a file we actually ship, which is a scenario covered by
       // ::testFileChanged().
-      $this->assertFileDoesNotExist($file);
-      file_put_contents($file, $this->randomString());
+      $this->assertFileDoesNotExist($module_file);
+      $this->assertFileDoesNotExist($theme_file);
+      file_put_contents($module_file, $this->randomString());
+      file_put_contents($theme_file, $this->randomString());
     }
 
     $expected_results = [
-      ValidationResult::createError(['Package Manager Bypass'], t('The update cannot proceed because possible database updates have been detected in the following modules.')),
+      ValidationResult::createError(['Package Manager Bypass', 'Automatic Updates Theme'], t('The update cannot proceed because possible database updates have been detected in the following extensions.')),
     ];
 
     try {

tests/themes/automatic_updates_theme/automatic_updates_theme.info.yml

+name: Automatic Updates Theme
+type: theme
+description: 'Empty theme for tests.'
+package: Testing
+base theme: false

tests/themes/automatic_updates_theme_with_updates/automatic_updates_theme_with_updates.info.yml

+name: Automatic Updates Theme With Updates
+type: theme
+description: 'Empty theme with updates for tests.'
+package: Testing
+base theme: false

tests/themes/automatic_updates_theme_with_updates/automatic_updates_theme_with_updates.install

+<?php
+
+/**
+ * @file
+ * Blank .install file.
+ *
+ * @see \Drupal\Tests\automatic_updates\Kernel\ReadinessValidation\StagedDatabaseUpdateValidatorTest
+ */

tests/themes/automatic_updates_theme_with_updates/automatic_updates_theme_with_updates.post_update.php

+<?php
+
+/**
+ * @file
+ * Blank .post_update file.
+ *
+ * @see \Drupal\Tests\automatic_updates\Kernel\ReadinessValidation\StagedDatabaseUpdateValidatorTest
+ */