From 2ba399922434571f6da23ccd652b4050dabf8891 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Tue, 3 May 2022 14:10:19 +0000
Subject: [PATCH] Issue #3277014 by phenaproxima, lhridley, p.ayekumi,
capysara, percoction: Increase timeouts during create and apply
---
package_manager/src/Stage.php | 18 ++++++++++----
.../package_manager_bypass/src/Beginner.php | 2 +-
.../package_manager_bypass/src/Committer.php | 2 +-
.../tests/src/Kernel/StageTest.php | 24 +++++++++++++++++++
src/CronUpdater.php | 18 +++++++++-----
src/Updater.php | 8 +++++--
6 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index 229d2eb882..516d08f9d1 100644
--- a/package_manager/src/Stage.php
+++ b/package_manager/src/Stage.php
@@ -238,6 +238,11 @@ class Stage {
* call ::claim(). However, if it was created during another request, the
* stage must be claimed before operations can be performed on it.
*
+ * @param int|null $timeout
+ * (optional) How long to allow the file copying operation to run before
+ * timing out, in seconds, or NULL to never time out. Defaults to 300
+ * seconds.
+ *
* @return string
* Unique ID for the stage, which can be used to claim the stage before
* performing other operations on it. Calling code should store this ID for
@@ -248,7 +253,7 @@ class Stage {
*
* @see ::claim()
*/
- public function create(): string {
+ public function create(?int $timeout = 300): string {
if (!$this->isAvailable()) {
throw new StageException('Cannot create a new stage because one already exists.');
}
@@ -270,7 +275,7 @@ class Stage {
// available.
$this->dispatch($event, [$this, 'markAsAvailable']);
- $this->beginner->begin($active_dir, $stage_dir, $event->getExcludedPaths());
+ $this->beginner->begin($active_dir, $stage_dir, $event->getExcludedPaths(), NULL, $timeout);
$this->dispatch(new PostCreateEvent($this));
return $id;
}
@@ -313,8 +318,13 @@ class Stage {
/**
* Applies staged changes to the active directory.
+ *
+ * @param int|null $timeout
+ * (optional) How long to allow the file copying operation to run before
+ * timing out, in seconds, or NULL to never time out. Defaults to 600
+ * seconds.
*/
- public function apply(): void {
+ public function apply(?int $timeout = 600): void {
$this->checkOwnership();
$active_dir = $this->pathLocator->getProjectRoot();
@@ -331,7 +341,7 @@ class Stage {
$this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime());
$this->dispatch($event, $release_apply);
- $this->committer->commit($stage_dir, $active_dir, $event->getExcludedPaths());
+ $this->committer->commit($stage_dir, $active_dir, $event->getExcludedPaths(), NULL, $timeout);
// Rebuild the container and clear all caches, to ensure that new services
// are picked up.
diff --git a/package_manager/tests/modules/package_manager_bypass/src/Beginner.php b/package_manager/tests/modules/package_manager_bypass/src/Beginner.php
index e9ff25509f..5fc291a1ee 100644
--- a/package_manager/tests/modules/package_manager_bypass/src/Beginner.php
+++ b/package_manager/tests/modules/package_manager_bypass/src/Beginner.php
@@ -14,7 +14,7 @@ class Beginner extends InvocationRecorderBase implements BeginnerInterface {
* {@inheritdoc}
*/
public function begin(string $activeDir, string $stagingDir, ?array $exclusions = [], ?OutputCallbackInterface $callback = NULL, ?int $timeout = 120): void {
- $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions);
+ $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions, $timeout);
}
}
diff --git a/package_manager/tests/modules/package_manager_bypass/src/Committer.php b/package_manager/tests/modules/package_manager_bypass/src/Committer.php
index 2feb0d0f70..f5f40847e4 100644
--- a/package_manager/tests/modules/package_manager_bypass/src/Committer.php
+++ b/package_manager/tests/modules/package_manager_bypass/src/Committer.php
@@ -31,7 +31,7 @@ class Committer extends InvocationRecorderBase implements CommitterInterface {
* {@inheritdoc}
*/
public function commit(string $stagingDir, string $activeDir, ?array $exclusions = [], ?OutputCallbackInterface $callback = NULL, ?int $timeout = 120): void {
- $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions);
+ $this->saveInvocationArguments($stagingDir, $activeDir, $exclusions, $timeout);
}
/**
diff --git a/package_manager/tests/src/Kernel/StageTest.php b/package_manager/tests/src/Kernel/StageTest.php
index bf68cc8f39..5d029db83e 100644
--- a/package_manager/tests/src/Kernel/StageTest.php
+++ b/package_manager/tests/src/Kernel/StageTest.php
@@ -208,6 +208,30 @@ class StageTest extends PackageManagerKernelTestBase {
$stage->apply();
}
+ /**
+ * Tests that Composer Stager is invoked with a long timeout.
+ */
+ public function testTimeouts(): void {
+ $stage = $this->createStage();
+ $stage->create(420);
+ $stage->apply();
+
+ $timeouts = [
+ // The beginner was given an explicit timeout.
+ 'package_manager.beginner' => 420,
+ // The committer should have been called with a longer timeout than
+ // Composer Stager's default of 120 seconds.
+ 'package_manager.committer' => 600,
+ ];
+ foreach ($timeouts as $service_id => $expected_timeout) {
+ $invocations = $this->container->get($service_id)->getInvocationArguments();
+
+ // The service should have been called with the expected timeout.
+ $this->assertCount(1, $invocations);
+ $this->assertSame($expected_timeout, end($invocations[0]));
+ }
+ }
+
}
/**
diff --git a/src/CronUpdater.php b/src/CronUpdater.php
index 34bd9fa608..c78b9954ea 100644
--- a/src/CronUpdater.php
+++ b/src/CronUpdater.php
@@ -76,15 +76,20 @@ class CronUpdater extends Updater {
/**
* Handles updates during cron.
+ *
+ * @param int|null $timeout
+ * (optional) How long to allow the file copying operation to run before
+ * timing out, in seconds, or NULL to never time out. Defaults to 300
+ * seconds.
*/
- public function handleCron(): void {
+ public function handleCron(?int $timeout = 300): void {
if ($this->getMode() === static::DISABLED) {
return;
}
$next_release = $this->releaseChooser->getLatestInInstalledMinor();
if ($next_release) {
- $this->performUpdate($next_release->getVersion());
+ $this->performUpdate($next_release->getVersion(), $timeout);
}
}
@@ -93,8 +98,11 @@ class CronUpdater extends Updater {
*
* @param string $update_version
* The version to which to update.
+ * @param int|null $timeout
+ * How long to allow the operation to run before timing out, in seconds, or
+ * NULL to never time out.
*/
- private function performUpdate(string $update_version): void {
+ private function performUpdate(string $update_version, ?int $timeout): void {
$installed_version = (new ProjectInfo('drupal'))->getInstalledVersion();
if (empty($installed_version)) {
$this->logger->error('Unable to determine the current version of Drupal core.');
@@ -105,9 +113,7 @@ class CronUpdater extends Updater {
// handle any exceptions or validation errors consistently, and destroy the
// stage regardless of whether the update succeeds.
try {
- $this->begin([
- 'drupal' => $update_version,
- ]);
+ $this->begin(['drupal' => $update_version], $timeout);
$this->stage();
$this->apply();
diff --git a/src/Updater.php b/src/Updater.php
index c333f6c376..83fe7b3dfe 100644
--- a/src/Updater.php
+++ b/src/Updater.php
@@ -22,6 +22,10 @@ class Updater extends Stage {
*
* @param string[] $project_versions
* The versions of the packages to update to, keyed by package name.
+ * @param int|null $timeout
+ * (optional) How long to allow the file copying operation to run before
+ * timing out, in seconds, or NULL to never time out. Defaults to 300
+ * seconds.
*
* @return string
* The unique ID of the stage.
@@ -29,7 +33,7 @@ class Updater extends Stage {
* @throws \InvalidArgumentException
* Thrown if no project version for Drupal core is provided.
*/
- public function begin(array $project_versions): string {
+ public function begin(array $project_versions, ?int $timeout = 300): string {
if (count($project_versions) !== 1 || !array_key_exists('drupal', $project_versions)) {
throw new \InvalidArgumentException("Currently only updates to Drupal core are supported.");
}
@@ -54,7 +58,7 @@ class Updater extends Stage {
$this->tempStore->set(static::TEMPSTORE_METADATA_KEY, [
'packages' => $package_versions,
]);
- return $this->create();
+ return $this->create($timeout);
}
/**
--
GitLab