From c9d276344217050906538d7ecce5b2ed4473b0f8 Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Sat, 9 Dec 2023 15:25:14 +0100 Subject: [PATCH] fix: issues with job failure and unique runs --- extensions/package-manager/extend.php | 2 +- .../js/src/admin/components/QueueSection.tsx | 7 ++++--- extensions/package-manager/js/src/admin/index.tsx | 2 +- extensions/package-manager/locale/en.yml | 3 ++- .../src/Command/CheckForUpdatesHandler.php | 7 ++++--- .../package-manager/src/Job/ComposerCommandJob.php | 10 +++++++--- extensions/package-manager/src/Job/Dispatcher.php | 2 ++ extensions/package-manager/src/Task/Task.php | 12 ++++++++++-- 8 files changed, 31 insertions(+), 14 deletions(-) diff --git a/extensions/package-manager/extend.php b/extensions/package-manager/extend.php index 114034d28e..ead41be9b2 100755 --- a/extensions/package-manager/extend.php +++ b/extensions/package-manager/extend.php @@ -50,7 +50,7 @@ ->default(Settings\LastUpdateRun::key(), json_encode(Settings\LastUpdateRun::default())) ->default('flarum-package-manager.queue_jobs', false) ->default('flarum-package-manager.minimum_stability', 'stable') - ->default('flarum-package-manager.task_retention_days', 30), + ->default('flarum-package-manager.task_retention_days', 7), (new Extend\ServiceProvider) ->register(PackageManagerServiceProvider::class), diff --git a/extensions/package-manager/js/src/admin/components/QueueSection.tsx b/extensions/package-manager/js/src/admin/components/QueueSection.tsx index 7a96575ad7..9be56e1bd1 100644 --- a/extensions/package-manager/js/src/admin/components/QueueSection.tsx +++ b/extensions/package-manager/js/src/admin/components/QueueSection.tsx @@ -8,6 +8,7 @@ import { Extension } from 'flarum/admin/AdminApplication'; import icon from 'flarum/common/helpers/icon'; import ItemList from 'flarum/common/utils/ItemList'; import extractText from 'flarum/common/utils/extractText'; +import Link from 'flarum/common/components/Link'; import Label from './Label'; import TaskOutputModal from './TaskOutputModal'; @@ -73,7 +74,7 @@ export default class QueueSection extends Component<{}> { const extension: Extension | null = app.data.extensions[task.package()?.replace(/(\/flarum-|\/flarum-ext-|\/)/g, '-')]; return extension ? ( -
+
{!!extension.icon && icon(extension.icon.name)}
@@ -81,7 +82,7 @@ export default class QueueSection extends Component<{}> { {extension.extra['flarum-extension'].title} {task.package()}
- + ) : ( task.package() ); @@ -114,7 +115,7 @@ export default class QueueSection extends Component<{}> { { label: extractText(app.translator.trans('flarum-package-manager.admin.sections.queue.columns.elapsed_time')), content: (task) => - !task.startedAt() ? ( + !task.startedAt() || !task.finishedAt() ? ( app.translator.trans('flarum-package-manager.admin.sections.queue.task_just_started') ) : ( diff --git a/extensions/package-manager/js/src/admin/index.tsx b/extensions/package-manager/js/src/admin/index.tsx index f7b4aad557..8817e770b5 100755 --- a/extensions/package-manager/js/src/admin/index.tsx +++ b/extensions/package-manager/js/src/admin/index.tsx @@ -76,7 +76,7 @@ app.initializers.add('flarum-package-manager', (app) => { }); }} > - Remove + {app.translator.trans('flarum-package-manager.admin.extensions.remove')} ); }); diff --git a/extensions/package-manager/locale/en.yml b/extensions/package-manager/locale/en.yml index 295d5775a3..7ee7a516c9 100755 --- a/extensions/package-manager/locale/en.yml +++ b/extensions/package-manager/locale/en.yml @@ -28,6 +28,7 @@ flarum-package-manager: install: Install a new extension install_help: Fill in the extension package name to proceed. Visit {extiverse} to browse extensions. proceed: Proceed + remove: Uninstall successful_install: "{extension} was installed successfully, redirecting.." successful_remove: Extension removed successfully. successful_update: "{extension} was updated successfully, redirecting.." @@ -59,7 +60,7 @@ flarum-package-manager: columns: details: Details elapsed_time: Completed in - peak_memory_used: Maximum Memory Used + peak_memory_used: Peak Memory Usage operation: Operation package: Package status: Status diff --git a/extensions/package-manager/src/Command/CheckForUpdatesHandler.php b/extensions/package-manager/src/Command/CheckForUpdatesHandler.php index 8cd96d1d0c..9a18d789de 100755 --- a/extensions/package-manager/src/Command/CheckForUpdatesHandler.php +++ b/extensions/package-manager/src/Command/CheckForUpdatesHandler.php @@ -56,9 +56,10 @@ public function handle(CheckForUpdates $command) $firstOutput = $this->runComposerCommand(false, $command); $firstOutput = json_decode($this->cleanJson($firstOutput), true); + $installed = $firstOutput['installed'] ?? []; $majorUpdates = false; - foreach ($firstOutput['installed'] as $package) { + foreach ($installed as $package) { if (isset($package['latest-status']) && $package['latest-status'] === 'update-possible' && Util::isMajorUpdate($package['version'], $package['latest'])) { $majorUpdates = true; break; @@ -74,7 +75,7 @@ public function handle(CheckForUpdates $command) $secondOutput = ['installed' => []]; } - foreach ($firstOutput['installed'] as &$mainPackageUpdate) { + foreach ($installed as &$mainPackageUpdate) { $mainPackageUpdate['latest-minor'] = $mainPackageUpdate['latest-major'] = null; if ($mainPackageUpdate['latest-status'] === 'up-to-date') { @@ -97,7 +98,7 @@ public function handle(CheckForUpdates $command) } return $this->lastUpdateCheck - ->with('installed', $firstOutput['installed']) + ->with('installed', $installed) ->save(); } diff --git a/extensions/package-manager/src/Job/ComposerCommandJob.php b/extensions/package-manager/src/Job/ComposerCommandJob.php index b3d116ca98..21d1485688 100644 --- a/extensions/package-manager/src/Job/ComposerCommandJob.php +++ b/extensions/package-manager/src/Job/ComposerCommandJob.php @@ -13,10 +13,11 @@ use Flarum\PackageManager\Command\AbstractActionCommand; use Flarum\PackageManager\Composer\ComposerAdapter; use Flarum\Queue\AbstractJob; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Queue\Middleware\WithoutOverlapping; use Throwable; -class ComposerCommandJob extends AbstractJob +class ComposerCommandJob extends AbstractJob implements ShouldBeUnique { /** * @var AbstractActionCommand @@ -56,11 +57,14 @@ public function abort(Throwable $exception) } $this->command->task->end(false); + } - $this->fail($exception); + public function failed(Throwable $exception): void + { + $this->command->task->end(false); } - public function middleware() + public function middleware(): array { return [ new WithoutOverlapping(), diff --git a/extensions/package-manager/src/Job/Dispatcher.php b/extensions/package-manager/src/Job/Dispatcher.php index 3f1722d1f8..d0ce3ecbf6 100644 --- a/extensions/package-manager/src/Job/Dispatcher.php +++ b/extensions/package-manager/src/Job/Dispatcher.php @@ -68,6 +68,8 @@ public function dispatch(AbstractActionCommand $command): DispatcherResponse { $queueJobs = ($this->runSyncOverride === false) || ($this->runSyncOverride !== true && $this->settings->get('flarum-package-manager.queue_jobs')); + // @todo: check and skip if there is already a pending or running task. + if ($queueJobs && (! $this->queue instanceof SyncQueue)) { $task = Task::build($command->getOperationName(), $command->package ?? null); diff --git a/extensions/package-manager/src/Task/Task.php b/extensions/package-manager/src/Task/Task.php index 9ce2f16d70..fb90e4e1ce 100644 --- a/extensions/package-manager/src/Task/Task.php +++ b/extensions/package-manager/src/Task/Task.php @@ -20,8 +20,8 @@ * @property string $package * @property string $output * @property Carbon $created_at - * @property Carbon $started_at - * @property Carbon $finished_at + * @property Carbon|null $started_at + * @property Carbon|null $finished_at * @property float $peak_memory_used */ class Task extends AbstractModel @@ -84,6 +84,14 @@ public function start(): bool public function end(bool $success): bool { + if ($this->finished_at) { + return true; + } + + if (! $this->started_at) { + $this->start(); + } + $this->status = $success ? static::SUCCESS : static::FAILURE; $this->finished_at = Carbon::now(); $this->peak_memory_used = round(memory_get_peak_usage() / 1024);