From 21942a75e642c370ceef10e5e49d74891f3f1306 Mon Sep 17 00:00:00 2001 From: hardcorefactions <108526035+hardcorefactions@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:20:11 +0200 Subject: [PATCH 1/2] fix modrinth downloads and installed plugins order --- minecraft-modrinth/lang/de/strings.php | 2 + minecraft-modrinth/lang/en/strings.php | 2 + .../src/Facades/MinecraftModrinth.php | 5 + .../Pages/MinecraftModrinthProjectPage.php | 156 +++---- .../src/Services/MinecraftModrinthService.php | 415 +++++++++++++----- 5 files changed, 397 insertions(+), 183 deletions(-) diff --git a/minecraft-modrinth/lang/de/strings.php b/minecraft-modrinth/lang/de/strings.php index f2d461fb..83c42741 100644 --- a/minecraft-modrinth/lang/de/strings.php +++ b/minecraft-modrinth/lang/de/strings.php @@ -65,6 +65,8 @@ 'update_failed_body' => 'Bei der Aktualisierung ist ein Fehler aufgetreten. Bitte versuche es erneut oder wende dich an den Support, wenn das Problem weiterhin besteht.', 'uninstall_success' => 'Deinstallation abgeschlossen', 'uninstall_success_body' => ':name erfolgreich deinstalliert', + 'uninstall_partial' => 'Deinstallation unvollständig', + 'uninstall_partial_body' => 'Die Datei von :name wurde gelöscht, konnte aber nicht aus der Liste der installierten Mods/Plugins entfernt werden. Sie wird eventuell weiterhin als installiert angezeigt.', 'uninstall_failed' => 'Deinstallation fehlgeschlagen', 'uninstall_failed_body' => 'Bei der Deinstallation ist ein Fehler aufgetreten. Bitte versuche es erneut oder wende dich an den Support, wenn das Problem weiterhin besteht.', ], diff --git a/minecraft-modrinth/lang/en/strings.php b/minecraft-modrinth/lang/en/strings.php index daf30eac..c2d359c5 100644 --- a/minecraft-modrinth/lang/en/strings.php +++ b/minecraft-modrinth/lang/en/strings.php @@ -65,6 +65,8 @@ 'update_failed_body' => 'An error occurred during the update. Please try again or contact support if the issue persists.', 'uninstall_success' => 'Uninstall completed', 'uninstall_success_body' => 'Successfully uninstalled :name', + 'uninstall_partial' => 'Uninstall incomplete', + 'uninstall_partial_body' => 'The file for :name was deleted, but it could not be removed from the installed list. It may still appear as installed.', 'uninstall_failed' => 'Uninstall failed', 'uninstall_failed_body' => 'An error occurred during uninstallation. Please try again or contact support if the issue persists.', ], diff --git a/minecraft-modrinth/src/Facades/MinecraftModrinth.php b/minecraft-modrinth/src/Facades/MinecraftModrinth.php index 525c5930..f922a01f 100644 --- a/minecraft-modrinth/src/Facades/MinecraftModrinth.php +++ b/minecraft-modrinth/src/Facades/MinecraftModrinth.php @@ -18,9 +18,14 @@ * @method static array{hits: array>, total_hits: int} getProjects(Server $server, ModrinthProjectType $modrinthProjectType, int $page = 1, ?string $search = null) * @method static array> getInstalledModsFromModrinth(array $installedMods, int $page = 1) * @method static array getProjectVersions(string $projectId, Server $server) + * @method static array> getProjectVersionsBulk(array $projectIds, Server $server) * @method static array getInstalledModsMetadata(Server $server, ModrinthProjectType $modrinthProjectType) * @method static bool saveModMetadata(Server $server, ModrinthProjectType $modrinthProjectType, string $projectId, string $projectSlug, string $projectTitle, string $versionId, string $versionNumber, string $filename, ?string $author = null) * @method static bool removeModMetadata(Server $server, ModrinthProjectType $modrinthProjectType, string $projectId) + * @method static void downloadFile(Server $server, string $url, string $folder, string $filename) + * @method static void deleteFile(Server $server, string $folder, string $filename) + * @method static bool fileExists(Server $server, string $folder, string $filename) + * @method static array listFolder(Server $server, string $folder) * @method static InstalledModMetadata|null getInstalledMod(Server $server, ModrinthProjectType $modrinthProjectType, string $projectId) * @method static bool isUpdateAvailable(array{version_id: string, version_number: string} $installedMod, array $availableVersions) * @method static array getInstalledMods(Server $server, ModrinthProjectType $modrinthProjectType) diff --git a/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php b/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php index 8ebdc5c2..3fbeb053 100644 --- a/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php +++ b/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php @@ -4,7 +4,6 @@ use App\Filament\Server\Resources\Files\Pages\ListFiles; use App\Models\Server; -use App\Repositories\Daemon\DaemonFileRepository; use App\Traits\Filament\BlockAccessInConflict; use Boy132\MinecraftModrinth\Enums\ModrinthProjectType; use Boy132\MinecraftModrinth\Facades\MinecraftModrinth; @@ -27,7 +26,6 @@ use Filament\Tables\Table; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Support\Carbon; -use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\HtmlString; @@ -132,6 +130,42 @@ protected function getCachedVersions(string $projectId): array return $this->versionsCache[$projectId]; } + /** + * Fetch the versions of every project on the current page up front. + * + * Each installed row asks for its versions to decide between the "installed" and "update" + * action, so without this the installed tab does one Modrinth request per row, one after + * the other, and a full page can take longer than the request is allowed to run. + * + * @param array $projectIds + */ + protected function primeVersionsCache(array $projectIds): void + { + $projectIds = array_values(array_diff(array_unique(array_filter($projectIds)), array_keys($this->versionsCache))); + + if (count($projectIds) < 2) { + return; + } + + /** @var Server $server */ + $server = Filament::getTenant(); + + $this->versionsCache += MinecraftModrinth::getProjectVersionsBulk($projectIds, $server); + } + + /** + * Drop everything read from the server so the next render reflects what is actually on disk. + */ + protected function forgetInstalledState(bool $refresh = true): void + { + $this->installedModsMetadata = null; + $this->versionsCache = []; + + if ($refresh) { + $this->js('$wire.$refresh()'); + } + } + /** * @param array $files * @return array{primary: bool, filename: string, url: string}|null @@ -174,17 +208,12 @@ private function performInstallOrUpdate( array $primaryFile, ?array $installedMod = null ): void { - $fileRepository = app(DaemonFileRepository::class); - $safeNewFilename = $this->validateFilename($primaryFile['filename']); $oldFilename = $installedMod ? $this->validateFilename($installedMod['filename']) : null; $folder = static::$modrinthProjectType->getFolder(); - $fileRepository - ->setServer($server) - ->pull($primaryFile['url'], $folder) - ->throw(); + MinecraftModrinth::downloadFile($server, $primaryFile['url'], $folder, $safeNewFilename); $saved = MinecraftModrinth::saveModMetadata( $server, @@ -201,12 +230,7 @@ private function performInstallOrUpdate( if (!$saved) { if (!$oldFilename || $oldFilename !== $safeNewFilename) { try { - Http::daemon($server->node) - ->post("/api/servers/{$server->uuid}/files/delete", [ - 'root' => '/', - 'files' => [$folder . '/' . $safeNewFilename], - ]) - ->throw(); + MinecraftModrinth::deleteFile($server, $folder, $safeNewFilename); } catch (Exception $rollbackException) { report($rollbackException); } @@ -217,20 +241,10 @@ private function performInstallOrUpdate( if ($oldFilename && $oldFilename !== $safeNewFilename) { try { - Http::daemon($server->node) - ->post("/api/servers/{$server->uuid}/files/delete", [ - 'root' => '/', - 'files' => [$folder . '/' . $oldFilename], - ]) - ->throw(); + MinecraftModrinth::deleteFile($server, $folder, $oldFilename); } catch (Exception $deleteException) { try { - Http::daemon($server->node) - ->post("/api/servers/{$server->uuid}/files/delete", [ - 'root' => '/', - 'files' => [$folder . '/' . $safeNewFilename], - ]) - ->throw(); + MinecraftModrinth::deleteFile($server, $folder, $safeNewFilename); } catch (Exception $rollbackException) { report($rollbackException); } @@ -275,12 +289,21 @@ public function table(Table $table): Table })); } + // Sort by title so a row keeps its place instead of moving around whenever its + // metadata entry is rewritten by an install, update or failed update. + usort($installedMods, fn (array $a, array $b) => strcasecmp($a['project_title'], $b['project_title'])); + $projects = MinecraftModrinth::getInstalledModsFromModrinth($installedMods, $page); + $this->primeVersionsCache(array_column($projects, 'project_id')); + return new LengthAwarePaginator($projects, count($installedMods), 20, $page); } else { $response = MinecraftModrinth::getProjects($server, static::$modrinthProjectType, $page, $search); + $installedIds = array_column($this->getInstalledModsMetadata(), 'project_id'); + $this->primeVersionsCache(array_intersect(array_column($response['hits'], 'project_id'), $installedIds)); + return new LengthAwarePaginator($response['hits'], $response['total_hits'], 20, $page); } }) @@ -383,9 +406,7 @@ public function table(Table $table): Table $this->performInstallOrUpdate($server, $record, $versionData, $primaryFile, $installedMod); - $this->installedModsMetadata = null; - $this->versionsCache = []; - $this->js('$wire.$refresh()'); + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.install_success')) @@ -398,9 +419,7 @@ public function table(Table $table): Table } catch (Exception $exception) { report($exception); - $this->installedModsMetadata = null; - $this->versionsCache = []; - $this->js('$wire.$refresh()'); + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.install_failed')) @@ -459,8 +478,7 @@ public function table(Table $table): Table $this->performInstallOrUpdate($server, $record, $latestVersion, $primaryFile); - $this->installedModsMetadata = null; - $this->versionsCache = []; + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.install_success')) @@ -473,8 +491,7 @@ public function table(Table $table): Table } catch (Exception $exception) { report($exception); - $this->installedModsMetadata = null; - $this->versionsCache = []; + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.install_failed')) @@ -541,8 +558,7 @@ public function table(Table $table): Table $this->performInstallOrUpdate($server, $record, $latestVersion, $primaryFile, $installedMod); - $this->installedModsMetadata = null; - $this->versionsCache = []; + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.update_success')) @@ -554,8 +570,7 @@ public function table(Table $table): Table } catch (Exception $exception) { report($exception); - $this->installedModsMetadata = null; - $this->versionsCache = []; + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.update_failed')) @@ -611,35 +626,29 @@ public function table(Table $table): Table $folder = static::$modrinthProjectType->getFolder(); - Http::daemon($server->node) - ->post("/api/servers/{$server->uuid}/files/delete", [ - 'root' => '/', - 'files' => [$folder . '/' . $safeFilename], - ]) - ->throw(); + MinecraftModrinth::deleteFile($server, $folder, $safeFilename); $metadataRemoved = MinecraftModrinth::removeModMetadata($server, static::$modrinthProjectType, $record['project_id']); + $this->forgetInstalledState(); + if (!$metadataRemoved) { Log::warning('Failed to remove mod metadata after successful file deletion', [ 'project_id' => $record['project_id'], 'server_id' => $server->id, ]); - if (is_array($this->installedModsMetadata)) { - $this->installedModsMetadata = array_values( - array_filter($this->installedModsMetadata, fn ($mod) => $mod['project_id'] !== $record['project_id']) - ); - } - - unset($this->versionsCache[$record['project_id']]); - } else { - $this->installedModsMetadata = null; - $this->versionsCache = []; - } - - if ($this->activeTab === 'installed') { - $this->js('$wire.$refresh()'); + // The jar is gone but the metadata file still lists it, so the row + // stays. Say so instead of reporting a clean uninstall. + Notification::make() + ->title(trans('minecraft-modrinth::strings.notifications.uninstall_partial')) + ->body(trans('minecraft-modrinth::strings.notifications.uninstall_partial_body', [ + 'name' => $record['title'], + ])) + ->warning() + ->send(); + + return; } Notification::make() @@ -652,12 +661,7 @@ public function table(Table $table): Table } catch (Exception $exception) { report($exception); - $this->installedModsMetadata = null; - $this->versionsCache = []; - - if ($this->activeTab === 'installed') { - $this->js('$wire.$refresh()'); - } + $this->forgetInstalledState(); Notification::make() ->title(trans('minecraft-modrinth::strings.notifications.uninstall_failed')) @@ -703,23 +707,9 @@ public function content(Schema $schema): Schema ->badge(), TextEntry::make('installed') ->label(fn () => trans('minecraft-modrinth::strings.page.installed', ['type' => static::$modrinthProjectType?->getLabel() ?? 'Modrinth'])) - ->state(function (DaemonFileRepository $fileRepository) use ($server) { - try { - $files = $fileRepository->setServer($server)->getDirectory(static::$modrinthProjectType->getFolder()); - - if (isset($files['error'])) { - throw new Exception($files['error']); - } - - return collect($files) - ->filter(fn ($file) => $file['mime'] === 'application/jar' || str($file['name'])->lower()->endsWith('.jar')) - ->count(); - } catch (Exception $exception) { - report($exception); - - return trans('minecraft-modrinth::strings.page.unknown'); - } - }) + ->state(fn () => collect(MinecraftModrinth::listFolder($server, static::$modrinthProjectType->getFolder())) + ->filter(fn ($file) => ($file['mime'] ?? null) === 'application/jar' || str($file['name'] ?? '')->lower()->endsWith('.jar')) + ->count()) ->badge(), ]), $this->getTabsContentComponent(), diff --git a/minecraft-modrinth/src/Services/MinecraftModrinthService.php b/minecraft-modrinth/src/Services/MinecraftModrinthService.php index 656909da..c9be310e 100644 --- a/minecraft-modrinth/src/Services/MinecraftModrinthService.php +++ b/minecraft-modrinth/src/Services/MinecraftModrinthService.php @@ -6,11 +6,17 @@ use App\Repositories\Daemon\DaemonFileRepository; use Boy132\MinecraftModrinth\Enums\ModrinthProjectType; use Exception; +use Illuminate\Contracts\Filesystem\FileNotFoundException; +use Illuminate\Http\Client\Pool; +use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; class MinecraftModrinthService { + // Seconds to wait for the daemon while it downloads a file for us. + protected const DOWNLOAD_TIMEOUT = 120; + public function getMinecraftVersion(Server $server): ?string { $version = $server->variables()->where(fn ($builder) => $builder->where('env_variable', 'MINECRAFT_VERSION')->orWhere('env_variable', 'MC_VERSION'))->first()?->server_value; @@ -119,26 +125,34 @@ public function getProjects(Server $server, ModrinthProjectType $modrinthProject if ($search) { $data['query'] = $search; - $key .= ":$search"; + $key .= ':'.md5($search); } - return cache()->remember($key, now()->addMinutes(30), function () use ($data) { - try { - return Http::asJson() - ->timeout(5) - ->connectTimeout(5) - ->throw() - ->get('https://api.modrinth.com/v2/search', $data) - ->json(); - } catch (Exception $exception) { - report($exception); + $cached = cache()->get($key); + if (is_array($cached)) { + return $cached; + } - return [ - 'hits' => [], - 'total_hits' => 0, - ]; - } - }); + try { + $response = Http::asJson() + ->timeout(5) + ->connectTimeout(5) + ->throw() + ->get('https://api.modrinth.com/v2/search', $data) + ->json(); + } catch (Exception $exception) { + report($exception); + + // not caching on purpose since caching a failure would keep the page empty for 30 minutes + return [ + 'hits' => [], + 'total_hits' => 0, + ]; + } + + cache()->put($key, $response, now()->addMinutes(30)); + + return $response; } /** @@ -151,7 +165,14 @@ public function getInstalledModsFromModrinth(array $installedMods, int $page = 1 return []; } - $projectIds = collect($installedMods)->pluck('project_id')->unique()->values()->all(); + $installedModsById = []; + foreach ($installedMods as $mod) { + if (!isset($installedModsById[$mod['project_id']])) { + $installedModsById[$mod['project_id']] = $mod; + } + } + + $projectIds = array_keys($installedModsById); $perPage = 20; $offset = ($page - 1) * $perPage; @@ -162,9 +183,13 @@ public function getInstalledModsFromModrinth(array $installedMods, int $page = 1 } $idsParam = json_encode($pageIds, JSON_THROW_ON_ERROR); - $modrinthProjects = cache()->remember('modrinth_bulk:' . md5($idsParam), now()->addMinutes(30), function () use ($idsParam) { + $cacheKey = 'modrinth_bulk:'.md5($idsParam); + + $modrinthProjects = cache()->get($cacheKey); + + if (!is_array($modrinthProjects)) { try { - return Http::asJson() + $modrinthProjects = Http::asJson() ->timeout(10) ->connectTimeout(5) ->throw() @@ -172,15 +197,17 @@ public function getInstalledModsFromModrinth(array $installedMods, int $page = 1 'ids' => $idsParam, ]) ->json(); + + if (!is_array($modrinthProjects)) { + $modrinthProjects = []; + } + + cache()->put($cacheKey, $modrinthProjects, now()->addMinutes(30)); } catch (Exception $exception) { report($exception); - return []; + $modrinthProjects = []; } - }); - - if (!is_array($modrinthProjects)) { - $modrinthProjects = []; } $modrinthMap = []; @@ -190,20 +217,9 @@ public function getInstalledModsFromModrinth(array $installedMods, int $page = 1 } } - $installedModsById = []; - foreach ($installedMods as $mod) { - if (!isset($installedModsById[$mod['project_id']])) { - $installedModsById[$mod['project_id']] = $mod; - } - } - $results = []; foreach ($pageIds as $projectId) { - $installedMod = $installedModsById[$projectId] ?? null; - - if (!$installedMod) { - continue; - } + $installedMod = $installedModsById[$projectId]; if (isset($modrinthMap[$projectId])) { $project = $modrinthMap[$projectId]; @@ -234,6 +250,31 @@ public function getInstalledModsFromModrinth(array $installedMods, int $page = 1 return $results; } + protected function getVersionsCacheKey(string $projectId, ?string $minecraftVersion, string $minecraftLoader): string + { + return "modrinth_versions:$projectId:$minecraftVersion:$minecraftLoader"; + } + + /** @return array{game_versions: string, loaders: string} */ + protected function getVersionsQuery(?string $minecraftVersion, string $minecraftLoader): array + { + return [ + 'game_versions' => "[\"$minecraftVersion\"]", + 'loaders' => "[\"$minecraftLoader\"]", + ]; + } + + /** + * @param array $versions + * @return array + */ + protected function sortVersions(array $versions): array + { + usort($versions, fn ($a, $b) => strcmp($b['date_published'] ?? '', $a['date_published'] ?? '')); + + return $versions; + } + /** @return array, game_version: string[], version_type: string, loaders: string[], featured: bool, status: string, requested_status: ?string, id: string, project_id: string, author_id: string, date_published: string, downloads: int, changelog_url: ?string, files: array}> */ public function getProjectVersions(string $projectId, Server $server): array { @@ -246,33 +287,113 @@ public function getProjectVersions(string $projectId, Server $server): array $minecraftVersion = $this->getMinecraftVersion($server); $minecraftLoader = $minecraftLoader['name']; - $data = [ - 'game_versions' => "[\"$minecraftVersion\"]", - 'loaders' => "[\"$minecraftLoader\"]", - ]; + $key = $this->getVersionsCacheKey($projectId, $minecraftVersion, $minecraftLoader); - return cache()->remember("modrinth_versions:$projectId:$minecraftVersion:$minecraftLoader", now()->addMinutes(30), function () use ($projectId, $data) { - try { - $versions = Http::asJson() - ->timeout(5) + $cached = cache()->get($key); + if (is_array($cached)) { + return $cached; + } + + try { + $versions = Http::asJson() + ->timeout(5) + ->connectTimeout(5) + ->throw() + ->get("https://api.modrinth.com/v2/project/$projectId/version", $this->getVersionsQuery($minecraftVersion, $minecraftLoader)) + ->json(); + } catch (Exception $exception) { + report($exception); + + // not cached on purpose since an empty list would hide the update action for 30 minutes. + return []; + } + + $versions = is_array($versions) ? $this->sortVersions($versions) : []; + + cache()->put($key, $versions, now()->addMinutes(30)); + + return $versions; + } + + /** + * fetch compatible versions of several projects at once, a full installed + * tab used to cost up to 20 sequential Modrinth round trips whilst this fetches in bulk. + * + * @param array $projectIds + * @return array> + */ + public function getProjectVersionsBulk(array $projectIds, Server $server): array + { + $projectIds = array_values(array_unique(array_filter($projectIds))); + + if (empty($projectIds)) { + return []; + } + + $minecraftLoader = $this->getLoaderFromServer($server); + + if (!$minecraftLoader) { + return array_fill_keys($projectIds, []); + } + + $minecraftVersion = $this->getMinecraftVersion($server); + $minecraftLoader = $minecraftLoader['name']; + $query = $this->getVersionsQuery($minecraftVersion, $minecraftLoader); + + $results = []; + $missing = []; + + foreach ($projectIds as $projectId) { + $cached = cache()->get($this->getVersionsCacheKey($projectId, $minecraftVersion, $minecraftLoader)); + + if (is_array($cached)) { + $results[$projectId] = $cached; + } else { + $missing[] = $projectId; + } + } + + if (empty($missing)) { + return $results; + } + + try { + $responses = Http::pool(fn (Pool $pool) => array_map( + fn (string $projectId) => $pool->as($projectId) + ->asJson() + ->timeout(10) ->connectTimeout(5) - ->throw() - ->get("https://api.modrinth.com/v2/project/$projectId/version", $data) - ->json(); + ->get("https://api.modrinth.com/v2/project/$projectId/version", $query), + $missing + )); + } catch (Exception $exception) { + report($exception); - if (is_array($versions) && !empty($versions)) { - usort($versions, function ($a, $b) { - return strcmp($b['date_published'] ?? '', $a['date_published'] ?? ''); - }); - } + $responses = []; + } - return $versions; - } catch (Exception $exception) { - report($exception); + foreach ($missing as $projectId) { + $response = $responses[$projectId] ?? null; - return []; + if ($response instanceof Response && $response->successful()) { + $versions = $response->json(); + $versions = is_array($versions) ? $this->sortVersions($versions) : []; + + cache()->put($this->getVersionsCacheKey($projectId, $minecraftVersion, $minecraftLoader), $versions, now()->addMinutes(30)); + + $results[$projectId] = $versions; + + continue; } - }); + + if ($response instanceof Exception) { + report($response); + } + + $results[$projectId] = []; + } + + return $results; } /** @@ -292,41 +413,47 @@ public function getInstalledModsMetadata(Server $server, ModrinthProjectType $mo $metadataPath = $this->getMetadataFilePath($modrinthProjectType); $content = $fileRepository->setServer($server)->getContent($metadataPath); $metadata = json_decode($content, true); + } catch (FileNotFoundException) { + return []; + } catch (Exception $exception) { + report($exception); - if (!is_array($metadata) || !isset($metadata['installed_mods']) || !is_array($metadata['installed_mods'])) { - return []; - } + return []; + } - $validInstalledMods = []; - $requiredKeys = [ - 'project_id', - 'project_slug', - 'project_title', - 'version_id', - 'version_number', - 'filename', - 'installed_at', - ]; + if (!is_array($metadata) || !isset($metadata['installed_mods']) || !is_array($metadata['installed_mods'])) { + return []; + } - $requiredKeysFlipped = array_flip($requiredKeys); + $requiredKeysFlipped = array_flip([ + 'project_id', + 'project_slug', + 'project_title', + 'version_id', + 'version_number', + 'filename', + 'installed_at', + ]); - foreach ($metadata['installed_mods'] as $entry) { - if (!is_array($entry)) { - continue; - } + $validInstalledMods = []; - $missingKeys = array_diff_key($requiredKeysFlipped, $entry); - if (empty($missingKeys)) { - $validInstalledMods[] = $entry; - } + foreach ($metadata['installed_mods'] as $entry) { + if (!is_array($entry)) { + continue; } - return $validInstalledMods; - } catch (Exception $exception) { - report($exception); + if (!empty(array_diff_key($requiredKeysFlipped, $entry))) { + continue; + } - return []; + if (isset($validInstalledMods[$entry['project_id']])) { + continue; + } + + $validInstalledMods[$entry['project_id']] = $entry; } + + return array_values($validInstalledMods); } public function saveModMetadata( @@ -344,14 +471,16 @@ public function saveModMetadata( return Cache::lock("modrinth_metadata:{$server->id}", 10)->block(5, function () use ($server, $modrinthProjectType, $projectId, $projectSlug, $projectTitle, $versionId, $versionNumber, $filename, $author) { $fileRepository = app(DaemonFileRepository::class); - $metadata = [ - 'installed_mods' => $this->getInstalledModsMetadata($server, $modrinthProjectType), - ]; + $installedMods = $this->getInstalledModsMetadata($server, $modrinthProjectType); - $metadata['installed_mods'] = collect($metadata['installed_mods']) - ->filter(fn ($mod) => $mod['project_id'] !== $projectId) - ->values() - ->toArray(); + $existingIndex = null; + foreach ($installedMods as $index => $mod) { + if ($mod['project_id'] === $projectId) { + $existingIndex = $index; + + break; + } + } $modEntry = [ 'project_id' => $projectId, @@ -360,19 +489,31 @@ public function saveModMetadata( 'version_id' => $versionId, 'version_number' => $versionNumber, 'filename' => $filename, - 'installed_at' => now()->toIso8601String(), + 'installed_at' => $existingIndex !== null + ? $installedMods[$existingIndex]['installed_at'] + : now()->toIso8601String(), ]; + if ($existingIndex !== null) { + $modEntry['updated_at'] = now()->toIso8601String(); + } + if ($author !== null) { $modEntry['author'] = $author; } - $metadata['installed_mods'][] = $modEntry; + if ($existingIndex !== null) { + // Replace in place. Removing and re-appending pushed the entry to the end of the + // list, which moved the row to the bottom of the installed tab on every update. + $installedMods[$existingIndex] = $modEntry; + } else { + $installedMods[] = $modEntry; + } $metadataPath = $this->getMetadataFilePath($modrinthProjectType); $response = $fileRepository->setServer($server)->putContent( $metadataPath, - json_encode($metadata, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) + json_encode(['installed_mods' => array_values($installedMods)], JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) ); return !$response->failed(); @@ -390,11 +531,7 @@ public function removeModMetadata(Server $server, ModrinthProjectType $modrinthP return Cache::lock("modrinth_metadata:{$server->id}", 10)->block(5, function () use ($server, $modrinthProjectType, $projectId) { $fileRepository = app(DaemonFileRepository::class); - $metadata = [ - 'installed_mods' => $this->getInstalledModsMetadata($server, $modrinthProjectType), - ]; - - $metadata['installed_mods'] = collect($metadata['installed_mods']) + $installedMods = collect($this->getInstalledModsMetadata($server, $modrinthProjectType)) ->filter(fn ($mod) => $mod['project_id'] !== $projectId) ->values() ->toArray(); @@ -402,7 +539,7 @@ public function removeModMetadata(Server $server, ModrinthProjectType $modrinthP $metadataPath = $this->getMetadataFilePath($modrinthProjectType); $response = $fileRepository->setServer($server)->putContent( $metadataPath, - json_encode($metadata, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) + json_encode(['installed_mods' => $installedMods], JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) ); return !$response->failed(); @@ -414,6 +551,84 @@ public function removeModMetadata(Server $server, ModrinthProjectType $modrinthP } } + /** + * Ask the daemon to download a file and wait until it is actually on disk. + * + * Without `foreground` the daemon downloads in the background, so the panel gets a success + * response before the file exists and never hears about a failed download. The explicit file + * name matters too: otherwise the daemon derives it from the URL, which can differ from the + * name Modrinth reports and would leave the metadata pointing at a file that isn't there. + * + * @throws Exception + */ + public function downloadFile(Server $server, string $url, string $folder, string $filename): void + { + try { + app(DaemonFileRepository::class) + ->setServer($server) + ->getHttpClient() + // The daemon holds the request open for the whole download, which easily outlives + // the timeout used for the small file operations everything else here does. + ->timeout(max((int) config('panel.guzzle.timeout'), self::DOWNLOAD_TIMEOUT)) + ->post("/api/servers/{$server->uuid}/files/pull", [ + 'url' => $url, + 'root' => $folder, + 'file_name' => $filename, + 'foreground' => true, + ]); + } catch (Exception $exception) { + // A slow download can outlive our request while still finishing on the node. + if (!$this->fileExists($server, $folder, $filename)) { + throw $exception; + } + + return; + } + + if (!$this->fileExists($server, $folder, $filename)) { + throw new Exception("Daemon reported success but $folder/$filename is missing after downloading $url"); + } + } + + /** + * @throws Exception + */ + public function deleteFile(Server $server, string $folder, string $filename): void + { + app(DaemonFileRepository::class) + ->setServer($server) + ->deleteFiles('/', [join_paths($folder, $filename)]) + ->throw(); + } + + public function fileExists(Server $server, string $folder, string $filename): bool + { + foreach ($this->listFolder($server, $folder) as $file) { + if (is_array($file) && ($file['name'] ?? null) === $filename) { + return true; + } + } + + return false; + } + + /** @return array */ + public function listFolder(Server $server, string $folder): array + { + try { + $files = app(DaemonFileRepository::class)->setServer($server)->getDirectory($folder); + } catch (Exception) { + // The folder may simply not exist yet. + return []; + } + + if (isset($files['error'])) { + return []; + } + + return $files; + } + /** @return array{project_id: string, project_slug: string, project_title: string, version_id: string, version_number: string, filename: string, installed_at: string, author?: string}|null */ public function getInstalledMod(Server $server, ModrinthProjectType $modrinthProjectType, string $projectId): ?array { From 71c7b4aabe8fb891dde1d74b801d033511cea858 Mon Sep 17 00:00:00 2001 From: hardcorefactions <108526035+hardcorefactions@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:40:17 +0200 Subject: [PATCH 2/2] address feedback from coderabbit --- .../Pages/MinecraftModrinthProjectPage.php | 15 ++++-- .../src/Services/MinecraftModrinthService.php | 53 +++++++++++++++---- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php b/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php index 3fbeb053..b7fbd7bd 100644 --- a/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php +++ b/minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php @@ -707,9 +707,18 @@ public function content(Schema $schema): Schema ->badge(), TextEntry::make('installed') ->label(fn () => trans('minecraft-modrinth::strings.page.installed', ['type' => static::$modrinthProjectType?->getLabel() ?? 'Modrinth'])) - ->state(fn () => collect(MinecraftModrinth::listFolder($server, static::$modrinthProjectType->getFolder())) - ->filter(fn ($file) => ($file['mime'] ?? null) === 'application/jar' || str($file['name'] ?? '')->lower()->endsWith('.jar')) - ->count()) + ->state(function () use ($server) { + try { + return collect(MinecraftModrinth::listFolder($server, static::$modrinthProjectType->getFolder())) + ->filter(fn ($file) => ($file['mime'] ?? null) === 'application/jar' || str($file['name'] ?? '')->lower()->endsWith('.jar')) + ->count(); + } catch (Exception $exception) { + report($exception); + + // Don't pass off an unreachable daemon as an empty folder. + return trans('minecraft-modrinth::strings.page.unknown'); + } + }) ->badge(), ]), $this->getTabsContentComponent(), diff --git a/minecraft-modrinth/src/Services/MinecraftModrinthService.php b/minecraft-modrinth/src/Services/MinecraftModrinthService.php index c9be310e..7db2b7f5 100644 --- a/minecraft-modrinth/src/Services/MinecraftModrinthService.php +++ b/minecraft-modrinth/src/Services/MinecraftModrinthService.php @@ -8,6 +8,7 @@ use Exception; use Illuminate\Contracts\Filesystem\FileNotFoundException; use Illuminate\Http\Client\Pool; +use Illuminate\Http\Client\RequestException; use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; @@ -17,6 +18,11 @@ class MinecraftModrinthService // Seconds to wait for the daemon while it downloads a file for us. protected const DOWNLOAD_TIMEOUT = 120; + // Version list cache TTL (minutes); kept short so a transient empty Modrinth response can't hide updates for long. + protected const VERSIONS_CACHE_MINUTES = 30; + + protected const EMPTY_VERSIONS_CACHE_MINUTES = 5; + public function getMinecraftVersion(Server $server): ?string { $version = $server->variables()->where(fn ($builder) => $builder->where('env_variable', 'MINECRAFT_VERSION')->orWhere('env_variable', 'MC_VERSION'))->first()?->server_value; @@ -264,6 +270,12 @@ protected function getVersionsQuery(?string $minecraftVersion, string $minecraft ]; } + /** @param array $versions */ + protected function cacheVersions(string $key, array $versions): void + { + cache()->put($key, $versions, now()->addMinutes(empty($versions) ? self::EMPTY_VERSIONS_CACHE_MINUTES : self::VERSIONS_CACHE_MINUTES)); + } + /** * @param array $versions * @return array @@ -310,7 +322,7 @@ public function getProjectVersions(string $projectId, Server $server): array $versions = is_array($versions) ? $this->sortVersions($versions) : []; - cache()->put($key, $versions, now()->addMinutes(30)); + $this->cacheVersions($key, $versions); return $versions; } @@ -379,7 +391,7 @@ public function getProjectVersionsBulk(array $projectIds, Server $server): array $versions = $response->json(); $versions = is_array($versions) ? $this->sortVersions($versions) : []; - cache()->put($this->getVersionsCacheKey($projectId, $minecraftVersion, $minecraftLoader), $versions, now()->addMinutes(30)); + $this->cacheVersions($this->getVersionsCacheKey($projectId, $minecraftVersion, $minecraftLoader), $versions); $results[$projectId] = $versions; @@ -563,6 +575,10 @@ public function removeModMetadata(Server $server, ModrinthProjectType $modrinthP */ public function downloadFile(Server $server, string $url, string $folder, string $filename): void { + // An update often reuses the name of the file it replaces. If the name is already taken, + // finding it there afterwards says nothing about whether our download replaced it. + $nameWasTaken = $this->fileExists($server, $folder, $filename); + try { app(DaemonFileRepository::class) ->setServer($server) @@ -577,8 +593,19 @@ public function downloadFile(Server $server, string $url, string $folder, string 'foreground' => true, ]); } catch (Exception $exception) { - // A slow download can outlive our request while still finishing on the node. - if (!$this->fileExists($server, $folder, $filename)) { + // A slow download can outlive our request while still finishing on the node, but a + // file appearing under a name that was free before is the only proof of that we get. + if ($nameWasTaken) { + throw $exception; + } + + try { + $landed = $this->fileExists($server, $folder, $filename); + } catch (Exception) { + throw $exception; + } + + if (!$landed) { throw $exception; } @@ -612,18 +639,26 @@ public function fileExists(Server $server, string $folder, string $filename): bo return false; } - /** @return array */ + /** + * @return array + * + * @throws Exception + */ public function listFolder(Server $server, string $folder): array { try { $files = app(DaemonFileRepository::class)->setServer($server)->getDirectory($folder); - } catch (Exception) { - // The folder may simply not exist yet. - return []; + } catch (RequestException $exception) { + if ($exception->response->status() === 404) { + // The folder simply doesn't exist yet. + return []; + } + + throw $exception; } if (isset($files['error'])) { - return []; + throw new Exception("Daemon returned an error while listing $folder: ".(is_string($files['error']) ? $files['error'] : 'unknown error')); } return $files;