From b068101a2383d660baa9845f4f06eaf1d45cac6a Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Wed, 9 Sep 2026 07:36:59 +0200 Subject: [PATCH] fix(extension-manager): stop re-checking for updates in the same process that ran composer update GlobalUpdateHandler, MinorUpdateHandler, and MajorUpdateHandler all run composer update in-process (ComposerAdapter runs Composer's own Application::run() directly, not a subprocess), then synchronously dispatch FlarumUpdated. ReCheckForUpdates' listener responds to that by immediately dispatching CheckForUpdates, which constructor-injects a Guzzle Client -- constructing it touches whatever HTTP handler class Guzzle picks, and if composer just bumped Guzzle itself to a version with a new class (e.g. GuzzleHttp\Handler\CurlVersion, added in 7.12.0), the still-running process's autoloader has no idea that class exists, and it fatals. This isn't fixable by making the recheck a queued job: Job\Dispatcher only defers to the queue when it isn't the sync driver, and Flarum defaults to SyncQueue for any install that hasn't configured Redis or a database queue -- which just runs the job immediately, in the same process, same as today. The one process boundary that's actually reliable regardless of queue config is a fresh request: the frontend already does a full window.location.reload() after a synchronous global update succeeds. So this skips the recheck entirely for FlarumUpdated (all three types share the exact same in-process-composer-then-synchronous-dispatch shape), while leaving the Updated event (single extension installs) untouched -- that path wasn't reported broken and is a narrower composer operation. LastUpdateRun still records a successful run; the limitedPackages diff it used to compute from the now-skipped check is left empty, since there's no safe way to compute it without touching Guzzle in this same process. --- .../src/Listener/ReCheckForUpdates.php | 31 +++----- .../tests/integration/api/MinorUpdateTest.php | 8 +- .../tests/unit/ReCheckForUpdatesTest.php | 75 +++++++++++++++++++ 3 files changed, 87 insertions(+), 27 deletions(-) create mode 100644 extensions/package-manager/tests/unit/ReCheckForUpdatesTest.php diff --git a/extensions/package-manager/src/Listener/ReCheckForUpdates.php b/extensions/package-manager/src/Listener/ReCheckForUpdates.php index 6e43e2162c..80aedb80f7 100644 --- a/extensions/package-manager/src/Listener/ReCheckForUpdates.php +++ b/extensions/package-manager/src/Listener/ReCheckForUpdates.php @@ -13,7 +13,6 @@ use Flarum\ExtensionManager\Command\CheckForUpdates; use Flarum\ExtensionManager\Event\FlarumUpdated; use Flarum\ExtensionManager\Extension\Event\Updated; -use Flarum\ExtensionManager\Settings\LastUpdateCheck; use Flarum\ExtensionManager\Settings\LastUpdateRun; class ReCheckForUpdates @@ -22,20 +21,14 @@ class ReCheckForUpdates * @var LastUpdateRun */ private $lastUpdateRun; - /** - * @var LastUpdateCheck - */ - private $lastUpdateCheck; - /** * @var Dispatcher */ private $bus; - public function __construct(LastUpdateRun $lastUpdateRun, LastUpdateCheck $lastUpdateCheck, Dispatcher $bus) + public function __construct(LastUpdateRun $lastUpdateRun, Dispatcher $bus) { $this->lastUpdateRun = $lastUpdateRun; - $this->lastUpdateCheck = $lastUpdateCheck; $this->bus = $bus; } @@ -44,25 +37,19 @@ public function __construct(LastUpdateRun $lastUpdateRun, LastUpdateCheck $lastU */ public function handle($event): void { - $previousUpdateCheck = $this->lastUpdateCheck->get(); - - $lastUpdateCheck = $this->bus->dispatch( - new CheckForUpdates($event->actor) - ); - if ($event instanceof FlarumUpdated) { - $mapPackageName = function (array $package) { - return $package['name']; - }; - - $previousPackages = array_map($mapPackageName, $previousUpdateCheck['updates']['installed']); - $lastPackages = array_map($mapPackageName, $lastUpdateCheck['updates']['installed']); - + // Composer replaced vendor files, so this process's stale autoloader cannot safely load new dependency classes. $this->lastUpdateRun ->for($event->type) ->with('status', LastUpdateRun::SUCCESS) - ->with('limitedPackages', array_intersect($previousPackages, $lastPackages)) + ->with('limitedPackages', []) ->save(); + + return; } + + $this->bus->dispatch( + new CheckForUpdates($event->actor) + ); } } diff --git a/extensions/package-manager/tests/integration/api/MinorUpdateTest.php b/extensions/package-manager/tests/integration/api/MinorUpdateTest.php index 3cf9ddd344..217c9605ae 100644 --- a/extensions/package-manager/tests/integration/api/MinorUpdateTest.php +++ b/extensions/package-manager/tests/integration/api/MinorUpdateTest.php @@ -81,14 +81,12 @@ public function can_update_with_latest_ext_incompatible_with_latest_core() /** @var LastUpdateRun $lastUpdateRun */ $lastUpdateRun = $this->app()->getContainer()->make(LastUpdateRun::class); + $lastMinorUpdateRun = $lastUpdateRun->for(FlarumUpdated::MINOR)->get(); $this->assertEquals(201, $response->getStatusCode()); $this->assertPackageVersion('flarum/tags', '*'); $this->assertPackageVersion('flarum/dummy-extension', '*'); - $this->assertEquals([ - 'flarum/core', - 'flarum/lang-english', - 'flarum/tags' - ], $lastUpdateRun->for(FlarumUpdated::MINOR)->get()['limitedPackages']); + $this->assertEquals(LastUpdateRun::SUCCESS, $lastMinorUpdateRun['status']); + $this->assertEquals([], $lastMinorUpdateRun['limitedPackages']); } } diff --git a/extensions/package-manager/tests/unit/ReCheckForUpdatesTest.php b/extensions/package-manager/tests/unit/ReCheckForUpdatesTest.php new file mode 100644 index 0000000000..6a1b64117a --- /dev/null +++ b/extensions/package-manager/tests/unit/ReCheckForUpdatesTest.php @@ -0,0 +1,75 @@ +createMock(SettingsRepositoryInterface::class); + $settings->expects($this->once()) + ->method('set') + ->with(LastUpdateRun::key(), $this->callback(function (string $value) use ($updateType): bool { + $lastUpdateRun = json_decode($value, true)[$updateType]; + + return $lastUpdateRun['status'] === LastUpdateRun::SUCCESS + && $lastUpdateRun['limitedPackages'] === []; + })); + + $bus = $this->createMock(Dispatcher::class); + $bus->expects($this->never())->method('dispatch'); + + $listener = new ReCheckForUpdates(new LastUpdateRun($settings), $bus); + $listener->handle(new FlarumUpdated(new User(), $updateType)); + } + + public static function coreUpdateTypes(): array + { + return [ + 'global update' => [FlarumUpdated::GLOBAL], + 'minor update' => [FlarumUpdated::MINOR], + 'major update' => [FlarumUpdated::MAJOR], + ]; + } + + #[Test] + public function extension_update_still_rechecks_for_updates(): void + { + $actor = new User(); + $bus = $this->createMock(Dispatcher::class); + $bus->expects($this->once()) + ->method('dispatch') + ->with($this->callback(function (CheckForUpdates $command) use ($actor): bool { + return $command->actor === $actor; + })) + ->willReturn([]); + + $listener = new ReCheckForUpdates( + $this->createStub(LastUpdateRun::class), + $bus + ); + $listener->handle(new Updated($actor, new Extension(__DIR__, ['name' => 'acme/example']))); + } +}