From 08e49532179c1e8ef687712d8125e3077cbb2a40 Mon Sep 17 00:00:00 2001 From: Phan An Date: Fri, 8 Jul 2022 16:53:04 +0200 Subject: [PATCH] feat: decouple artist/album and the media information --- app/Events/AlbumInformationFetched.php | 19 +--- app/Events/ArtistInformationFetched.php | 19 +--- .../API/MediaInformation/AlbumController.php | 2 +- .../API/MediaInformation/ArtistController.php | 2 +- .../API/MediaInformation/SongController.php | 14 ++- .../Controllers/V6/API/AlbumController.php | 13 +-- .../Controllers/V6/API/ArtistController.php | 5 +- .../API/FetchAlbumInformationController.php | 15 ++++ .../API/FetchArtistInformationController.php | 15 ++++ app/Http/Resources/AlbumResource.php | 2 - app/Http/Resources/ArtistResource.php | 2 - app/Listeners/DownloadAlbumCover.php | 16 +--- app/Listeners/DownloadArtistImage.php | 16 +--- app/Services/LastfmService.php | 86 +++---------------- app/Services/MediaInformationService.php | 22 ++--- app/Values/AlbumInformation.php | 46 ++++++++++ app/Values/ArtistInformation.php | 36 ++++++++ app/Values/FormatsLastFmText.php | 16 ++++ .../js/__tests__/factory/albumFactory.ts | 20 ----- .../js/__tests__/factory/artistFactory.ts | 8 -- .../assets/js/components/album/AlbumInfo.vue | 34 +++++--- .../js/components/album/AlbumTrackList.vue | 50 ++++++----- .../components/album/AlbumTrackListItem.vue | 36 ++++++-- .../js/components/artist/ArtistInfo.vue | 32 ++++--- .../js/components/screens/AlbumScreen.vue | 2 +- .../js/components/screens/ArtistScreen.vue | 2 +- resources/assets/js/services/cache.ts | 6 +- .../assets/js/services/mediaInfoService.ts | 23 +++++ resources/assets/js/stores/albumStore.ts | 6 +- resources/assets/js/stores/artistStore.ts | 9 +- resources/assets/js/types.d.ts | 3 - resources/assets/sass/partials/_mixins.scss | 64 +++----------- routes/api.v6.php | 6 ++ 33 files changed, 326 insertions(+), 321 deletions(-) create mode 100644 app/Http/Controllers/V6/API/FetchAlbumInformationController.php create mode 100644 app/Http/Controllers/V6/API/FetchArtistInformationController.php create mode 100644 app/Values/AlbumInformation.php create mode 100644 app/Values/ArtistInformation.php create mode 100644 app/Values/FormatsLastFmText.php create mode 100644 resources/assets/js/services/mediaInfoService.ts diff --git a/app/Events/AlbumInformationFetched.php b/app/Events/AlbumInformationFetched.php index 898bc709..941ded50 100644 --- a/app/Events/AlbumInformationFetched.php +++ b/app/Events/AlbumInformationFetched.php @@ -3,29 +3,14 @@ namespace App\Events; use App\Models\Album; +use App\Values\AlbumInformation; use Illuminate\Queue\SerializesModels; class AlbumInformationFetched extends Event { use SerializesModels; - private Album $album; - private array $information; - - public function __construct(Album $album, array $information) + public function __construct(public Album $album, public AlbumInformation $information) { - $this->album = $album; - $this->information = $information; - } - - public function getAlbum(): Album - { - return $this->album; - } - - /** @return array */ - public function getInformation(): array - { - return $this->information; } } diff --git a/app/Events/ArtistInformationFetched.php b/app/Events/ArtistInformationFetched.php index 1f9fb63e..3c8c527a 100644 --- a/app/Events/ArtistInformationFetched.php +++ b/app/Events/ArtistInformationFetched.php @@ -3,29 +3,14 @@ namespace App\Events; use App\Models\Artist; +use App\Values\ArtistInformation; use Illuminate\Queue\SerializesModels; class ArtistInformationFetched { use SerializesModels; - private Artist $artist; - private array $information; - - public function __construct(Artist $artist, array $information) + public function __construct(public Artist $artist, public ArtistInformation $information) { - $this->artist = $artist; - $this->information = $information; - } - - public function getArtist(): Artist - { - return $this->artist; - } - - /** @return array */ - public function getInformation(): array - { - return $this->information; } } diff --git a/app/Http/Controllers/API/MediaInformation/AlbumController.php b/app/Http/Controllers/API/MediaInformation/AlbumController.php index 4767d067..55d77eda 100644 --- a/app/Http/Controllers/API/MediaInformation/AlbumController.php +++ b/app/Http/Controllers/API/MediaInformation/AlbumController.php @@ -8,6 +8,6 @@ class AlbumController extends Controller { public function show(Album $album) { - return response()->json($this->mediaInformationService->getAlbumInformation($album)); + return response()->json($this->mediaInformationService->getAlbumInformation($album)?->toArray() ?: []); } } diff --git a/app/Http/Controllers/API/MediaInformation/ArtistController.php b/app/Http/Controllers/API/MediaInformation/ArtistController.php index bfaaa9e2..cc088ae9 100644 --- a/app/Http/Controllers/API/MediaInformation/ArtistController.php +++ b/app/Http/Controllers/API/MediaInformation/ArtistController.php @@ -8,6 +8,6 @@ class ArtistController extends Controller { public function show(Artist $artist) { - return response()->json($this->mediaInformationService->getArtistInformation($artist)); + return response()->json($this->mediaInformationService->getArtistInformation($artist)?->toArray() ?: []); } } diff --git a/app/Http/Controllers/API/MediaInformation/SongController.php b/app/Http/Controllers/API/MediaInformation/SongController.php index dc99011e..f4ee8b9c 100644 --- a/app/Http/Controllers/API/MediaInformation/SongController.php +++ b/app/Http/Controllers/API/MediaInformation/SongController.php @@ -8,21 +8,19 @@ use App\Services\YouTubeService; class SongController extends Controller { - private YouTubeService $youTubeService; - - public function __construct(MediaInformationService $mediaInformationService, YouTubeService $youTubeService) - { + public function __construct( + protected MediaInformationService $mediaInformationService, + private YouTubeService $youTubeService + ) { parent::__construct($mediaInformationService); - - $this->youTubeService = $youTubeService; } public function show(Song $song) { return response()->json([ 'lyrics' => $song->lyrics, - 'album_info' => $this->mediaInformationService->getAlbumInformation($song->album), - 'artist_info' => $this->mediaInformationService->getArtistInformation($song->artist), + 'album_info' => $this->mediaInformationService->getAlbumInformation($song->album)?->toArray() ?: [], + 'artist_info' => $this->mediaInformationService->getArtistInformation($song->artist)?->toArray() ?: [], 'youtube' => $this->youTubeService->searchVideosRelatedToSong($song), ]); } diff --git a/app/Http/Controllers/V6/API/AlbumController.php b/app/Http/Controllers/V6/API/AlbumController.php index ae0cdb59..9018d685 100644 --- a/app/Http/Controllers/V6/API/AlbumController.php +++ b/app/Http/Controllers/V6/API/AlbumController.php @@ -7,17 +7,13 @@ use App\Http\Resources\AlbumResource; use App\Models\Album; use App\Models\User; use App\Repositories\AlbumRepository; -use App\Services\MediaInformationService; use Illuminate\Contracts\Auth\Authenticatable; class AlbumController extends Controller { /** @param User $user */ - public function __construct( - private AlbumRepository $albumRepository, - private MediaInformationService $informationService, - private ?Authenticatable $user - ) { + public function __construct(private AlbumRepository $albumRepository, private ?Authenticatable $user) + { } public function index() @@ -32,9 +28,6 @@ class AlbumController extends Controller public function show(Album $album) { - $album = $this->albumRepository->getOne($album->id, $this->user); - $album->information = $this->informationService->getAlbumInformation($album); - - return AlbumResource::make($album); + return AlbumResource::make($this->albumRepository->getOne($album->id, $this->user)); } } diff --git a/app/Http/Controllers/V6/API/ArtistController.php b/app/Http/Controllers/V6/API/ArtistController.php index 1f5b1a98..1c6572d2 100644 --- a/app/Http/Controllers/V6/API/ArtistController.php +++ b/app/Http/Controllers/V6/API/ArtistController.php @@ -32,9 +32,6 @@ class ArtistController extends Controller public function show(Artist $artist) { - $artist = $this->artistRepository->getOne($artist->id, $this->user); - $artist->information = $this->informationService->getArtistInformation($artist); - - return ArtistResource::make($artist); + return ArtistResource::make($this->artistRepository->getOne($artist->id, $this->user)); } } diff --git a/app/Http/Controllers/V6/API/FetchAlbumInformationController.php b/app/Http/Controllers/V6/API/FetchAlbumInformationController.php new file mode 100644 index 00000000..139d7d7c --- /dev/null +++ b/app/Http/Controllers/V6/API/FetchAlbumInformationController.php @@ -0,0 +1,15 @@ +json($informationService->getAlbumInformation($album)); + } +} diff --git a/app/Http/Controllers/V6/API/FetchArtistInformationController.php b/app/Http/Controllers/V6/API/FetchArtistInformationController.php new file mode 100644 index 00000000..704a28fe --- /dev/null +++ b/app/Http/Controllers/V6/API/FetchArtistInformationController.php @@ -0,0 +1,15 @@ +json($informationService->getArtistInformation($artist)); + } +} diff --git a/app/Http/Resources/AlbumResource.php b/app/Http/Resources/AlbumResource.php index ef56c4ac..f10efc12 100644 --- a/app/Http/Resources/AlbumResource.php +++ b/app/Http/Resources/AlbumResource.php @@ -4,7 +4,6 @@ namespace App\Http\Resources; use App\Models\Album; use Illuminate\Http\Resources\Json\JsonResource; -use Illuminate\Support\Arr; class AlbumResource extends JsonResource { @@ -27,7 +26,6 @@ class AlbumResource extends JsonResource 'length' => $this->album->length, 'play_count' => (int) $this->album->play_count, 'song_count' => (int) $this->album->song_count, - 'info' => Arr::wrap($this->album->information), ]; } } diff --git a/app/Http/Resources/ArtistResource.php b/app/Http/Resources/ArtistResource.php index bd658f46..975fc333 100644 --- a/app/Http/Resources/ArtistResource.php +++ b/app/Http/Resources/ArtistResource.php @@ -4,7 +4,6 @@ namespace App\Http\Resources; use App\Models\Artist; use Illuminate\Http\Resources\Json\JsonResource; -use Illuminate\Support\Arr; class ArtistResource extends JsonResource { @@ -26,7 +25,6 @@ class ArtistResource extends JsonResource 'song_count' => (int) $this->artist->song_count, 'album_count' => (int) $this->artist->album_count, 'created_at' => $this->artist->created_at, - 'info' => Arr::wrap($this->artist->information), ]; } } diff --git a/app/Listeners/DownloadAlbumCover.php b/app/Listeners/DownloadAlbumCover.php index 92be6658..b3511657 100644 --- a/app/Listeners/DownloadAlbumCover.php +++ b/app/Listeners/DownloadAlbumCover.php @@ -8,25 +8,17 @@ use Throwable; class DownloadAlbumCover { - private MediaMetadataService $mediaMetadataService; - - public function __construct(MediaMetadataService $mediaMetadataService) + public function __construct(private MediaMetadataService $mediaMetadataService) { - $this->mediaMetadataService = $mediaMetadataService; } public function handle(AlbumInformationFetched $event): void { - $info = $event->getInformation(); - $album = $event->getAlbum(); - - $image = array_get($info, 'image'); - // If our current album has no cover, and Last.fm has one, steal it? - if (!$album->has_cover && $image && ini_get('allow_url_fopen')) { + if (!$event->album->has_cover && $event->information->cover && ini_get('allow_url_fopen')) { try { - $this->mediaMetadataService->downloadAlbumCover($album, $image); - } catch (Throwable $e) { + $this->mediaMetadataService->downloadAlbumCover($event->album, $event->information->cover); + } catch (Throwable) { } } } diff --git a/app/Listeners/DownloadArtistImage.php b/app/Listeners/DownloadArtistImage.php index 831fab12..1aeb7462 100644 --- a/app/Listeners/DownloadArtistImage.php +++ b/app/Listeners/DownloadArtistImage.php @@ -8,25 +8,17 @@ use Throwable; class DownloadArtistImage { - private MediaMetadataService $mediaMetadataService; - - public function __construct(MediaMetadataService $mediaMetadataService) + public function __construct(private MediaMetadataService $mediaMetadataService) { - $this->mediaMetadataService = $mediaMetadataService; } public function handle(ArtistInformationFetched $event): void { - $info = $event->getInformation(); - $artist = $event->getArtist(); - - $image = array_get($info, 'image'); - // If our artist has no image, and Last.fm has one, we steal it? - if (!$artist->has_image && $image && ini_get('allow_url_fopen')) { + if (!$event->artist->has_image && $event->information->image && ini_get('allow_url_fopen')) { try { - $this->mediaMetadataService->downloadArtistImage($artist, $image); - } catch (Throwable $e) { + $this->mediaMetadataService->downloadArtistImage($event->artist, $event->information->image); + } catch (Throwable) { } } } diff --git a/app/Services/LastfmService.php b/app/Services/LastfmService.php index 2545a72f..56e175f9 100644 --- a/app/Services/LastfmService.php +++ b/app/Services/LastfmService.php @@ -3,6 +3,8 @@ namespace App\Services; use App\Models\User; +use App\Values\AlbumInformation; +use App\Values\ArtistInformation; use App\Values\LastfmLoveTrackParameters; use GuzzleHttp\Promise\Promise; use GuzzleHttp\Promise\Utils; @@ -32,8 +34,7 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface return $this->getKey() && $this->getSecret(); } - /** @return array|null */ - public function getArtistInformation(string $name): ?array + public function getArtistInformation(string $name): ?ArtistInformation { if (!$this->enabled()) { return null; @@ -45,14 +46,10 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface return $this->cache->remember( md5("lastfm_artist_$name"), now()->addWeek(), - function () use ($name): ?array { + function () use ($name): ?ArtistInformation { $response = $this->get("?method=artist.getInfo&autocorrect=1&artist=$name&format=json"); - if (!$response || !isset($response->artist)) { - return null; - } - - return $this->buildArtistInformation($response->artist); + return $response?->artist ? ArtistInformation::fromLastFmData($response->artist) : null; } ); } catch (Throwable $e) { @@ -62,27 +59,7 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface } } - /** - * Build a Koel-usable array of artist information using the data from Last.fm. - * - * @param mixed $data - * - * @return array - */ - private function buildArtistInformation($data): array - { - return [ - 'url' => $data->url, - 'image' => count($data->image) > 3 ? $data->image[3]->{'#text'} : $data->image[0]->{'#text'}, - 'bio' => [ - 'summary' => isset($data->bio) ? $this->formatText($data->bio->summary) : '', - 'full' => isset($data->bio) ? $this->formatText($data->bio->content) : '', - ], - ]; - } - - /** @return array|null */ - public function getAlbumInformation(string $albumName, string $artistName): ?array + public function getAlbumInformation(string $albumName, string $artistName): ?AlbumInformation { if (!$this->enabled()) { return null; @@ -97,15 +74,11 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface return $this->cache->remember( $cacheKey, now()->addWeek(), - function () use ($albumName, $artistName): ?array { + function () use ($albumName, $artistName): ?AlbumInformation { $response = $this ->get("?method=album.getInfo&autocorrect=1&album=$albumName&artist=$artistName&format=json"); - if (!$response || !isset($response->album)) { - return null; - } - - return $this->buildAlbumInformation($response->album); + return $response?->album ? AlbumInformation::fromLastFmData($response->album) : null; } ); } catch (Throwable $e) { @@ -115,30 +88,6 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface } } - /** - * Build a Koel-usable array of album information using the data from Last.fm. - * - * @param mixed $data - * - * @return array - */ - private function buildAlbumInformation($data): array - { - return [ - 'url' => $data->url, - 'image' => count($data->image) > 3 ? $data->image[3]->{'#text'} : $data->image[0]->{'#text'}, - 'wiki' => [ - 'summary' => isset($data->wiki) ? $this->formatText($data->wiki->summary) : '', - 'full' => isset($data->wiki) ? $this->formatText($data->wiki->content) : '', - ], - 'tracks' => array_map(static fn ($track): array => [ - 'title' => $track->name, - 'length' => (int) $track->duration, - 'url' => $track->url, - ], isset($data->tracks) ? $data->tracks->track : []), - ]; - } - /** * Get Last.fm's session key for the authenticated user using a token. * @@ -223,14 +172,11 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface } } - /** - * @param int|float $duration Duration of the track, in seconds - */ public function updateNowPlaying( string $artistName, string $trackName, string $albumName, - $duration, + int|float $duration, string $sessionKey ): void { $params = [ @@ -265,7 +211,7 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface * * @return array|string */ - public function buildAuthCallParams(array $params, bool $toString = false) // @phpcs:ignore + public function buildAuthCallParams(array $params, bool $toString = false): array|string { $params['api_key'] = $this->getKey(); ksort($params); @@ -294,18 +240,6 @@ class LastfmService extends AbstractApiClient implements ApiConsumerInterface return rtrim($query, '&'); } - /** - * Correctly format a value returned by Last.fm. - */ - protected function formatText(?string $value): string - { - if (!$value) { - return ''; - } - - return trim(str_replace('Read more on Last.fm', '', nl2br(strip_tags(html_entity_decode($value))))); - } - public function getKey(): ?string { return config('koel.lastfm.key'); diff --git a/app/Services/MediaInformationService.php b/app/Services/MediaInformationService.php index e2c54afd..dc1a6e44 100644 --- a/app/Services/MediaInformationService.php +++ b/app/Services/MediaInformationService.php @@ -8,6 +8,8 @@ use App\Models\Album; use App\Models\Artist; use App\Repositories\AlbumRepository; use App\Repositories\ArtistRepository; +use App\Values\AlbumInformation; +use App\Values\ArtistInformation; class MediaInformationService { @@ -18,12 +20,7 @@ class MediaInformationService ) { } - /** - * Get extra information about an album from Last.fm. - * - * @return array|null the album info in an array format, or null on failure - */ - public function getAlbumInformation(Album $album): ?array + public function getAlbumInformation(Album $album): ?AlbumInformation { if ($album->is_unknown) { return null; @@ -33,20 +30,14 @@ class MediaInformationService if ($info) { event(new AlbumInformationFetched($album, $info)); - // The album cover may have been updated. - $info['cover'] = $this->albumRepository->getOneById($album->id)->cover; + $info->cover = $this->albumRepository->getOneById($album->id)->cover; } return $info; } - /** - * Get extra information about an artist from Last.fm. - * - * @return array|null the artist info in an array format, or null on failure - */ - public function getArtistInformation(Artist $artist): ?array + public function getArtistInformation(Artist $artist): ?ArtistInformation { if ($artist->is_unknown) { return null; @@ -56,9 +47,8 @@ class MediaInformationService if ($info) { event(new ArtistInformationFetched($artist, $info)); - // The artist image may have been updated. - $info['image'] = $this->artistRepository->getOneById($artist->id)->image; + $info->image = $this->artistRepository->getOneById($artist->id)->image; } return $info; diff --git a/app/Values/AlbumInformation.php b/app/Values/AlbumInformation.php new file mode 100644 index 00000000..5c0d98dc --- /dev/null +++ b/app/Values/AlbumInformation.php @@ -0,0 +1,46 @@ +url, + cover: count($data->image) > 3 ? $data->image[3]->{'#text'} : $data->image[0]->{'#text'}, + wiki: [ + 'summary' => isset($data->wiki) ? self::formatLastFmText($data->wiki->summary) : '', + 'full' => isset($data->wiki) ? self::formatLastFmText($data->wiki->content) : '', + ], + tracks: array_map(static fn ($track): array => [ + 'title' => $track->name, + 'length' => (int) $track->duration, + 'url' => $track->url, + ], $data->tracks?->track ?? []), + ); + } + + /** @return array */ + public function toArray(): array + { + return [ + 'url' => $this->url, + 'cover' => $this->cover, + 'wiki' => $this->wiki, + 'tracks' => $this->tracks, + ]; + } +} diff --git a/app/Values/ArtistInformation.php b/app/Values/ArtistInformation.php new file mode 100644 index 00000000..30dc4999 --- /dev/null +++ b/app/Values/ArtistInformation.php @@ -0,0 +1,36 @@ +url, + image: count($data->image) > 3 ? $data->image[3]->{'#text'} : $data->image[0]->{'#text'}, + bio: [ + 'summary' => isset($data->bio) ? self::formatLastFmText($data->bio->summary) : '', + 'full' => isset($data->bio) ? self::formatLastFmText($data->bio->content) : '', + ], + ); + } + + /** @return array */ + public function toArray(): array + { + return [ + 'url' => $this->url, + 'image' => $this->image, + 'bio' => $this->bio, + ]; + } +} diff --git a/app/Values/FormatsLastFmText.php b/app/Values/FormatsLastFmText.php new file mode 100644 index 00000000..64cd3567 --- /dev/null +++ b/app/Values/FormatsLastFmText.php @@ -0,0 +1,16 @@ + { id: faker.datatype.number({ min: 2 }), // avoid Unknown Album by default name: faker.lorem.sentence(), cover: faker.image.imageUrl(), - info: { - image: faker.image.imageUrl(), - wiki: { - summary: faker.lorem.sentence(), - full: faker.lorem.paragraph() - }, - tracks: [ - { - title: faker.lorem.sentence(), - length: 222, - fmt_length: '3:42' - }, - { - title: faker.lorem.sentence(), - length: 157, - fmt_length: '2:37' - } - ], - url: faker.internet.url() - }, play_count: faker.datatype.number(), length, fmt_length: secondsToHis(length), diff --git a/resources/assets/js/__tests__/factory/artistFactory.ts b/resources/assets/js/__tests__/factory/artistFactory.ts index 3fd93c91..392e712b 100644 --- a/resources/assets/js/__tests__/factory/artistFactory.ts +++ b/resources/assets/js/__tests__/factory/artistFactory.ts @@ -8,14 +8,6 @@ export default (faker: Faker): Artist => { type: 'artists', id: faker.datatype.number({ min: 3 }), // avoid Unknown and Various Artist by default name: faker.name.findName(), - info: { - image: faker.image.imageUrl(), - bio: { - summary: faker.lorem.sentence(), - full: faker.lorem.paragraph() - }, - url: faker.internet.url() - }, image: 'foo.jpg', play_count: faker.datatype.number(), album_count: faker.datatype.number({ max: 10 }), diff --git a/resources/assets/js/components/album/AlbumInfo.vue b/resources/assets/js/components/album/AlbumInfo.vue index 63eb60f4..a1c95a56 100644 --- a/resources/assets/js/components/album/AlbumInfo.vue +++ b/resources/assets/js/components/album/AlbumInfo.vue @@ -10,21 +10,21 @@
- diff --git a/resources/assets/js/components/album/AlbumTrackListItem.vue b/resources/assets/js/components/album/AlbumTrackListItem.vue index ae55f1ea..21007011 100644 --- a/resources/assets/js/components/album/AlbumTrackListItem.vue +++ b/resources/assets/js/components/album/AlbumTrackListItem.vue @@ -1,5 +1,6 @@ diff --git a/resources/assets/js/components/artist/ArtistInfo.vue b/resources/assets/js/components/artist/ArtistInfo.vue index e8e50dd1..a71fee1d 100644 --- a/resources/assets/js/components/artist/ArtistInfo.vue +++ b/resources/assets/js/components/artist/ArtistInfo.vue @@ -7,22 +7,22 @@ -
+
-