From b859f0bfec854aac178eb46c25fac17d7d72ab04 Mon Sep 17 00:00:00 2001 From: Phan An Date: Tue, 1 Oct 2024 00:15:38 +0200 Subject: [PATCH] feat: replace attempt() with built-in rescue() (#1833) --- app/Casts/SmartPlaylistRulesCast.php | 8 ++--- app/Helpers.php | 31 +++---------------- app/Http/Controllers/API/AuthController.php | 2 +- .../Integrations/Spotify/SpotifyClient.php | 2 +- app/Listeners/WriteSyncLog.php | 2 +- app/Models/SongZipArchive.php | 2 +- app/Observers/AlbumObserver.php | 2 +- app/Repositories/Repository.php | 2 +- app/Rules/ImageData.php | 4 +-- app/Rules/SupportedAudioFile.php | 4 +-- app/Rules/ValidSmartPlaylistRulePayload.php | 2 +- .../ApplicationInformationService.php | 2 +- app/Services/FileScanner.php | 6 ++-- app/Services/ITunesService.php | 2 +- app/Services/LastfmService.php | 10 +++--- app/Services/MediaInformationService.php | 4 +-- app/Services/MediaMetadataService.php | 6 ++-- ...rt_user_preferences_from_array_to_json.php | 4 +-- ..._04_092531_drop_contributing_artist_id.php | 4 +-- .../ValidSmartPlaylistRulePayloadTest.php | 9 +++--- 20 files changed, 43 insertions(+), 65 deletions(-) diff --git a/app/Casts/SmartPlaylistRulesCast.php b/app/Casts/SmartPlaylistRulesCast.php index 38bb2c93..c5892a73 100644 --- a/app/Casts/SmartPlaylistRulesCast.php +++ b/app/Casts/SmartPlaylistRulesCast.php @@ -9,11 +9,9 @@ class SmartPlaylistRulesCast implements CastsAttributes { public function get($model, string $key, $value, array $attributes): ?SmartPlaylistRuleGroupCollection { - if (!$value) { - return null; - } - - return attempt(static fn () => SmartPlaylistRuleGroupCollection::create(json_decode($value, true))); + return $value + ? rescue(static fn () => SmartPlaylistRuleGroupCollection::create(json_decode($value, true))) + : null; } public function set($model, string $key, $value, array $attributes): ?string diff --git a/app/Helpers.php b/app/Helpers.php index 08b99f18..fbc7f8c0 100644 --- a/app/Helpers.php +++ b/app/Helpers.php @@ -2,7 +2,6 @@ use App\Facades\License; use Illuminate\Support\Facades\File as FileFacade; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; /** @@ -64,34 +63,14 @@ function koel_version(): string return trim(FileFacade::get(base_path('.version'))); } -/** - * @throws Throwable - */ -function attempt(callable $callback, bool $log = true, bool $throw = false): mixed +function rescue_if($condition, callable $callback): mixed { - try { - return $callback(); - } catch (Throwable $e) { - if (app()->runningUnitTests() || $throw) { - throw $e; - } - - if ($log) { - Log::error('Failed attempt', ['error' => $e]); - } - - return null; - } + return value($condition) ? rescue($callback) : null; } -function attempt_if($condition, callable $callback, bool $log = true): mixed +function rescue_unless($condition, callable $callback): mixed { - return value($condition) ? attempt($callback, $log) : null; -} - -function attempt_unless($condition, callable $callback, bool $log = true): mixed -{ - return !value($condition) ? attempt($callback, $log) : null; + return !value($condition) ? rescue($callback) : null; } function gravatar(string $email, int $size = 192): string @@ -146,5 +125,5 @@ function get_mtime(string|SplFileInfo $file): int $file = is_string($file) ? new SplFileInfo($file) : $file; // Workaround for #344, where getMTime() fails for certain files with Unicode names on Windows. - return attempt(static fn () => $file->getMTime()) ?? time(); + return rescue(static fn () => $file->getMTime()) ?? time(); } diff --git a/app/Http/Controllers/API/AuthController.php b/app/Http/Controllers/API/AuthController.php index 669513cf..92d7a653 100644 --- a/app/Http/Controllers/API/AuthController.php +++ b/app/Http/Controllers/API/AuthController.php @@ -57,7 +57,7 @@ class AuthController extends Controller public function logout(Request $request): Response { - attempt(fn () => $this->auth->logoutViaBearerToken($request->bearerToken())); + rescue(fn () => $this->auth->logoutViaBearerToken($request->bearerToken())); return response()->noContent(); } diff --git a/app/Http/Integrations/Spotify/SpotifyClient.php b/app/Http/Integrations/Spotify/SpotifyClient.php index 3f73538f..b5020e1c 100644 --- a/app/Http/Integrations/Spotify/SpotifyClient.php +++ b/app/Http/Integrations/Spotify/SpotifyClient.php @@ -22,7 +22,7 @@ class SpotifyClient ) { if (SpotifyService::enabled()) { $this->wrapped->setOptions(['return_assoc' => true]); - attempt(fn () => $this->setAccessToken()); + rescue(fn () => $this->setAccessToken()); } } diff --git a/app/Listeners/WriteSyncLog.php b/app/Listeners/WriteSyncLog.php index 85605a4e..d6f18ed1 100644 --- a/app/Listeners/WriteSyncLog.php +++ b/app/Listeners/WriteSyncLog.php @@ -18,7 +18,7 @@ class WriteSyncLog ? $event->results->map($transformer) : $event->results->error()->map($transformer); - attempt(static function () use ($messages): void { + rescue(static function () use ($messages): void { $file = storage_path('logs/sync-' . now()->format('Ymd-His') . '.log'); File::put($file, implode(PHP_EOL, $messages->toArray())); }); diff --git a/app/Models/SongZipArchive.php b/app/Models/SongZipArchive.php index 1cc0988f..1483dec4 100644 --- a/app/Models/SongZipArchive.php +++ b/app/Models/SongZipArchive.php @@ -39,7 +39,7 @@ class SongZipArchive public function addSong(Song $song): static { - attempt(function () use ($song): void { + rescue(function () use ($song): void { $path = Download::getLocalPath($song); $this->archive->addFile($path, $this->generateZipContentFileNameFromPath($path)); }); diff --git a/app/Observers/AlbumObserver.php b/app/Observers/AlbumObserver.php index 4ad325a9..2db151a4 100644 --- a/app/Observers/AlbumObserver.php +++ b/app/Observers/AlbumObserver.php @@ -12,6 +12,6 @@ class AlbumObserver return; } - attempt(static fn () => unlink($album->cover_path)); + rescue(static fn () => unlink($album->cover_path)); } } diff --git a/app/Repositories/Repository.php b/app/Repositories/Repository.php index c01705b1..07bf242c 100644 --- a/app/Repositories/Repository.php +++ b/app/Repositories/Repository.php @@ -21,7 +21,7 @@ abstract class Repository implements RepositoryInterface // This instantiation may fail during a console command if e.g. APP_KEY is empty, // rendering the whole installation failing. - attempt(fn () => $this->auth = app(Guard::class), false); + rescue(fn () => $this->auth = app(Guard::class)); } private static function guessModelClass(): string diff --git a/app/Rules/ImageData.php b/app/Rules/ImageData.php index 32e3460e..c29d958f 100644 --- a/app/Rules/ImageData.php +++ b/app/Rules/ImageData.php @@ -10,9 +10,9 @@ class ImageData implements ValidationRule { public function validate(string $attribute, mixed $value, Closure $fail): void { - $passes = attempt(static function () use ($value) { + $passes = rescue(static function () use ($value) { return (bool) preg_match('/data:image\/(jpe?g|png|webp|gif)/i', Str::before($value, ';')); - }, false) ?? false; + }) ?? false; if (!$passes) { $fail('Invalid DataURL string'); diff --git a/app/Rules/SupportedAudioFile.php b/app/Rules/SupportedAudioFile.php index d6c56d41..e333172c 100644 --- a/app/Rules/SupportedAudioFile.php +++ b/app/Rules/SupportedAudioFile.php @@ -14,14 +14,14 @@ class SupportedAudioFile implements ValidationRule public function validate(string $attribute, mixed $value, Closure $fail): void { - $passes = attempt(static function () use ($value) { + $passes = rescue(static function () use ($value) { Assert::oneOf( Arr::get((new getID3())->analyze($value->getRealPath()), 'fileformat'), self::SUPPORTED_FORMATS ); return true; - }, false) ?? false; + }) ?? false; if (!$passes) { $fail('Unsupported audio file'); diff --git a/app/Rules/ValidSmartPlaylistRulePayload.php b/app/Rules/ValidSmartPlaylistRulePayload.php index 480f503a..46ba93e8 100644 --- a/app/Rules/ValidSmartPlaylistRulePayload.php +++ b/app/Rules/ValidSmartPlaylistRulePayload.php @@ -11,7 +11,7 @@ class ValidSmartPlaylistRulePayload implements ValidationRule { public function validate(string $attribute, mixed $value, Closure $fail): void { - $passes = (bool) attempt(static fn () => SmartPlaylistRuleGroupCollection::create(Arr::wrap($value))); + $passes = (bool) rescue(static fn () => SmartPlaylistRuleGroupCollection::create(Arr::wrap($value))); if (!$passes) { $fail('Invalid smart playlist rules'); diff --git a/app/Services/ApplicationInformationService.php b/app/Services/ApplicationInformationService.php index c44b3554..9b6e7f28 100644 --- a/app/Services/ApplicationInformationService.php +++ b/app/Services/ApplicationInformationService.php @@ -16,7 +16,7 @@ class ApplicationInformationService */ public function getLatestVersionNumber(): string { - return attempt(function () { + return rescue(function () { return Cache::remember('latestKoelVersion', now()->addDay(), function (): string { return json_decode($this->client->get('https://api.github.com/repos/koel/koel/tags')->getBody())[0] ->name; diff --git a/app/Services/FileScanner.php b/app/Services/FileScanner.php index 720430d6..b5055631 100644 --- a/app/Services/FileScanner.php +++ b/app/Services/FileScanner.php @@ -111,7 +111,7 @@ class FileScanner */ private function tryGenerateAlbumCover(Album $album, ?array $coverData): void { - attempt(function () use ($album, $coverData): void { + rescue(function () use ($album, $coverData): void { // If the album has no cover, we try to get the cover image from existing tag data if ($coverData) { $this->mediaMetadataService->writeAlbumCover($album, $coverData['data']); @@ -125,7 +125,7 @@ class FileScanner if ($cover) { $this->mediaMetadataService->writeAlbumCover($album, $cover); } - }, false); + }); } /** @@ -157,7 +157,7 @@ class FileScanner private static function isImage(string $path): bool { - return attempt(static fn () => (bool) exif_imagetype($path)) ?? false; + return rescue(static fn () => (bool) exif_imagetype($path)) ?? false; } /** diff --git a/app/Services/ITunesService.php b/app/Services/ITunesService.php index 85ad2b38..2cbb2993 100644 --- a/app/Services/ITunesService.php +++ b/app/Services/ITunesService.php @@ -20,7 +20,7 @@ class ITunesService public function getTrackUrl(string $trackName, Album $album): ?string { - return attempt(function () use ($trackName, $album): ?string { + return rescue(function () use ($trackName, $album): ?string { $request = new GetTrackRequest($trackName, $album); $hash = md5(serialize($request->query())); diff --git a/app/Services/LastfmService.php b/app/Services/LastfmService.php index 1b0b8e49..d7d57e09 100644 --- a/app/Services/LastfmService.php +++ b/app/Services/LastfmService.php @@ -48,7 +48,7 @@ class LastfmService implements MusicEncyclopedia return null; } - return attempt_if(static::enabled(), function () use ($artist): ?ArtistInformation { + return rescue_if(static::enabled(), function () use ($artist): ?ArtistInformation { return Cache::remember( "lastfm.artist.$artist->id", now()->addWeek(), @@ -63,7 +63,7 @@ class LastfmService implements MusicEncyclopedia return null; } - return attempt_if(static::enabled(), function () use ($album): ?AlbumInformation { + return rescue_if(static::enabled(), function () use ($album): ?AlbumInformation { return Cache::remember( "lastfm.album.$album->id", now()->addWeek(), @@ -74,12 +74,12 @@ class LastfmService implements MusicEncyclopedia public function scrobble(Song $song, User $user, int $timestamp): void { - attempt(fn () => $this->connector->send(new ScrobbleRequest($song, $user, $timestamp))); + rescue(fn () => $this->connector->send(new ScrobbleRequest($song, $user, $timestamp))); } public function toggleLoveTrack(Song $song, User $user, bool $love): void { - attempt(fn () => $this->connector->send(new ToggleLoveTrackRequest($song, $user, $love))); + rescue(fn () => $this->connector->send(new ToggleLoveTrackRequest($song, $user, $love))); } /** @@ -101,7 +101,7 @@ class LastfmService implements MusicEncyclopedia public function updateNowPlaying(Song $song, User $user): void { - attempt(fn () => $this->connector->send(new UpdateNowPlayingRequest($song, $user))); + rescue(fn () => $this->connector->send(new UpdateNowPlayingRequest($song, $user))); } public function getSessionKey(string $token): ?string diff --git a/app/Services/MediaInformationService.php b/app/Services/MediaInformationService.php index ce5ea817..8cdff3f8 100644 --- a/app/Services/MediaInformationService.php +++ b/app/Services/MediaInformationService.php @@ -26,7 +26,7 @@ class MediaInformationService return Cache::remember("album.info.$album->id", now()->addWeek(), function () use ($album): AlbumInformation { $info = $this->encyclopedia->getAlbumInformation($album) ?: AlbumInformation::make(); - attempt_unless($album->has_cover, function () use ($info, $album): void { + rescue_unless($album->has_cover, function () use ($info, $album): void { $this->mediaMetadataService->tryDownloadAlbumCover($album); $info->cover = $album->cover; }); @@ -47,7 +47,7 @@ class MediaInformationService function () use ($artist): ArtistInformation { $info = $this->encyclopedia->getArtistInformation($artist) ?: ArtistInformation::make(); - attempt_unless($artist->has_image, function () use ($artist, $info): void { + rescue_unless($artist->has_image, function () use ($artist, $info): void { $this->mediaMetadataService->tryDownloadArtistImage($artist); $info->image = $artist->image; }); diff --git a/app/Services/MediaMetadataService.php b/app/Services/MediaMetadataService.php index 0f082557..6fd0fb20 100644 --- a/app/Services/MediaMetadataService.php +++ b/app/Services/MediaMetadataService.php @@ -31,7 +31,7 @@ class MediaMetadataService */ public function writeAlbumCover(Album $album, string $source, ?string $destination = '', bool $cleanUp = true): void { - attempt(function () use ($album, $source, $destination, $cleanUp): void { + rescue(function () use ($album, $source, $destination, $cleanUp): void { $destination = $destination ?: $this->generateAlbumCoverPath(); $this->imageWriter->write($destination, $source); @@ -63,7 +63,7 @@ class MediaMetadataService ?string $destination = '', bool $cleanUp = true ): void { - attempt(function () use ($artist, $source, $destination, $cleanUp): void { + rescue(function () use ($artist, $source, $destination, $cleanUp): void { $destination = $destination ?: $this->generateArtistImagePath(); $this->imageWriter->write($destination, $source); @@ -77,7 +77,7 @@ class MediaMetadataService public function writePlaylistCover(Playlist $playlist, string $source): void { - attempt(function () use ($playlist, $source): void { + rescue(function () use ($playlist, $source): void { $destination = $this->generatePlaylistCoverPath(); $this->imageWriter->write($destination, $source); diff --git a/database/migrations/2021_06_04_153259_convert_user_preferences_from_array_to_json.php b/database/migrations/2021_06_04_153259_convert_user_preferences_from_array_to_json.php index 09ad730d..a8158207 100644 --- a/database/migrations/2021_06_04_153259_convert_user_preferences_from_array_to_json.php +++ b/database/migrations/2021_06_04_153259_convert_user_preferences_from_array_to_json.php @@ -9,11 +9,11 @@ class ConvertUserPreferencesFromArrayToJson extends Migration public function up(): void { User::all()->each(static function (User $user): void { - attempt(static function () use ($user): void { + rescue(static function () use ($user): void { $preferences = unserialize($user->getRawOriginal('preferences')); $user->preferences->lastFmSessionKey = Arr::get($preferences, 'lastfm_session_key'); $user->save(); - }, false); + }); }); } } diff --git a/database/migrations/2022_08_04_092531_drop_contributing_artist_id.php b/database/migrations/2022_08_04_092531_drop_contributing_artist_id.php index 02b358c3..09602516 100644 --- a/database/migrations/2022_08_04_092531_drop_contributing_artist_id.php +++ b/database/migrations/2022_08_04_092531_drop_contributing_artist_id.php @@ -11,7 +11,7 @@ return new class extends Migration { Schema::table('songs', static function (Blueprint $table): void { // This migration is actually to fix a mistake that the original one was deleted. // Therefore, we just "try" it and ignore on error. - attempt_if(Schema::hasColumn('songs', 'contributing_artist_id'), static function () use ($table): void { + rescue_if(Schema::hasColumn('songs', 'contributing_artist_id'), static function () use ($table): void { Schema::disableForeignKeyConstraints(); if (DB::getDriverName() !== 'sqlite') { // @phpstan-ignore-line @@ -20,7 +20,7 @@ return new class extends Migration { $table->dropColumn('contributing_artist_id'); Schema::enableForeignKeyConstraints(); - }, false); + }); }); } }; diff --git a/tests/Unit/Rules/ValidSmartPlaylistRulePayloadTest.php b/tests/Unit/Rules/ValidSmartPlaylistRulePayloadTest.php index 3ce038e7..ccdd50b5 100644 --- a/tests/Unit/Rules/ValidSmartPlaylistRulePayloadTest.php +++ b/tests/Unit/Rules/ValidSmartPlaylistRulePayloadTest.php @@ -3,8 +3,8 @@ namespace Tests\Unit\Rules; use App\Rules\ValidSmartPlaylistRulePayload; +use Exception; use Tests\TestCase; -use Throwable; class ValidSmartPlaylistRulePayloadTest extends TestCase { @@ -94,9 +94,10 @@ class ValidSmartPlaylistRulePayloadTest extends TestCase /** @dataProvider provideInvalidPayloads */ public function testInvalidCases($value): void { - $this->expectException(Throwable::class); - (new ValidSmartPlaylistRulePayload())->validate('rules', $value, static fn ($foo) => $foo); - self::addToAssertionCount(1); + $this->expectExceptionMessage('Invalid smart playlist rules'); + + $fail = static fn (string $message) => throw new Exception($message); + (new ValidSmartPlaylistRulePayload())->validate('rules', $value, $fail); } /** @return array */