Issue #3230507 by phenaproxima, tedbow: Support sites using drupal/core-recommended

automatic_updates.services.yml

@@ -4,7 +4,7 @@ services:
     arguments: ['@keyvalue.expirable', '@datetime.time', 24]
   automatic_updates.updater:
     class: Drupal\automatic_updates\Updater
-    arguments: ['@state', '@string_translation','@automatic_updates.beginner', '@automatic_updates.stager', '@automatic_updates.cleaner', '@automatic_updates.committer', '@event_dispatcher']
+    arguments: ['@state', '@string_translation','@automatic_updates.beginner', '@automatic_updates.stager', '@automatic_updates.cleaner', '@automatic_updates.committer', '@event_dispatcher', '@config.factory']
   automatic_updates.staged_package_validator:
     class: Drupal\automatic_updates\Validation\StagedProjectsValidation
     arguments: ['@string_translation', '@automatic_updates.updater' ]
@@ -78,7 +78,7 @@ services:
       - { name: event_subscriber }
   automatic_updates.update_version_subscriber:
     class: Drupal\automatic_updates\Event\UpdateVersionSubscriber
-    arguments: ['@module_handler']
+    arguments: ['@automatic_updates.updater']
     tags:
       - { name: event_subscriber }
   automatic_updates.composer_executable_validator:

src/Event/UpdateVersionSubscriber.php

@@ -3,9 +3,9 @@
 namespace Drupal\automatic_updates\Event;
 
 use Drupal\automatic_updates\AutomaticUpdatesEvents;
+use Drupal\automatic_updates\Updater;
 use Drupal\automatic_updates\Validation\ValidationResult;
 use Drupal\Core\Extension\ExtensionVersion;
