Issue #3332256 by tedbow, yash.rode, phenaproxima: package_manager_bypass...

Issue #3332256 by tedbow, yash.rode, phenaproxima: package_manager_bypass should *minimally* bypass php-tuf/composer-stager, not maximally

package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml

+services:
+  package_manager_bypass.beginner:
+    class: Drupal\package_manager_bypass\Beginner
+    arguments: ['@state', '@package_manager_bypass.beginner.inner' ]
+    decorates: 'package_manager.beginner'
+  package_manager_bypass.committer:
+    class: Drupal\package_manager_bypass\Committer
+    arguments: [ '@state', '@package_manager_bypass.committer.inner' ]
+    decorates: 'package_manager.committer'

package_manager/tests/modules/package_manager_bypass/src/Beginner.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\package_manager_bypass;
 
+use Drupal\Core\State\StateInterface;
 use Drupal\fixture_manipulator\StageFixtureManipulator;
 use PhpTuf\ComposerStager\Domain\Core\Beginner\BeginnerInterface;
 use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
@@ -12,10 +13,30 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface;
 
 /**
- * Defines an update beginner which doesn't do anything.
+ * Defines a service that decorates the Composer Stager beginner service.
  */
 class Beginner extends BypassedStagerServiceBase implements BeginnerInterface {
 
+  /**
+   * The decorated service.
+   *
+   * @var \PhpTuf\ComposerStager\Domain\Core\Beginner\BeginnerInterface
+   */
+  private $inner;
+
+  /**
+   * Constructs a Beginner object.
+   *
+   * @param \Drupal\Core\State\StateInterface $state
+   *   The state service.
+   * @param \PhpTuf\ComposerStager\Domain\Core\Beginner\BeginnerInterface $inner
+   *   The decorated beginner service.
+   */
+  public function __construct(StateInterface $state, BeginnerInterface $inner) {
+    $this->state = $state;
+    $this->inner = $inner;
+  }
+
   /**
    * A reference to the stage fixture manipulator, if any.
    *
@@ -31,7 +52,7 @@ class Beginner extends BypassedStagerServiceBase implements BeginnerInterface {
    */
   public function begin(PathInterface $activeDir, PathInterface $stagingDir, ?PathListInterface $exclusions = NULL, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT): void {
     $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions, $timeout);
-    $this->copyFixtureFilesTo($stagingDir);
+    $this->inner->begin($activeDir, $stagingDir, $exclusions, $callback, $timeout);
 
     /** @var \Drupal\fixture_manipulator\StageFixtureManipulator|null $stageManipulator */
     $stageManipulator = $this->state->get(__CLASS__ . '-stage-manipulator', NULL);

package_manager/tests/modules/package_manager_bypass/src/BypassedStagerServiceBase.php

@@ -4,10 +4,6 @@ declare(strict_types = 1);
 
 namespace Drupal\package_manager_bypass;
 
-use Drupal\Core\State\StateInterface;
-use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
-use Symfony\Component\Filesystem\Filesystem;
-
 /**
  * Records information about method invocations.
  *
@@ -24,59 +20,6 @@ abstract class BypassedStagerServiceBase {
    */
   protected $state;
 
-  /**
-   * The Symfony file system service.
-   *
-   * @var \Symfony\Component\Filesystem\Filesystem
-   */
-  protected $fileSystem;
-
-  /**
-   * Constructs an InvocationRecorderBase object.
-   *
-   * @param \Drupal\Core\State\StateInterface $state
-   *   The state service.
-   * @param \Symfony\Component\Filesystem\Filesystem $file_system
-   *   The Symfony file system service.
-   */
-  public function __construct(StateInterface $state, Filesystem $file_system) {
-    $this->state = $state;
-    $this->fileSystem = $file_system;
-  }
-
-  /**
-   * Sets a path to be mirrored into a destination by the main class method.
-   *
-   * @param string|null $path
-   *   A path to mirror into a destination directory when the main class method
-   *   is called, or NULL to disable.
-   *
-   * @see ::copyFixtureFilesTo()
-   */
-  public static function setFixturePath(?string $path): void {
-    \Drupal::state()->set(static::class . ' fixture', $path);
-  }
-
-  /**
-   * If a fixture path has been set, mirrors it to the given path.
-   *
-   * Files in the destination directory but not in the source directory will
-   * not be deleted.
-   *
-   * @param \PhpTuf\ComposerStager\Domain\Value\Path\PathInterface $destination
-   *   The path to which the fixture files should be mirrored.
-   */
-  protected function copyFixtureFilesTo(PathInterface $destination): void {
-    $fixture_path = $this->state->get(static::class . ' fixture');
-
-    if ($fixture_path && is_dir($fixture_path)) {
-      $this->fileSystem->mirror($fixture_path, $destination->resolve(), NULL, [
-        'override' => TRUE,
-        'delete' => FALSE,
-      ]);
-    }
-  }
-
   /**
    * Returns the arguments from every invocation of the main class method.
    *

package_manager/tests/modules/package_manager_bypass/src/Committer.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\package_manager_bypass;
 
+use Drupal\Core\State\StateInterface;
 use PhpTuf\ComposerStager\Domain\Core\Committer\CommitterInterface;
 use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
 use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ProcessRunnerInterface;
@@ -11,10 +12,30 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface;
 
 /**
- * Defines an update committer which doesn't do any actual committing.
+ * Defines a service that decorates the Composer Stager committer service.
  */
 class Committer extends BypassedStagerServiceBase implements CommitterInterface {
 
+  /**
+   * The decorated service.
+   *
+   * @var \PhpTuf\ComposerStager\Domain\Core\Committer\CommitterInterface
+   */
+  private $inner;
+
+  /**
+   * Constructs an Committer object.
+   *
+   * @param \Drupal\Core\State\StateInterface $state
+   *   The state service.
+   * @param \PhpTuf\ComposerStager\Domain\Core\Committer\CommitterInterface $inner
+   *   The decorated committer service.
+   */
+  public function __construct(StateInterface $state, CommitterInterface $inner) {
+    $this->state = $state;
+    $this->inner = $inner;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -23,17 +44,7 @@ class Committer extends BypassedStagerServiceBase implements CommitterInterface
     if ($exception = $this->state->get(static::class . '-exception')) {
       throw $exception;
     }
-    $this->copyFixtureFilesTo($activeDir);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public static function setFixturePath(?string $path): void {
-    // We haven't yet encountered a situation where we need the committer to
-    // copy fixture files to the active directory, but when we do, go ahead and
-    // remove this entire method.
-    throw new \BadMethodCallException('This is not implemented yet.');
+    $this->inner->commit($stagingDir, $activeDir, $exclusions, $callback, $timeout);
   }
 
   /**

package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php

@@ -8,7 +8,6 @@ use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\DependencyInjection\ServiceProviderBase;
 use Drupal\Core\Site\Settings;
 use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\Filesystem\Filesystem;
 
 /**
  * Defines services to bypass Package Manager's core functionality.
@@ -22,19 +21,8 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase {
     parent::alter($container);
 
     $state = new Reference('state');
-    $arguments = [
-      $state,
-      new Reference(Filesystem::class),
-    ];
     if (Settings::get('package_manager_bypass_composer_stager', TRUE)) {
-      $services = [
-        'package_manager.beginner' => Beginner::class,
-        'package_manager.stager' => Stager::class,
-        'package_manager.committer' => Committer::class,
-      ];
-      foreach ($services as $id => $class) {
-        $container->getDefinition($id)->setClass($class)->setArguments($arguments);
-      }
+      $container->getDefinition('package_manager.stager')->setClass(Stager::class)->setArguments([$state]);
     }
 
     $definition = $container->getDefinition('package_manager.path_locator')

package_manager/tests/modules/package_manager_bypass/src/Stager.php

@@ -5,6 +5,7 @@ declare(strict_types = 1);
 namespace Drupal\package_manager_bypass;
 
 use Composer\Json\JsonFile;
+use Drupal\Core\State\StateInterface;
 use PhpTuf\ComposerStager\Domain\Core\Stager\StagerInterface;
 use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
 use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ProcessRunnerInterface;
@@ -15,12 +16,21 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
  */
 class Stager extends BypassedStagerServiceBase implements StagerInterface {
 
+  /**
+   * Constructs a Stager object.
+   *
+   * @param \Drupal\Core\State\StateInterface $state
+   *   The state service.
+   */
+  public function __construct(StateInterface $state) {
+    $this->state = $state;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function stage(array $composerCommand, PathInterface $activeDir, PathInterface $stagingDir, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT): void {
     $this->saveInvocationArguments($composerCommand, $stagingDir, $timeout);
-    $this->copyFixtureFilesTo($stagingDir);
 
     // If desired, simulate a change to the lock file (e.g., as a result of
     // running `composer update`).

package_manager/tests/src/Kernel/FixtureManipulatorTest.php

@@ -379,13 +379,14 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
     // test project, it's located inside the project root.
     $this->assertTrue(chmod($path_locator->getVendorDirectory(), 0777));
     $this->assertTrue(chmod($path_locator->getProjectRoot(), 0777));
+    $stage_path = $path_locator->getStagingRoot() . '/stage' . $this->databasePrefix;
 
     // Simulate a stage beginning, which would commit the changes.
     // @see \Drupal\package_manager_bypass\Beginner::begin()
     $path_factory = new PathFactory();
     $this->container->get('package_manager.beginner')->begin(
       $path_factory->create($path_locator->getProjectRoot()),
-      $path_factory->create($path_locator->getStagingRoot()),
+      $path_factory->create($stage_path),
     );
   }
 

package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php

@@ -15,7 +15,6 @@ use Drupal\package_manager\UnusedConfigFactory;
 use Drupal\package_manager\Validator\DiskSpaceValidator;
 use Drupal\package_manager\Exception\StageValidationException;
 use Drupal\package_manager\Stage;
-use Drupal\package_manager_bypass\Beginner;
 use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait;
 use Drupal\Tests\package_manager\Traits\FixtureUtilityTrait;
 use Drupal\Tests\package_manager\Traits\ValidationTestTrait;
@@ -261,10 +260,6 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
     $path_locator = $this->container->get('package_manager.path_locator');
     $path_locator->setPaths($active_dir, $active_dir . '/vendor', '', $staging_root);
 
-    // Ensure the active directory will be copied into the test project's stage
-    // directory.
-    Beginner::setFixturePath($active_dir);
-
     // This validator will persist through container rebuilds.
     // @see ::register()
     $validator = new TestDiskSpaceValidator(

package_manager/tests/src/Kernel/StageOwnershipTest.php

@@ -10,7 +10,6 @@ use Drupal\package_manager\Event\PostDestroyEvent;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Exception\StageException;
 use Drupal\package_manager\Exception\StageOwnershipException;
-use Drupal\package_manager_bypass\Stager;
 use Drupal\package_manager_test_validation\EventSubscriber\TestSubscriber;
 use Drupal\Tests\user\Traits\UserCreationTrait;
 use ColinODell\PsrTestLogger\TestLogger;
@@ -198,12 +197,6 @@ class StageOwnershipTest extends PackageManagerKernelTestBase {
       'postApply' => [],
       'destroy' => [],
     ];
-    // Since we deliberately don't call create() on the stages we create as
-    // we loop through the life cycle methods, ensure that the active directory
-    // is mirrored into the stage directory when a package is required.
-    $active_dir = $this->container->get('package_manager.path_locator')
-      ->getProjectRoot();
-    Stager::setFixturePath($active_dir);
     foreach ($callbacks as $method => $arguments) {
       // Create a new stage instance for each method.
       $this->createStage()->claim($stage_id)->$method(...$arguments);

package_manager/tests/src/Kernel/StageTest.php

@@ -17,7 +17,6 @@ use Drupal\package_manager\Event\StageEvent;
 use Drupal\package_manager\Exception\ApplyFailedException;
 use Drupal\package_manager\Exception\StageException;
 use Drupal\package_manager\Stage;
-use Drupal\package_manager_bypass\Beginner;
 use Drupal\package_manager_bypass\Committer;
 use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
 use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
@@ -58,9 +57,6 @@ class StageTest extends PackageManagerKernelTestBase {
     $validator = $this->container->get('package_manager.validator.file_system');
     $this->container->get('event_dispatcher')->removeSubscriber($validator);
 
-    // Don't mirror the active directory from the test project.
-    Beginner::setFixturePath(NULL);
-
     /** @var \Drupal\package_manager_bypass\PathLocator $path_locator */
     $path_locator = $this->container->get('package_manager.path_locator');
 
@@ -104,54 +100,55 @@ class StageTest extends PackageManagerKernelTestBase {
    *   The test cases.
    */
   public function providerDestroyDuringApply(): array {
+    $error_message_while_being_applied = 'Cannot destroy the stage directory while it is being applied to the active directory.';
     return [
       'force destroy on pre-apply, fresh' => [
         PreApplyEvent::class,
         TRUE,
         1,
-        TRUE,
+        $error_message_while_being_applied,
       ],
       'destroy on pre-apply, fresh' => [
         PreApplyEvent::class,
         FALSE,
         1,
-        TRUE,
+        $error_message_while_being_applied,
       ],
       'force destroy on pre-apply, stale' => [
         PreApplyEvent::class,
         TRUE,
         7200,
-        FALSE,
+        'Stage directory does not exist',
       ],
       'destroy on pre-apply, stale' => [
         PreApplyEvent::class,
         FALSE,
         7200,
-        FALSE,
+        'Stage directory does not exist',
       ],
       'force destroy on post-apply, fresh' => [
         PostApplyEvent::class,
         TRUE,
         1,
-        TRUE,
+        $error_message_while_being_applied,
       ],
       'destroy on post-apply, fresh' => [
         PostApplyEvent::class,
         FALSE,
         1,
-        TRUE,
+        $error_message_while_being_applied,
       ],
       'force destroy on post-apply, stale' => [
         PostApplyEvent::class,
         TRUE,
         7200,
-        FALSE,
+        NULL,
       ],
       'destroy on post-apply, stale' => [
         PostApplyEvent::class,
         FALSE,
         7200,
-        FALSE,
+        NULL,
       ],
     ];
   }
@@ -166,12 +163,13 @@ class StageTest extends PackageManagerKernelTestBase {
    * @param int $time_offset
    *   How many simulated seconds should have elapsed between the PreApplyEvent
    *   being dispatched and the attempt to destroy the stage.
-   * @param bool $expect_exception
-   *   Whether or not destroying the stage will raise an exception.
+   * @param string|null $expected_exception_message
+   *   The expected exception message string if an exception is expected, or
+   *   NULL if no exception message was expected.
    *
    * @dataProvider providerDestroyDuringApply
    */
-  public function testDestroyDuringApply(string $event_class, bool $force, int $time_offset, bool $expect_exception): void {
+  public function testDestroyDuringApply(string $event_class, bool $force, int $time_offset, ?string $expected_exception_message): void {
     $listener = function (StageEvent $event) use ($force, $time_offset): void {
       // Simulate that a certain amount of time has passed since we started
       // applying staged changes. After a point, it should be possible to
@@ -183,15 +181,22 @@ class StageTest extends PackageManagerKernelTestBase {
       // simulate an attempt to destroy the stage while it's being applied, for
       // testing purposes.
       $event->getStage()->destroy($force);
+      // @see \PhpTuf\ComposerStager\Infrastructure\Service\Precondition\StagingDirDoesNotExist
+      Committer::setException(
+        new PreconditionException(
+          $this->prophesize(PreconditionInterface::class)->reveal(),
+          'Stage directory does not exist',
+        )
+      );
     };
     $this->addEventTestListener($listener, $event_class, 0);
 
     $stage = $this->createStage();
     $stage->create();
     $stage->require(['ext-json:*']);
-    if ($expect_exception) {
+    if ($expected_exception_message) {
       $this->expectException(StageException::class);
-      $this->expectExceptionMessage('Cannot destroy the stage directory while it is being applied to the active directory.');
+      $this->expectExceptionMessage($expected_exception_message);
     }
     $stage->apply();
 

tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php

@@ -6,7 +6,6 @@ namespace Drupal\Tests\automatic_updates\Functional;
 
 use Drupal\automatic_updates\CronUpdater;
 use Drupal\Core\Site\Settings;
-use Drupal\package_manager_bypass\Beginner;
 use Drupal\Tests\BrowserTestBase;
 use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait;
 use Drupal\Tests\package_manager\Traits\FixtureUtilityTrait;
@@ -206,7 +205,6 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
     // unique for each test run. This will enable changing files in the
     // directory and not affect other tests.
     $active_dir = $this->copyFixtureToTempDirectory($fixture_directory);
-    Beginner::setFixturePath($active_dir);
     $this->container->get('package_manager.path_locator')
       ->setPaths($active_dir, $active_dir . '/vendor', '', NULL);
   }