Issue #3232004 by phenaproxima: Move path computation into a Locator service

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', '@config.factory']
+    arguments: ['@state', '@string_translation','@automatic_updates.beginner', '@automatic_updates.stager', '@automatic_updates.cleaner', '@automatic_updates.committer', '@event_dispatcher', '@automatic_updates.path_locator']
   automatic_updates.beginner:
     class: PhpTuf\ComposerStager\Domain\Beginner
     arguments:
@@ -73,7 +73,7 @@ services:
       - { name: event_subscriber }
   automatic_updates.staged_projects_validator:
     class: Drupal\automatic_updates\Validator\StagedProjectsValidator
-    arguments: [ '@string_translation', '@automatic_updates.updater' ]
+    arguments: ['@string_translation', '@automatic_updates.path_locator']
     tags:
       - { name: event_subscriber }
   automatic_updates.update_version_validator:
@@ -86,3 +86,7 @@ services:
     arguments: ['@automatic_updates.exec_finder']
     tags:
       - { name: event_subscriber }
+  automatic_updates.path_locator:
+    class: Drupal\automatic_updates\PathLocator
+    arguments:
+      - '@config.factory'

src/PathLocator.php

+<?php
+
+namespace Drupal\automatic_updates;
+
+use Composer\Autoload\ClassLoader;
+use Drupal\Component\FileSystem\FileSystem;
+use Drupal\Core\Config\ConfigFactoryInterface;
+
+/**
+ * Computes file system paths that are needed for automatic updates.
+ */
+class PathLocator {
+
+  /**
+   * The config factory service.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
+  /**
+   * Constructs a PathLocator object.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory service.
+   */
+  public function __construct(ConfigFactoryInterface $config_factory) {
+    $this->configFactory = $config_factory;
+  }
+
+  /**
+   * Returns the path of the active directory, which should be updated.
+   *
+   * @return string
+   *   The absolute path which should be updated.
+   */
+  public function getActiveDirectory(): string {
+    return $this->getProjectRoot();
+  }
+
+  /**
+   * Returns the path of the directory where updates should be staged.
+   *
+   * @return string
+   *   The absolute path of the directory where updates should be staged.
+   */
+  public function getStageDirectory(): string {
+    // 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() . DIRECTORY_SEPARATOR . '.automatic_updates_stage_' . $site_id;
+  }
+
+  /**
+   * Returns the absolute path of the project root.
+   *
+   * This is where the project-level composer.json should normally be found, and
+   * may or may not be the same path as the Drupal code base.
+   *
+   * @return string
+   *   The absolute path of the project root.
+   */
+  public function getProjectRoot(): string {
+    // Assume that the vendor directory is immediately below the project root.
+    return realpath($this->getVendorDirectory() . DIRECTORY_SEPARATOR . '..');
+  }
+
+  /**
+   * Returns the absolute path of the vendor directory.
+   *
+   * @return string
+   *   The absolute path of the vendor directory.
+   */
+  public function getVendorDirectory(): string {
+    $reflector = new \ReflectionClass(ClassLoader::class);
+    return dirname($reflector->getFileName(), 2);
+  }
+
+}

src/Updater.php

@@ -2,14 +2,11 @@
 
 namespace Drupal\automatic_updates;
 
-use Composer\Autoload\ClassLoader;
 use Drupal\automatic_updates\Event\PreCommitEvent;
 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;