-use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
@@ -16,15 +16,21 @@ class UpdateVersionSubscriber implements EventSubscriberInterface {
 
   use StringTranslationTrait;
 
+  /**
+   * The updater service.
+   *
+   * @var \Drupal\automatic_updates\Updater
+   */
+  protected $updater;
+
   /**
    * Constructs an UpdateVersionSubscriber.
    *
-   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
-   *   The module handler service.
+   * @param \Drupal\automatic_updates\Updater $updater
+   *   The updater service.
    */
-  public function __construct(ModuleHandlerInterface $module_handler) {
-    // Load procedural functions needed for ::getCoreVersion().
-    $module_handler->loadInclude('update', 'inc', 'update.compare');
+  public function __construct(Updater $updater) {
+    $this->updater = $updater;
   }
 
   /**
@@ -34,7 +40,11 @@ class UpdateVersionSubscriber implements EventSubscriberInterface {
    *   The running core version as known to the Update module.
    */
   protected function getCoreVersion(): string {
-    $available_updates = update_calculate_project_data(update_get_available());
+    // We need to call these functions separately, because
+    // update_get_available() will include the file that contains
+    // update_calculate_project_data().
+    $available_updates = update_get_available();
+    $available_updates = update_calculate_project_data($available_updates);
     return $available_updates['drupal']['existing_version'];
   }
 
@@ -46,7 +56,8 @@ class UpdateVersionSubscriber implements EventSubscriberInterface {
    */
   public function checkUpdateVersion(PreStartEvent $event): void {
     $from_version = ExtensionVersion::createFromVersionString($this->getCoreVersion());
-    $to_version = ExtensionVersion::createFromVersionString($event->getPackageVersions()['drupal/core']);
+    $core_package_name = $this->updater->getCorePackageName();
+    $to_version = ExtensionVersion::createFromVersionString($event->getPackageVersions()[$core_package_name]);
 
     if ($from_version->getMajorVersion() !== $to_version->getMajorVersion()) {
       $error = ValidationResult::createError([

src/Updater.php

@@ -8,6 +8,8 @@ use Drupal\automatic_updates\Event\PreStartEvent;
 use Drupal\automatic_updates\Event\UpdateEvent;
 use Drupal\automatic_updates\Exception\UpdateException;
 use Drupal\Component\FileSystem\FileSystem;
+use Drupal\Component\Serialization\Json;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\State\StateInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\StringTranslation\TranslationInterface;
@@ -74,6 +76,13 @@ class Updater {
    */
   protected $eventDispatcher;
 
+  /**
+   * The config factory service.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
   /**
    * Constructs an Updater object.
    *
@@ -91,8 +100,10 @@ class Updater {
    *   The Composer Stager's committer service.
    * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher
    *   The event dispatcher service.
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory service.
    */
-  public function __construct(StateInterface $state, TranslationInterface $translation, BeginnerInterface $beginner, StagerInterface $stager, CleanerInterface $cleaner, CommitterInterface $committer, EventDispatcherInterface $event_dispatcher) {
+  public function __construct(StateInterface $state, TranslationInterface $translation, BeginnerInterface $beginner, StagerInterface $stager, CleanerInterface $cleaner, CommitterInterface $committer, EventDispatcherInterface $event_dispatcher, ConfigFactoryInterface $config_factory) {
     $this->state = $state;
     $this->beginner = $beginner;
     $this->stager = $stager;
@@ -100,6 +111,7 @@ class Updater {
     $this->committer = $committer;
     $this->setStringTranslation($translation);
     $this->eventDispatcher = $event_dispatcher;
+    $this->configFactory = $config_factory;
   }
 
   /**
@@ -120,10 +132,10 @@ class Updater {
    *   The absolute path for stage directory.
    */
   public function getStageDirectory(): string {
-    // @todo This should be unique, in order to support parallel runs, or
-    // multiple sites on the same server. Find a way to make it unique, and
-    // persistent for the entire lifetime of the update process.
-    return FileSystem::getOsTemporaryDirectory() . '/.automatic_updates_stage';
+    // Append the site ID to the directory in order to support parallel test
+    // runs, or multiple sites hosted on the same server.
+    $site_id = $this->configFactory->get('system.site')->get('uuid');
+    return FileSystem::getOsTemporaryDirectory() . '/.automatic_updates_stage_' . $site_id;
   }
 
   /**
@@ -167,7 +179,7 @@ class Updater {
       throw new \InvalidArgumentException("Currently only updates to Drupal core are supported.");
     }
     $packages = [
-      'drupal/core' => $project_versions['drupal'],
+      $this->getCorePackageName() => $project_versions['drupal'],
     ];
     $stage_key = $this->createActiveStage($packages);
     /** @var \Drupal\automatic_updates\Event\PreStartEvent $event */
@@ -176,6 +188,46 @@ class Updater {
     return $stage_key;
   }
 
+  /**
+   * Determines the name of the core package in the project composer.json.
+   *
+   * This makes the following assumptions:
+   * - The vendor directory is next to the project composer.json.
+   * - The project composer.json contains a requirement for a core package.
+   * - That requirement is either for drupal/core or drupal/core-recommended.
+   *
+   * @return string
+   *   The name of the core package (either drupal/core or
+   *   drupal/core-recommended).
+   *
+   * @throws \RuntimeException
+   *   If the project composer.json is not found.
+   * @throws \LogicException
+   *   If the project composer.json does not contain one of the supported core
+   *   packages.
+   *
+   * @todo Move this to an update validator, or use a more robust method of
+   *   detecting the core package.
+   */
+  public function getCorePackageName(): string {
+    $composer = realpath(static::getVendorDirectory() . '/../composer.json');
+
+    if (empty($composer) || !file_exists($composer)) {
+      throw new \RuntimeException("Could not find project-level composer.json");
+    }
+
+    $composer = file_get_contents($composer);
+    $composer = Json::decode($composer);
+
+    if (isset($composer['require']['drupal/core'])) {
+      return 'drupal/core';
+    }
+    elseif (isset($composer['require']['drupal/core-recommended'])) {
+      return 'drupal/core-recommended';
+    }
+    throw new \LogicException("Could not determine the Drupal core package in the project-level composer.json.");
+  }
+
   /**
    * Gets the excluded paths collected by an event object.
    *

tests/src/Build/AttendedCoreRecommendedUpdateTest.php

+<?php
+
+namespace Drupal\Tests\automatic_updates\Build;
+
+/**
+ * Tests an end-to-end core update via the core-recommended metapackage.
+ *
+ * @group automatic_updates
+ */
+class AttendedCoreRecommendedUpdateTest extends AttendedCoreUpdateTest {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $webRoot = 'docroot/';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getConfigurationForUpdate(string $version): array {
+    $changes = parent::getConfigurationForUpdate($version);
+
+    // Create a fake version of drupal/core-recommended which requires the
+    // target version of drupal/core.
+    $dir = $this->copyPackage($this->getDrupalRoot() . '/composer/Metapackage/CoreRecommended');
+    $this->alterPackage($dir, [
+      'version' => $version,
+      'require' => [
+        'drupal/core' => $version,
+      ],
+    ]);
+    $changes['repositories']['drupal/core-recommended']['url'] = $dir;
+
+    return $changes;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getInitialConfiguration(): array {
+    $configuration = parent::getInitialConfiguration();
+
+    // Use drupal/core-recommended to build the test site, instead of directly
+    // requiring drupal/core.
+    $require = &$configuration['require'];
+    $require['drupal/core-recommended'] = $require['drupal/core'];
+    unset($require['drupal/core']);
+
+    $configuration['repositories']['drupal/core-recommended'] = [
+      'type' => 'path',
+      'url' => implode(DIRECTORY_SEPARATOR, [
+        $this->getDrupalRoot(),
+        'composer',
+        'Metapackage',
+        'CoreRecommended',
+      ]),
+    ];
+    return $configuration;
+  }
+
+}

tests/src/Build/AttendedCoreUpdateTest.php

@@ -2,8 +2,6 @@
 
 namespace Drupal\Tests\automatic_updates\Build;
 
-use Symfony\Component\Filesystem\Filesystem;
-
 /**
  * Tests an end-to-end update of Drupal core within the UI.
  *
@@ -11,49 +9,16 @@ use Symfony\Component\Filesystem\Filesystem;
  */
 class AttendedCoreUpdateTest extends AttendedUpdateTestBase {
 
-  /**
-   * A directory containing a fake version of core that we will update to.
-   *
-   * @var string
-   */
-  private $coreDir;
-
   /**
    * {@inheritdoc}
    */
   protected function tearDown(): void {
-    if ($this->destroyBuild && $this->coreDir) {
-      (new Filesystem())->remove($this->coreDir);
+    if ($this->destroyBuild) {
+      $this->deleteCopiedPackages();
     }
     parent::tearDown();
   }
 
-  /**
-   * Creates a Drupal core code base and assigns it an arbitrary version number.
-   *
-   * @param string $version
-   *   The version number that the Drupal core code base should have.
-   *
-   * @return string
-   *   The path of the code base.
-   */
-  protected function createTargetCorePackage(string $version): string {
-    $dir = $this->getWorkspaceDirectory();
-    $source = "$dir/core";
-    $this->assertDirectoryExists($source);
-    $destination = $dir . uniqid('_core_');
-    $this->assertDirectoryDoesNotExist($destination);
-
-    $fs = new Filesystem();
-    $fs->mirror($source, $destination);
-
-    $this->setCoreVersion($destination, $version);
-    // This is for us to be certain that we actually update to our local, fake
-    // version of Drupal core.
-    file_put_contents($destination . '/README.txt', "Placeholder for Drupal core $version.");
-    return $destination;
-  }
-
   /**
    * Modifies a Drupal core code base to set its version.
    *
@@ -63,10 +28,7 @@ class AttendedCoreUpdateTest extends AttendedUpdateTestBase {
    *   The version number to set.
    */
   private function setCoreVersion(string $dir, string $version): void {
-    $composer = "$dir/composer.json";
-    $data = $this->readJson($composer);
-    $data['version'] = $version;
-    $this->writeJson($composer, $data);
+    $this->alterPackage($dir, ['version' => $version]);
 
     $drupal_php = "$dir/lib/Drupal.php";
     $this->assertIsWritable($drupal_php);
@@ -80,7 +42,37 @@ class AttendedCoreUpdateTest extends AttendedUpdateTestBase {
    */
   protected function createTestSite(): void {
     parent::createTestSite();
-    $this->setCoreVersion($this->getWorkspaceDirectory() . '/core', '9.8.0');
+    $this->setCoreVersion($this->getWebRoot() . '/core', '9.8.0');
+  }
+
+  /**
+   * Returns composer.json changes that are needed to update core.
+   *
+   * @param string $version
+   *   The version of core we will be updating to.
+   *
+   * @return array
+   *   The changes to merge into the test site's composer.json.
+   */
+  protected function getConfigurationForUpdate(string $version): array {
+    // Create a fake version of core with the given version number, and change
+    // its README so that we can actually be certain that we update to this
+    // fake version.
+    $dir = $this->copyPackage($this->getWebRoot() . '/core');
+    $this->setCoreVersion($dir, $version);
+    file_put_contents("$dir/README.txt", "Placeholder for Drupal core $version.");
+
+    return [
+      'repositories' => [
+        'drupal/core' => [
+          'type' => 'path',
+          'url' => $dir,
+          'options' => [
+            'symlink' => FALSE,
+          ],
+        ],
+      ],
+    ];
   }
 
   /**
@@ -88,18 +80,7 @@ class AttendedCoreUpdateTest extends AttendedUpdateTestBase {
    */
   public function test(): void {
     $this->createTestSite();
-    $this->coreDir = $this->createTargetCorePackage('9.8.1');
-
-    $composer = $this->getWorkspaceDirectory() . "/composer.json";
-    $data = $this->readJson($composer);
-    $data['repositories']['drupal/core'] = [
-      'type' => 'path',
-      'url' => $this->coreDir,
-      'options' => [
-        'symlink' => FALSE,
-      ],
-    ];
-    $this->writeJson($composer, $data);
+    $this->alterPackage($this->getWorkspaceDirectory(), $this->getConfigurationForUpdate('9.8.1'));
 
     $this->installQuickStart('minimal');
     $this->setReleaseMetadata(['drupal' => '0.0']);
@@ -126,7 +107,7 @@ class AttendedCoreUpdateTest extends AttendedUpdateTestBase {
     $assert_session->pageTextContains('Update complete!');
     $this->assertCoreVersion('9.8.1');
 
-    $placeholder = file_get_contents($this->getWorkspaceDirectory() . '/core/README.txt');
+    $placeholder = file_get_contents($this->getWebRoot() . '/core/README.txt');
     $this->assertSame('Placeholder for Drupal core 9.8.1.', $placeholder);
   }
 

tests/src/Build/AttendedUpdateTestBase.php

@@ -14,6 +14,7 @@ abstract class AttendedUpdateTestBase extends QuickStartTestBase {
 
   use LocalPackagesTrait {
     getPackagePath as traitGetPackagePath;
+    copyPackage as traitCopyPackage;
   }
   use SettingsTrait;
 
@@ -24,6 +25,13 @@ abstract class AttendedUpdateTestBase extends QuickStartTestBase {
    */
   private $metadataServer;
 
+  /**
+   * The test site's document root, relative to the workspace directory.
+   *
+   * @var string
+   */
+  protected $webRoot = './';
+
   /**
    * {@inheritdoc}
    */
@@ -34,6 +42,13 @@ abstract class AttendedUpdateTestBase extends QuickStartTestBase {
     parent::tearDown();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function copyPackage(string $source_dir, string $destination_dir = NULL): string {
+    return $this->traitCopyPackage($source_dir, $destination_dir ?: $this->getWorkspaceDirectory());
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -52,6 +67,16 @@ abstract class AttendedUpdateTestBase extends QuickStartTestBase {
     return $this->traitGetPackagePath($package);
   }
 
+  /**
+   * Returns the full path to the test site's document root.
+   *
+   * @return string
+   *   The full path of the test site's document root.
+   */
+  protected function getWebRoot(): string {
+    return $this->getWorkspaceDirectory() . DIRECTORY_SEPARATOR . $this->webRoot;
+  }
+
   /**
    * Prepares the test site to serve an XML feed of available release metadata.
    *
@@ -72,12 +97,12 @@ END;
     // about available updates.
     if (empty($this->metadataServer)) {
       $port = $this->findAvailablePort();
-      $this->metadataServer = $this->instantiateServer($port);
+      $this->metadataServer = $this->instantiateServer($port, $this->webRoot);
       $code .= <<<END
 \$config['update.settings']['fetch']['url'] = 'http://localhost:$port/automatic-update-test';
 END;
     }
-    $this->addSettings($code, $this->getWorkspaceDirectory());
+    $this->addSettings($code, $this->getWebRoot());
   }
 
   /**
@@ -91,11 +116,25 @@ END;
     $this->assertCommandSuccessful();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function visit($request_uri = '/', $working_dir = NULL) {
+    return parent::visit($request_uri, $working_dir ?: $this->webRoot);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function formLogin($username, $password, $working_dir = NULL) {
+    parent::formLogin($username, $password, $working_dir ?: $this->webRoot);
+  }
+
   /**
    * {@inheritdoc}
    */
   public function installQuickStart($profile, $working_dir = NULL) {
-    parent::installQuickStart($profile, $working_dir);
+    parent::installQuickStart($profile, $working_dir ?: $this->webRoot);
 
     // Always allow test modules to be installed in the UI and, for easier
     // debugging, always display errors in their dubious glory.
@@ -103,25 +142,30 @@ END;
 \$settings['extension_discovery_scan_tests'] = TRUE;
 \$config['system.logging']['error_level'] = 'verbose';
 END;
-    $this->addSettings($php, $this->getWorkspaceDirectory());
+    $this->addSettings($php, $this->getWebRoot());
   }
 
   /**
    * Uses our already-installed dependencies to build a test site to update.
    */
   protected function createTestSite(): void {
+    // The project-level composer.json lives in the workspace root directory,
+    // which may or may not be the same directory as the web root (where Drupal
+    // itself lives).
     $composer = $this->getWorkspaceDirectory() . DIRECTORY_SEPARATOR . 'composer.json';
-    $this->writeJson($composer, $this->getComposerConfiguration());
+    $this->writeJson($composer, $this->getInitialConfiguration());
     $this->runComposer('update');
   }
 
   /**
-   * Returns the data to write to the test site's composer.json.
+   * Returns the initial data to write to the test site's composer.json.
    *
-   * @return mixed[]
+   * This configuration will be used to build the pre-update test site.
+   *
+   * @return array
    *   The data that should be written to the test site's composer.json.
    */
-  protected function getComposerConfiguration(): array {
+  protected function getInitialConfiguration(): array {
     $core_constraint = preg_replace('/\.[0-9]+-dev$/', '.x-dev', \Drupal::VERSION);
 
     $drupal_root = $this->getDrupalRoot();
@@ -166,11 +210,16 @@ END;
       ],
       'repositories' => $repositories,
       'extra' => [
+        'drupal-scaffold' => [
+          'locations' => [
+            'web-root' => $this->webRoot,
+          ],
+        ],
         'installer-paths' => [
-          'core' => [
+          $this->webRoot . 'core' => [
             'type:drupal-core',
           ],
-          'modules/{$name}' => [
+          $this->webRoot . 'modules/{$name}' => [
             'type:drupal-module',
           ],
         ],

tests/src/Traits/LocalPackagesTrait.php

@@ -2,7 +2,10 @@
 
 namespace Drupal\Tests\automatic_updates\Traits;
 
+use Drupal\Component\FileSystem\FileSystem;
+use Drupal\Component\Utility\NestedArray;
 use PHPUnit\Framework\Assert;
+use Symfony\Component\Filesystem\Filesystem as SymfonyFilesystem;
 
 /**
  * Provides methods for interacting with installed Composer packages.
@@ -11,6 +14,16 @@ trait LocalPackagesTrait {
 
   use JsonTrait;
 
+  /**
+   * The paths of temporary copies of packages.
+   *
+   * @see ::copyPackage()
+   * @see ::deleteCopiedPackages()
+   *
+   * @var string[]
+   */
+  private $copiedPackages = [];
+
   /**
    * Returns the path of an installed package, relative to composer.json.
    *
@@ -24,6 +37,50 @@ trait LocalPackagesTrait {
     return 'vendor' . DIRECTORY_SEPARATOR . $package['name'];
   }
 
+  /**
+   * Deletes all copied packages.
+   *
+   * @see ::copyPackage()
+   */
+  protected function deleteCopiedPackages(): void {
+    (new SymfonyFilesystem())->remove($this->copiedPackages);
+  }
+
+  /**
+   * Copies a package's entire directory to another location.
+   *
+   * The copies' paths will be stored so that they can be easily deleted by
+   * ::deleteCopiedPackages().
+   *
+   * @param string $source_dir
+   *   The path of the package directory to copy.
+   * @param string|null $destination_dir
+   *   (optional) The directory to which the package should be copied. Will be
+   *   suffixed with a random string to ensure uniqueness. If not given, the
+   *   system temporary directory will be used.
+   *
+   * @return string
+   *   The path of the temporary copy.
+   *
+   * @see ::deleteCopiedPackages()
+   */
+  protected function copyPackage(string $source_dir, string $destination_dir = NULL): string {
+    Assert::assertDirectoryExists($source_dir);
+
+    if (empty($destination_dir)) {
+      $destination_dir = FileSystem::getOsTemporaryDirectory();
+      Assert::assertNotEmpty($destination_dir);
+      $destination_dir .= DIRECTORY_SEPARATOR;
+    }
+    $destination_dir = uniqid($destination_dir);
+    Assert::assertDirectoryDoesNotExist($destination_dir);
+
+    (new SymfonyFilesystem())->mirror($source_dir, $destination_dir);
+    array_push($this->copiedPackages, $destination_dir);
+
+    return $destination_dir;
+  }
+
   /**
    * Generates local path repositories for a set of installed packages.
    *
@@ -61,6 +118,21 @@ trait LocalPackagesTrait {
     return $repositories;
   }
 
+  /**
+   * Alters a package's composer.json file.
+   *
+   * @param string $package_dir
+   *   The package directory.
+   * @param array $changes
+   *   The changes to merge into composer.json.
+   */
+  protected function alterPackage(string $package_dir, array $changes): void {
+    $composer = $package_dir . DIRECTORY_SEPARATOR . 'composer.json';
+    $data = $this->readJson($composer);
+    $data = NestedArray::mergeDeep($data, $changes);
+    $this->writeJson($composer, $data);
+  }
+
   /**
    * Reads all package information from a composer.lock file.
    *