@@ -77,11 +74,11 @@ class Updater {
   protected $eventDispatcher;
 
   /**
-   * The config factory service.
+   * The path locator service.
    *
-   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   * @var \Drupal\automatic_updates\PathLocator
    */
-  protected $configFactory;
+  protected $pathLocator;
 
   /**
    * Constructs an Updater object.
@@ -100,10 +97,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.
+   * @param \Drupal\automatic_updates\PathLocator $path_locator
+   *   The path locator service.
    */
-  public function __construct(StateInterface $state, TranslationInterface $translation, BeginnerInterface $beginner, StagerInterface $stager, CleanerInterface $cleaner, CommitterInterface $committer, EventDispatcherInterface $event_dispatcher, ConfigFactoryInterface $config_factory) {
+  public function __construct(StateInterface $state, TranslationInterface $translation, BeginnerInterface $beginner, StagerInterface $stager, CleanerInterface $cleaner, CommitterInterface $committer, EventDispatcherInterface $event_dispatcher, PathLocator $path_locator) {
     $this->state = $state;
     $this->beginner = $beginner;
     $this->stager = $stager;
@@ -111,31 +108,7 @@ class Updater {
     $this->committer = $committer;
     $this->setStringTranslation($translation);
     $this->eventDispatcher = $event_dispatcher;
-    $this->configFactory = $config_factory;
-  }
-
-  /**
-   * Gets the vendor directory.
-   *
-   * @return string
-   *   The absolute path for vendor directory.
-   */
-  private static function getVendorDirectory(): string {
-    $class_loader_reflection = new \ReflectionClass(ClassLoader::class);
-    return dirname($class_loader_reflection->getFileName(), 2);
-  }
-
-  /**
-   * Gets the stage directory.
-   *
-   * @return string
-   *   The absolute path for stage directory.
-   */
-  public function getStageDirectory(): string {
-    // 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;
+    $this->pathLocator = $path_locator;
   }
 
   /**
@@ -145,23 +118,13 @@ class Updater {
    *   TRUE if there is active update, otherwise FALSE.
    */
   public function hasActiveUpdate(): bool {
-    $staged_dir = $this->getStageDirectory();
+    $staged_dir = $this->pathLocator->getStageDirectory();
     if (is_dir($staged_dir) || $this->state->get(static::STATE_KEY)) {
       return TRUE;
     }
     return FALSE;
   }
 
-  /**
-   * Gets the active directory.
-   *
-   * @return string
-   *   The absolute path for active directory.
-   */
-  public function getActiveDirectory(): string {
-    return realpath(static::getVendorDirectory() . '/..');
-  }
-
   /**
    * Begins the update.
    *
@@ -184,7 +147,7 @@ class Updater {
     $stage_key = $this->createActiveStage($packages);
     /** @var \Drupal\automatic_updates\Event\PreStartEvent $event */
     $event = $this->dispatchUpdateEvent(new PreStartEvent($packages), AutomaticUpdatesEvents::PRE_START);
-    $this->beginner->begin(static::getActiveDirectory(), static::getStageDirectory(), $this->getExclusions($event));
+    $this->beginner->begin($this->pathLocator->getActiveDirectory(), $this->pathLocator->getStageDirectory(), $this->getExclusions($event));
     return $stage_key;
   }
 
@@ -210,7 +173,7 @@ class Updater {
    *   detecting the core package.
    */
   public function getCorePackageName(): string {
-    $composer = realpath(static::getVendorDirectory() . '/../composer.json');
+    $composer = realpath($this->pathLocator->getProjectRoot() . '/composer.json');
 
     if (empty($composer) || !file_exists($composer)) {
       throw new \RuntimeException("Could not find project-level composer.json");
@@ -239,7 +202,7 @@ class Updater {
    */
   private function getExclusions($event): array {
     $make_relative = function (string $path): string {
-      return str_replace($this->getActiveDirectory() . '/', '', $path);
+      return str_replace($this->pathLocator->getActiveDirectory() . '/', '', $path);
     };
     return array_map($make_relative, $event->getExcludedPaths());
   }
@@ -270,15 +233,16 @@ class Updater {
   public function commit(): void {
     /** @var \Drupal\automatic_updates\Event\PreCommitEvent $event */
     $event = $this->dispatchUpdateEvent(new PreCommitEvent(), AutomaticUpdatesEvents::PRE_COMMIT);
-    $this->committer->commit($this->getStageDirectory(), static::getActiveDirectory(), $this->getExclusions($event));
+    $this->committer->commit($this->pathLocator->getStageDirectory(), $this->pathLocator->getActiveDirectory(), $this->getExclusions($event));
   }
 
   /**
    * Cleans the current update.
    */
   public function clean(): void {
-    if (is_dir($this->getStageDirectory())) {
-      $this->cleaner->clean($this->getStageDirectory());
+    $stage_dir = $this->pathLocator->getStageDirectory();
+    if (is_dir($stage_dir)) {
+      $this->cleaner->clean($stage_dir);
     }
     $this->state->delete(static::STATE_KEY);
   }
@@ -293,7 +257,7 @@ class Updater {
    * @see \PhpTuf\ComposerStager\Domain\StagerInterface::stage()
    */
   protected function stageCommand(array $command): void {
-    $this->stager->stage($command, $this->getStageDirectory());
+    $this->stager->stage($command, $this->pathLocator->getStageDirectory());
   }
 
   /**

src/Validator/StagedProjectsValidator.php

@@ -5,7 +5,7 @@ namespace Drupal\automatic_updates\Validator;
 use Drupal\automatic_updates\AutomaticUpdatesEvents;
 use Drupal\automatic_updates\Event\UpdateEvent;
 use Drupal\automatic_updates\Exception\UpdateException;
-use Drupal\automatic_updates\Updater;
+use Drupal\automatic_updates\PathLocator;
 use Drupal\automatic_updates\Validation\ValidationResult;
 use Drupal\Component\Serialization\Json;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
@@ -20,23 +20,23 @@ final class StagedProjectsValidator implements EventSubscriberInterface {
   use StringTranslationTrait;
 
   /**
-   * The updater service.
+   * The path locator service.
    *
-   * @var \Drupal\automatic_updates\Updater
+   * @var \Drupal\automatic_updates\PathLocator
    */
-  protected $updater;
+  protected $pathLocator;
 
   /**
    * Constructs a StagedProjectsValidation object.
    *
    * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
    *   The translation service.
-   * @param \Drupal\automatic_updates\Updater $updater
-   *   The updater service.
+   * @param \Drupal\automatic_updates\PathLocator $path_locator
+   *   The path locator service.
    */
-  public function __construct(TranslationInterface $translation, Updater $updater) {
+  public function __construct(TranslationInterface $translation, PathLocator $path_locator) {
     $this->setStringTranslation($translation);
-    $this->updater = $updater;
+    $this->pathLocator = $path_locator;
   }
 
   /**
@@ -87,8 +87,8 @@ final class StagedProjectsValidator implements EventSubscriberInterface {
    */
   public function validateStagedProjects(UpdateEvent $event): void {
     try {
-      $active_packages = $this->getDrupalPackagesFromLockFile($this->updater->getActiveDirectory() . "/composer.lock");
-      $staged_packages = $this->getDrupalPackagesFromLockFile($this->updater->getStageDirectory() . "/composer.lock");
+      $active_packages = $this->getDrupalPackagesFromLockFile($this->pathLocator->getActiveDirectory() . "/composer.lock");
+      $staged_packages = $this->getDrupalPackagesFromLockFile($this->pathLocator->getStageDirectory() . "/composer.lock");
     }
     catch (UpdateException $e) {
       foreach ($e->getValidationResults() as $result) {

tests/modules/automatic_updates_test/src/AutomaticUpdatesTestServiceProvider.php

-<?php
-
-namespace Drupal\automatic_updates_test;
-
-use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\DependencyInjection\ServiceProviderBase;
-
-/**
- * Modifies service definitions for testing purposes.
- */
-class AutomaticUpdatesTestServiceProvider extends ServiceProviderBase {
-
-  /**
-   * {@inheritdoc}
-   */
-  public function alter(ContainerBuilder $container) {
-    parent::alter($container);
-
-    $service = 'automatic_updates.updater';
-    if ($container->hasDefinition($service)) {
-      $container->getDefinition($service)->setClass(TestUpdater::class);
-    }
-  }
-
-}

tests/modules/automatic_updates_test/src/TestUpdater.php

-<?php
-
-namespace Drupal\automatic_updates_test;
-
-use Drupal\automatic_updates\Updater;
-
-/**
- * A testing updater that allows arbitrary active and stage directories.
- */
-class TestUpdater extends Updater {
-
-  /**
-   * The active directory to use, if different from the default.
-   *
-   * @var string
-   */
-  public $activeDirectory;
-
-  /**
-   * The stage directory to use, if different from the default.
-   *
-   * @var string
-   */
-  public $stageDirectory;
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getActiveDirectory(): string {
-    return $this->activeDirectory ?: parent::getActiveDirectory();
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getStageDirectory(): string {
-    return $this->stageDirectory ?: parent::getStageDirectory();
-  }
-
-}

tests/src/Functional/ExclusionsTest.php

@@ -2,6 +2,8 @@
 
 namespace Drupal\Tests\automatic_updates\Functional;
 
+use Drupal\automatic_updates\PathLocator;
+use Drupal\automatic_updates\Updater;
 use Drupal\Core\Site\Settings;
 use Drupal\Tests\BrowserTestBase;
 
@@ -30,10 +32,22 @@ class ExclusionsTest extends BrowserTestBase {
   public function testExclusions(): void {
     $stage_dir = "$this->siteDirectory/stage";
 
-    /** @var \Drupal\automatic_updates_test\TestUpdater $updater */
-    $updater = $this->container->get('automatic_updates.updater');
-    $updater->activeDirectory = __DIR__ . '/../../fixtures/fake-site';
-    $updater->stageDirectory = $stage_dir;
+    /** @var \Drupal\automatic_updates\PathLocator|\Prophecy\Prophecy\ObjectProphecy $locator */
+    $locator = $this->prophesize(PathLocator::class);
+    $locator->getActiveDirectory()->willReturn(__DIR__ . '/../../fixtures/fake-site');
+    $locator->getStageDirectory()->willReturn($stage_dir);
+    $locator->getProjectRoot()->willReturn($this->getDrupalRoot());
+
+    $updater = new Updater(
+      $this->container->get('state'),
+      $this->container->get('string_translation'),
+      $this->container->get('automatic_updates.beginner'),
+      $this->container->get('automatic_updates.stager'),
+      $this->container->get('automatic_updates.cleaner'),
+      $this->container->get('automatic_updates.committer'),
+      $this->container->get('event_dispatcher'),
+      $locator->reveal()
+    );
 
     $settings = Settings::getAll();
     $settings['file_public_path'] = 'files/public';

tests/src/Unit/StagedProjectsValidatorTest.php

@@ -3,7 +3,7 @@
 namespace Drupal\Tests\automatic_updates\Unit;
 
 use Drupal\automatic_updates\Event\UpdateEvent;
-use Drupal\automatic_updates\Updater;
+use Drupal\automatic_updates\PathLocator;
 use Drupal\automatic_updates\Validator\StagedProjectsValidator;
 use Drupal\Component\FileSystem\FileSystem;
 use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
@@ -26,10 +26,10 @@ class StagedProjectsValidatorTest extends UnitTestCase {
     $active_dir = uniqid($prefix);
     $stage_dir = uniqid($prefix);
 
-    $updater = $this->prophesize(Updater::class);
-    $updater->getActiveDirectory()->willReturn($active_dir);
-    $updater->getStageDirectory()->willReturn($stage_dir);
-    $validator = new StagedProjectsValidator(new TestTranslationManager(), $updater->reveal());
+    $locator = $this->prophesize(PathLocator::class);
+    $locator->getActiveDirectory()->willReturn($active_dir);
+    $locator->getStageDirectory()->willReturn($stage_dir);
+    $validator = new StagedProjectsValidator(new TestTranslationManager(), $locator->reveal());
 
     $event = new UpdateEvent();
     $validator->validateStagedProjects($event);
@@ -56,13 +56,13 @@ class StagedProjectsValidatorTest extends UnitTestCase {
    * @covers ::validateStagedProjects
    */
   public function testErrors(string $fixtures_dir, string $expected_summary, array $expected_messages): void {
-    $updater = $this->prophesize(Updater::class);
+    $locator = $this->prophesize(PathLocator::class);
     $this->assertNotEmpty($fixtures_dir);
     $this->assertDirectoryExists($fixtures_dir);
 
-    $updater->getActiveDirectory()->willReturn("$fixtures_dir/active");
-    $updater->getStageDirectory()->willReturn("$fixtures_dir/staged");
-    $validator = new StagedProjectsValidator(new TestTranslationManager(), $updater->reveal());
+    $locator->getActiveDirectory()->willReturn("$fixtures_dir/active");
+    $locator->getStageDirectory()->willReturn("$fixtures_dir/staged");
+    $validator = new StagedProjectsValidator(new TestTranslationManager(), $locator->reveal());
     $event = new UpdateEvent();
     $validator->validateStagedProjects($event);
     $results = $event->getResults();
@@ -121,10 +121,10 @@ class StagedProjectsValidatorTest extends UnitTestCase {
    */
   public function testNoErrors(): void {
     $fixtures_dir = realpath(__DIR__ . '/../../fixtures/project_staged_validation/no_errors');
-    $updater = $this->prophesize(Updater::class);
-    $updater->getActiveDirectory()->willReturn("$fixtures_dir/active");
-    $updater->getStageDirectory()->willReturn("$fixtures_dir/staged");
-    $validator = new StagedProjectsValidator(new TestTranslationManager(), $updater->reveal());
+    $locator = $this->prophesize(PathLocator::class);
+    $locator->getActiveDirectory()->willReturn("$fixtures_dir/active");
+    $locator->getStageDirectory()->willReturn("$fixtures_dir/staged");
+    $validator = new StagedProjectsValidator(new TestTranslationManager(), $locator->reveal());
     $event = new UpdateEvent();
     $validator->validateStagedProjects($event);
     $results = $event->getResults();
@@ -137,10 +137,10 @@ class StagedProjectsValidatorTest extends UnitTestCase {
    */
   public function testNoLockFile(): void {
     $fixtures_dir = realpath(__DIR__ . '/../../fixtures/project_staged_validation/no_errors');
-    $updater = $this->prophesize(Updater::class);
-    $updater->getActiveDirectory()->willReturn("$fixtures_dir/active");
-    $updater->getStageDirectory()->willReturn("$fixtures_dir");
-    $validator = new StagedProjectsValidator(new TestTranslationManager(), $updater->reveal());
+    $locator = $this->prophesize(PathLocator::class);
+    $locator->getActiveDirectory()->willReturn("$fixtures_dir/active");
+    $locator->getStageDirectory()->willReturn("$fixtures_dir");
+    $validator = new StagedProjectsValidator(new TestTranslationManager(), $locator->reveal());
     $event = new UpdateEvent();
     $validator->validateStagedProjects($event);
     $results = $event->getResults();