From 5fbec01c50a984f404de862334e16ff218bd7bc5 Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 19 Aug 2018 11:05:33 +0200 Subject: [PATCH] Big revamp for artist and album info --- app/Console/Commands/SyncMedia.php | 2 - app/Events/AlbumInformationFetched.php | 36 ++++++ app/Events/ArtistInformationFetched.php | 36 ++++++ app/Events/Event.php | 10 +- app/Events/LibraryChanged.php | 16 --- app/Events/SongLikeToggled.php | 10 -- app/Events/SongStartedPlaying.php | 10 -- .../API/ObjectStorage/S3/SongController.php | 10 +- app/Listeners/ClearMediaCache.php | 7 -- app/Listeners/DownloadAlbumCover.php | 29 +++++ app/Listeners/DownloadArtistImage.php | 29 +++++ app/Models/Album.php | 67 ---------- app/Models/Artist.php | 28 ----- app/Models/File.php | 45 ++++--- app/Providers/EventServiceProvider.php | 33 +++-- app/Providers/MediaServiceProvider.php | 4 +- app/Services/MediaInformationService.php | 34 ++--- app/Services/MediaMetadataService.php | 117 ++++++++++++++++++ app/Services/{Media.php => MediaService.php} | 16 ++- composer.json | 3 +- tests/Feature/DownloadTest.php | 4 + .../{MediaTest.php => MediaSyncTest.php} | 74 ++++++----- tests/Feature/TestCase.php | 21 +--- .../Listeners/DownloadAlbumCoverTest.php | 48 +++++++ .../Listeners/DownloadArtistImageTest.php | 48 +++++++ tests/Integration/Models/AlbumTest.php | 39 ------ tests/Integration/Models/ArtistTest.php | 20 --- .../Services/MediaInformationServiceTest.php | 29 ++++- .../Services/MediaMetadataServiceTest.php | 64 ++++++++++ tests/TestCase.php | 3 +- tests/Traits/InteractsWithIoc.php | 24 ++++ 31 files changed, 608 insertions(+), 308 deletions(-) create mode 100644 app/Events/AlbumInformationFetched.php create mode 100644 app/Events/ArtistInformationFetched.php create mode 100644 app/Listeners/DownloadAlbumCover.php create mode 100644 app/Listeners/DownloadArtistImage.php create mode 100644 app/Services/MediaMetadataService.php rename app/Services/{Media.php => MediaService.php} (95%) rename tests/Feature/{MediaTest.php => MediaSyncTest.php} (82%) create mode 100644 tests/Integration/Listeners/DownloadAlbumCoverTest.php create mode 100644 tests/Integration/Listeners/DownloadArtistImageTest.php create mode 100644 tests/Integration/Services/MediaMetadataServiceTest.php create mode 100644 tests/Traits/InteractsWithIoc.php diff --git a/app/Console/Commands/SyncMedia.php b/app/Console/Commands/SyncMedia.php index dc9c4e94..8aca5b1b 100644 --- a/app/Console/Commands/SyncMedia.php +++ b/app/Console/Commands/SyncMedia.php @@ -40,8 +40,6 @@ class SyncMedia extends Command /** * Execute the console command. - * - * @return mixed */ public function handle() { diff --git a/app/Events/AlbumInformationFetched.php b/app/Events/AlbumInformationFetched.php new file mode 100644 index 00000000..a0c55b0a --- /dev/null +++ b/app/Events/AlbumInformationFetched.php @@ -0,0 +1,36 @@ +album = $album; + $this->information = $information; + } + + /** + * @return Album + */ + public function getAlbum() + { + return $this->album; + } + + /** + * @return array + */ + public function getInformation() + { + return $this->information; + } +} diff --git a/app/Events/ArtistInformationFetched.php b/app/Events/ArtistInformationFetched.php new file mode 100644 index 00000000..66657851 --- /dev/null +++ b/app/Events/ArtistInformationFetched.php @@ -0,0 +1,36 @@ +artist = $artist; + $this->information = $information; + } + + /** + * @return Artist + */ + public function getArtist() + { + return $this->artist; + } + + /** + * @return array + */ + public function getInformation() + { + return $this->information; + } +} diff --git a/app/Events/Event.php b/app/Events/Event.php index ba2f8883..2982f2e6 100644 --- a/app/Events/Event.php +++ b/app/Events/Event.php @@ -4,5 +4,13 @@ namespace App\Events; abstract class Event { - // + /** + * Get the channels the event should be broadcast on. + * + * @return array + */ + public function broadcastOn() + { + return []; + } } diff --git a/app/Events/LibraryChanged.php b/app/Events/LibraryChanged.php index b4504267..64eb99ef 100644 --- a/app/Events/LibraryChanged.php +++ b/app/Events/LibraryChanged.php @@ -4,20 +4,4 @@ namespace App\Events; class LibraryChanged extends Event { - /** - * Create a new event instance. - */ - public function __construct() - { - } - - /** - * Get the channels the event should be broadcast on. - * - * @return array - */ - public function broadcastOn() - { - return []; - } } diff --git a/app/Events/SongLikeToggled.php b/app/Events/SongLikeToggled.php index ec5c4588..dc147e3e 100644 --- a/app/Events/SongLikeToggled.php +++ b/app/Events/SongLikeToggled.php @@ -35,14 +35,4 @@ class SongLikeToggled extends Event $this->interaction = $interaction; $this->user = $user ?: auth()->user(); } - - /** - * Get the channels the event should be broadcast on. - * - * @return array - */ - public function broadcastOn() - { - return []; - } } diff --git a/app/Events/SongStartedPlaying.php b/app/Events/SongStartedPlaying.php index 4b77d38c..9ce5a092 100644 --- a/app/Events/SongStartedPlaying.php +++ b/app/Events/SongStartedPlaying.php @@ -35,14 +35,4 @@ class SongStartedPlaying extends Event $this->song = $song; $this->user = $user; } - - /** - * Get the channels the event should be broadcast on. - * - * @return array - */ - public function broadcastOn() - { - return []; - } } diff --git a/app/Http/Controllers/API/ObjectStorage/S3/SongController.php b/app/Http/Controllers/API/ObjectStorage/S3/SongController.php index 090d6fec..e9dec958 100644 --- a/app/Http/Controllers/API/ObjectStorage/S3/SongController.php +++ b/app/Http/Controllers/API/ObjectStorage/S3/SongController.php @@ -8,12 +8,20 @@ use App\Http\Requests\API\ObjectStorage\S3\RemoveSongRequest; use App\Models\Album; use App\Models\Artist; use App\Models\Song; +use App\Services\MediaMetadataService; use Exception; use Illuminate\Http\JsonResponse; use Media; class SongController extends Controller { + private $mediaMetadataService; + + public function __construct(MediaMetadataService $mediaMetadataService) + { + $this->mediaMetadataService = $mediaMetadataService; + } + /** * Store a new song or update an existing one with data from AWS. * @@ -32,7 +40,7 @@ class SongController extends Controller $album = Album::get($artist, array_get($tags, 'album'), $compilation); if ($cover = array_get($tags, 'cover')) { - $album->writeCoverFile(base64_decode($cover['data']), $cover['extension']); + $this->mediaMetadataService->writeAlbumCover($album, base64_decode($cover['data']), $cover['extension']); } $song = Song::updateOrCreate(['id' => Media::getHash($path)], [ diff --git a/app/Listeners/ClearMediaCache.php b/app/Listeners/ClearMediaCache.php index 623955cb..893ac91a 100644 --- a/app/Listeners/ClearMediaCache.php +++ b/app/Listeners/ClearMediaCache.php @@ -6,13 +6,6 @@ use MediaCache; class ClearMediaCache { - /** - * Create the event listener. - */ - public function __construct() - { - } - /** * Fired every time a LibraryChanged event is triggered. * Clears the media cache. diff --git a/app/Listeners/DownloadAlbumCover.php b/app/Listeners/DownloadAlbumCover.php new file mode 100644 index 00000000..35a6f34a --- /dev/null +++ b/app/Listeners/DownloadAlbumCover.php @@ -0,0 +1,29 @@ +mediaMetadataService = $mediaMetadataService; + } + + public function handle(AlbumInformationFetched $event) + { + $info = $event->getInformation(); + $album = $event->getAlbum(); + + $image = array_get($info, 'image'); + + // If our current album has no cover, and Last.fm has one, why don't we steal it? + if (!$album->has_cover && is_string($image) && ini_get('allow_url_fopen')) { + $this->mediaMetadataService->downloadAlbumCover($album, $image); + } + } +} diff --git a/app/Listeners/DownloadArtistImage.php b/app/Listeners/DownloadArtistImage.php new file mode 100644 index 00000000..5d9cb9a2 --- /dev/null +++ b/app/Listeners/DownloadArtistImage.php @@ -0,0 +1,29 @@ +mediaMetadataService = $mediaMetadataService; + } + + public function handle(ArtistInformationFetched $event) + { + $info = $event->getInformation(); + $artist = $event->getArtist(); + + $image = array_get($info, 'image'); + + // If our current album has no cover, and Last.fm has one, why don't we steal it? + if (!$artist->has_image && is_string($image) && ini_get('allow_url_fopen')) { + $this->mediaMetadataService->downloadArtistImage($artist, $image); + } + } +} diff --git a/app/Models/Album.php b/app/Models/Album.php index 01399905..6de1700b 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -84,73 +84,6 @@ class Album extends Model ]); } - /** - * Generate a cover from provided data. - * - * @param array $cover The cover data in array format, extracted by getID3. - * For example: - * [ - * 'data' => '', - * 'image_mime' => 'image/png', - * 'image_width' => 512, - * 'image_height' => 512, - * 'imagetype' => 'PNG', // not always present - * 'picturetype' => 'Other', - * 'description' => '', - * 'datalength' => 7627, - * ] - */ - public function generateCover(array $cover) - { - $extension = explode('/', $cover['image_mime']); - $extension = empty($extension[1]) ? 'png' : $extension[1]; - - $this->writeCoverFile($cover['data'], $extension); - } - - /** - * Write a cover image file with binary data and update the Album with the new cover file. - * - * @param string $binaryData - * @param string $extension The file extension - * @param string $destination The destination path. Automatically generated if empty. - */ - public function writeCoverFile($binaryData, $extension, $destination = '') - { - $extension = trim(strtolower($extension), '. '); - $destination = $destination ?: $this->generateRandomCoverPath($extension); - file_put_contents($destination, $binaryData); - - $this->update(['cover' => basename($destination)]); - } - - /** - * Copy a cover file from an existing image on the system. - * - * @param string $source The original image's full path. - * @param string $destination The destination path. Automatically generated if empty. - */ - public function copyCoverFile($source, $destination = '') - { - $extension = pathinfo($source, PATHINFO_EXTENSION); - $destination = $destination ?: $this->generateRandomCoverPath($extension); - copy($source, $destination); - - $this->update(['cover' => basename($destination)]); - } - - /** - * Generate a random path for the cover image. - * - * @param string $extension The extension of the cover (without dot) - * - * @return string - */ - private function generateRandomCoverPath($extension) - { - return app()->publicPath().'/public/img/covers/'.uniqid('', true).".$extension"; - } - /** * Set the album cover. * diff --git a/app/Models/Artist.php b/app/Models/Artist.php index eebc33ea..a39529d9 100644 --- a/app/Models/Artist.php +++ b/app/Models/Artist.php @@ -115,34 +115,6 @@ class Artist extends Model return self::firstOrCreate(compact('name'), compact('name')); } - /** - * Write an artist image file with binary data and update the Artist with the new cover file. - * - * @param string $binaryData - * @param string $extension The file extension - * @param string $destination The destination path. Automatically generated if empty. - */ - public function writeImageFile($binaryData, $extension, $destination = '') - { - $extension = trim(strtolower($extension), '. '); - $destination = $destination ?: $this->generateRandomImagePath($extension); - file_put_contents($destination, $binaryData); - - $this->update(['image' => basename($destination)]); - } - - /** - * Generate a random path for the artist's image. - * - * @param string $extension The extension of the cover (without dot) - * - * @return string - */ - private function generateRandomImagePath($extension) - { - return app()->publicPath().'/public/img/artists/'.uniqid('', true).".$extension"; - } - /** * Turn the image name into its absolute URL. * diff --git a/app/Models/File.php b/app/Models/File.php index 0340d093..18ca790c 100644 --- a/app/Models/File.php +++ b/app/Models/File.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Services\MediaMetadataService; use Cache; use Exception; use getID3; +use getid3_exception; use getid3_lib; -use Illuminate\Support\Facades\Log; -use Media; +use InvalidArgumentException; use SplFileInfo; use Symfony\Component\Finder\Finder; @@ -42,6 +43,11 @@ class File */ protected $getID3; + /** + * @var MediaMetadataService + */ + private $mediaMetadataService; + /** * The SplFileInfo object of the file. * @@ -71,13 +77,17 @@ class File * Construct our File object. * Upon construction, we'll set the path, hash, and associated Song object (if any). * - * @param string|SplFileInfo $path Either the file's path, or a SplFileInfo object - * @param getID3 $getID3 A getID3 object for DI (and better performance) + * @param string|SplFileInfo $path Either the file's path, or a SplFileInfo object + * @param getID3|null $getID3 A getID3 object + * @param MediaMetadataService|null $mediaMetadataService + * + * @throws getid3_exception */ - public function __construct($path, $getID3 = null) + public function __construct($path, getID3 $getID3 = null, MediaMetadataService $mediaMetadataService = null) { $this->splFileInfo = $path instanceof SplFileInfo ? $path : new SplFileInfo($path); $this->setGetID3($getID3); + $this->setMediaMetadataService($mediaMetadataService); // Workaround for #344, where getMTime() fails for certain files with Unicode names on Windows. try { @@ -260,18 +270,17 @@ class File { // If the album has no cover, we try to get the cover image from existing tag data if ($coverData) { - try { - $album->generateCover($coverData); + $extension = explode('/', $coverData['image_mime']); + $extension = empty($extension[1]) ? 'png' : $extension[1]; - return; - } catch (Exception $e) { - Log::error($e); - } + $this->mediaMetadataService->writeAlbumCover($album, $coverData['data'], $extension); + + return; } // Or, if there's a cover image under the same directory, use it. if ($cover = $this->getCoverFileUnderSameDirectory()) { - $album->copyCoverFile($cover); + $this->mediaMetadataService->copyAlbumCover($album, $cover); } } @@ -325,8 +334,10 @@ class File /** * @param getID3 $getID3 + * + * @throws getid3_exception */ - public function setGetID3($getID3 = null) + public function setGetID3(getID3 $getID3 = null) { $this->getID3 = $getID3 ?: new getID3(); } @@ -344,7 +355,7 @@ class File * Some albums have its own cover image under the same directory as cover|folder.jpg/png. * We'll check if such a cover file is found, and use it if positive. * - * @throws \InvalidArgumentException + * @throws InvalidArgumentException * * @return string|false The cover file's full path, or false if none found */ @@ -364,6 +375,7 @@ class File ); $cover = $matches ? $matches[0] : false; + // Even if a file is found, make sure it's a real image. if ($cover && exif_imagetype($cover) === false) { $cover = false; @@ -384,4 +396,9 @@ class File { return md5(config('app.key').$path); } + + private function setMediaMetadataService(MediaMetadataService $mediaMetadataService = null) + { + $this->mediaMetadataService = $mediaMetadataService ?: app(MediaMetadataService::class); + } } diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 4f671bc3..9bf3aae4 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -2,6 +2,17 @@ namespace App\Providers; +use App\Events\AlbumInformationFetched; +use App\Events\ArtistInformationFetched; +use App\Events\LibraryChanged; +use App\Events\SongLikeToggled; +use App\Events\SongStartedPlaying; +use App\Listeners\ClearMediaCache; +use App\Listeners\DownloadAlbumCover; +use App\Listeners\DownloadArtistImage; +use App\Listeners\LoveTrackOnLastfm; +use App\Listeners\TidyLibrary; +use App\Listeners\UpdateLastfmNowPlaying; use App\Models\Album; use App\Models\File; use App\Models\Song; @@ -16,17 +27,25 @@ class EventServiceProvider extends ServiceProvider * @var array */ protected $listen = [ - 'App\Events\SongLikeToggled' => [ - 'App\Listeners\LoveTrackOnLastfm', + SongLikeToggled::class => [ + LoveTrackOnLastfm::class, ], - 'App\Events\SongStartedPlaying' => [ - 'App\Listeners\UpdateLastfmNowPlaying', + SongStartedPlaying::class => [ + UpdateLastfmNowPlaying::class, ], - 'App\Events\LibraryChanged' => [ - 'App\Listeners\TidyLibrary', - 'App\Listeners\ClearMediaCache', + LibraryChanged::class => [ + TidyLibrary::class, + ClearMediaCache::class, + ], + + AlbumInformationFetched::class => [ + DownloadAlbumCover::class, + ], + + ArtistInformationFetched::class => [ + DownloadArtistImage::class, ], ]; diff --git a/app/Providers/MediaServiceProvider.php b/app/Providers/MediaServiceProvider.php index 1e5e76aa..7615ed79 100644 --- a/app/Providers/MediaServiceProvider.php +++ b/app/Providers/MediaServiceProvider.php @@ -2,7 +2,7 @@ namespace App\Providers; -use App\Services\Media; +use App\Services\MediaService; use Illuminate\Support\ServiceProvider; class MediaServiceProvider extends ServiceProvider @@ -25,7 +25,7 @@ class MediaServiceProvider extends ServiceProvider public function register() { app()->singleton('Media', function () { - return new Media(); + return app()->make(MediaService::class); }); } } diff --git a/app/Services/MediaInformationService.php b/app/Services/MediaInformationService.php index b92228c6..9beb112d 100644 --- a/app/Services/MediaInformationService.php +++ b/app/Services/MediaInformationService.php @@ -2,10 +2,10 @@ namespace App\Services; +use App\Events\AlbumInformationFetched; +use App\Events\ArtistInformationFetched; use App\Models\Album; use App\Models\Artist; -use Exception; -use Log; class MediaInformationService { @@ -30,18 +30,11 @@ class MediaInformationService } $info = $this->lastfmService->getAlbumInfo($album->name, $album->artist->name); - $image = array_get($info, 'image'); + event(new AlbumInformationFetched($album, $info)); - // If our current album has no cover, and Last.fm has one, why don't we steal it? - if (!$album->has_cover && is_string($image) && ini_get('allow_url_fopen')) { - try { - $extension = explode('.', $image); - $album->writeCoverFile(file_get_contents($image), last($extension)); - $info['cover'] = $album->cover; - } catch (Exception $e) { - Log::error($e); - } - } + // The album may have been updated. + $album->refresh(); + $info['cover'] = $album->cover; return $info; } @@ -60,18 +53,11 @@ class MediaInformationService } $info = $this->lastfmService->getArtistInfo($artist->name); - $image = array_get($info, 'image'); + event(new ArtistInformationFetched($artist, $info)); - // If our current artist has no image, and Last.fm has one, copy the image for our local use. - if (!$artist->has_image && is_string($image) && ini_get('allow_url_fopen')) { - try { - $extension = explode('.', $image); - $artist->writeImageFile(file_get_contents($image), last($extension)); - $info['image'] = $artist->image; - } catch (Exception $e) { - Log::error($e); - } - } + // The artist may have been updated. + $artist->refresh(); + $info['image'] = $artist->image; return $info; } diff --git a/app/Services/MediaMetadataService.php b/app/Services/MediaMetadataService.php new file mode 100644 index 00000000..923a5f31 --- /dev/null +++ b/app/Services/MediaMetadataService.php @@ -0,0 +1,117 @@ +writeAlbumCover($album, file_get_contents($imageUrl), last($extension)); + } + + /** + * Copy a cover file from an existing image on the system. + * + * @param Album $album + * @param string $source The original image's full path. + * @param string $destination The destination path. Automatically generated if empty. + */ + public function copyAlbumCover(Album $album, $source, $destination = '') + { + $extension = pathinfo($source, PATHINFO_EXTENSION); + $destination = $destination ?: $this->generateAlbumCoverPath($extension); + copy($source, $destination); + + $album->update(['cover' => basename($destination)]); + } + + /** + * Write an album cover image file with binary data and update the Album with the new cover attribute. + * + * @param Album $album + * @param string $binaryData + * @param string $extension The file extension + * @param string $destination The destination path. Automatically generated if empty. + */ + public function writeAlbumCover(Album $album, $binaryData, $extension, $destination = '') + { + try { + $extension = trim(strtolower($extension), '. '); + $destination = $destination ?: $this->generateAlbumCoverPath($extension); + file_put_contents($destination, $binaryData); + + $album->update(['cover' => basename($destination)]); + } catch (Exception $e) { + Log::error($e); + } + } + + /** + * Download a copy of the artist image. + * + * @param Artist $artist + * @param string $imageUrl + */ + public function downloadArtistImage(Artist $artist, $imageUrl) + { + $extension = explode('.', $imageUrl); + $this->writeArtistImage($artist, file_get_contents($imageUrl), last($extension)); + } + + /** + * Write an artist image file with binary data and update the Artist with the new image attribute. + * + * @param Artist $artist + * @param string $binaryData + * @param string $extension The file extension + * @param string $destination The destination path. Automatically generated if empty. + */ + public function writeArtistImage(Artist $artist, $binaryData, $extension, $destination = '') + { + try { + $extension = trim(strtolower($extension), '. '); + $destination = $destination ?: $this->generateArtistImagePath($extension); + file_put_contents($destination, $binaryData); + + $artist->update(['image' => basename($destination)]); + } catch (Exception $e) { + Log::error($e); + } + } + + /** + * Generate a random path for an album cover image. + * + * @param string $extension The extension of the cover (without dot) + * + * @return string + */ + private function generateAlbumCoverPath($extension) + { + return sprintf('%s/public/img/covers/%s.%s', app()->publicPath(), uniqid('', true), $extension); + } + + /** + * Generate a random path for an artist image. + * + * @param string $extension The extension of the cover (without dot) + * + * @return string + */ + private function generateArtistImagePath($extension) + { + return sprintf('%s/public/img/artists/%s.%s', app()->publicPath(), uniqid('', true), $extension); + } +} diff --git a/app/Services/Media.php b/app/Services/MediaService.php similarity index 95% rename from app/Services/Media.php rename to app/Services/MediaService.php index 3540c741..62b90a2d 100644 --- a/app/Services/Media.php +++ b/app/Services/MediaService.php @@ -12,10 +12,11 @@ use App\Models\Setting; use App\Models\Song; use Exception; use getID3; -use Illuminate\Support\Facades\Log; +use getid3_exception; +use Log; use Symfony\Component\Finder\Finder; -class Media +class MediaService { /** * All applicable tags in a media file that we cater for. @@ -36,6 +37,13 @@ class Media 'compilation', ]; + private $mediaMetadataService; + + public function __construct(MediaMetadataService $mediaMetadataService) + { + $this->mediaMetadataService = $mediaMetadataService; + } + /** * Tags to be synced. * @@ -79,7 +87,7 @@ class Media $syncCommand && $syncCommand->createProgressBar(count($songPaths)); foreach ($songPaths as $path) { - $file = new File($path, $getID3); + $file = new File($path, $getID3, $this->mediaMetadataService); switch ($result = $file->sync($this->tags, $force)) { case File::SYNC_RESULT_SUCCESS: @@ -181,6 +189,8 @@ class Media * Sync a directory's watch record. * * @param WatchRecordInterface $record + * + * @throws getid3_exception */ private function syncDirectoryRecord(WatchRecordInterface $record) { diff --git a/composer.json b/composer.json index 2b5c1d87..152cabe6 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "pusher/pusher-php-server": "^2.6", "predis/predis": "~1.0", "doctrine/dbal": "^2.5", - "jackiedo/dotenv-editor": "^1.0" + "jackiedo/dotenv-editor": "^1.0", + "ext-exif": "*" }, "require-dev": { "fzaninotto/faker": "~1.4", diff --git a/tests/Feature/DownloadTest.php b/tests/Feature/DownloadTest.php index 55ed934c..13fc29a2 100644 --- a/tests/Feature/DownloadTest.php +++ b/tests/Feature/DownloadTest.php @@ -8,6 +8,7 @@ use App\Models\Playlist; use App\Models\Song; use App\Models\User; use App\Services\DownloadService; +use Exception; use Mockery\MockInterface; class DownloadTest extends TestCase @@ -17,6 +18,9 @@ class DownloadTest extends TestCase */ private $downloadService; + /** + * @throws Exception + */ public function setUp() { parent::setUp(); diff --git a/tests/Feature/MediaTest.php b/tests/Feature/MediaSyncTest.php similarity index 82% rename from tests/Feature/MediaTest.php rename to tests/Feature/MediaSyncTest.php index 90e2ed32..734d922c 100644 --- a/tests/Feature/MediaTest.php +++ b/tests/Feature/MediaSyncTest.php @@ -8,15 +8,27 @@ use App\Models\Album; use App\Models\Artist; use App\Models\File; use App\Models\Song; -use App\Services\Media; +use App\Services\MediaService; +use Exception; use getID3; +use getid3_exception; use Illuminate\Foundation\Testing\WithoutMiddleware; use Mockery as m; -class MediaTest extends TestCase +class MediaSyncTest extends TestCase { use WithoutMiddleware; + /** @var MediaService */ + private $mediaService; + + public function setUp() + { + parent::setUp(); + + $this->mediaService = app(MediaService::class); + } + protected function tearDown() { m::close(); @@ -26,14 +38,13 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function songs_can_be_synced() { $this->expectsEvents(LibraryChanged::class); - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); // Standard mp3 files under root path should be recognized $this->seeInDatabase('songs', [ @@ -82,7 +93,7 @@ class MediaTest extends TestCase // Modified file should be recognized touch($song->path, $time = time()); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); $song = Song::find($song->id); $this->assertEquals($time, $song->mtime); @@ -93,14 +104,13 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function songs_can_be_force_synced() { $this->expectsEvents(LibraryChanged::class); - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); // Make some modification to the records $song = Song::orderBy('id', 'desc')->first(); @@ -113,7 +123,7 @@ class MediaTest extends TestCase ]); // Resync without forcing - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); // Validate that the changes are not lost $song = Song::orderBy('id', 'desc')->first(); @@ -121,7 +131,7 @@ class MediaTest extends TestCase $this->assertEquals('Booom Wroooom', $song->lyrics); // Resync with force - $media->sync($this->mediaPath, [], true); + $this->mediaService->sync($this->mediaPath, [], true); // All is lost. $song = Song::orderBy('id', 'desc')->first(); @@ -132,14 +142,13 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function songs_can_be_synced_with_selectively_tags() { $this->expectsEvents(LibraryChanged::class); - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); // Make some modification to the records $song = Song::orderBy('id', 'desc')->first(); @@ -151,7 +160,7 @@ class MediaTest extends TestCase ]); // Sync only the selective tags - $media->sync($this->mediaPath, ['title'], true); + $this->mediaService->sync($this->mediaPath, ['title'], true); // Validate that the specified tags are changed, other remains the same $song = Song::orderBy('id', 'desc')->first(); @@ -162,20 +171,19 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function all_tags_are_catered_for_if_syncing_new_file() { // First we sync the test directory to get the data - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); // Now delete the first song. $song = Song::orderBy('id')->first(); $song->delete(); // Selectively sync only one tag - $media->sync($this->mediaPath, ['track'], true); + $this->mediaService->sync($this->mediaPath, ['track'], true); // but we still expect the whole song to be added back with all info $addedSong = Song::findOrFail($song->id)->toArray(); @@ -188,7 +196,7 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function added_song_is_synced_when_watching() { @@ -196,7 +204,7 @@ class MediaTest extends TestCase $path = $this->mediaPath.'/blank.mp3'; - (new Media())->syncByWatchRecord(new InotifyWatchRecord("CLOSE_WRITE,CLOSE $path")); + $this->mediaService->syncByWatchRecord(new InotifyWatchRecord("CLOSE_WRITE,CLOSE $path")); $this->seeInDatabase('songs', ['path' => $path]); } @@ -204,7 +212,7 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function deleted_song_is_synced_when_watching() { @@ -213,7 +221,7 @@ class MediaTest extends TestCase $this->createSampleMediaSet(); $song = Song::orderBy('id', 'desc')->first(); - (new Media())->syncByWatchRecord(new InotifyWatchRecord("DELETE {$song->path}")); + $this->mediaService->syncByWatchRecord(new InotifyWatchRecord("DELETE {$song->path}")); $this->notSeeInDatabase('songs', ['id' => $song->id]); } @@ -221,23 +229,26 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function deleted_directory_is_synced_when_watching() { $this->expectsEvents(LibraryChanged::class); - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); - $media->syncByWatchRecord(new InotifyWatchRecord("MOVED_FROM,ISDIR {$this->mediaPath}/subdir")); + $this->mediaService->syncByWatchRecord(new InotifyWatchRecord("MOVED_FROM,ISDIR {$this->mediaPath}/subdir")); $this->notSeeInDatabase('songs', ['path' => $this->mediaPath.'/subdir/sic.mp3']); $this->notSeeInDatabase('songs', ['path' => $this->mediaPath.'/subdir/no-name.mp3']); $this->notSeeInDatabase('songs', ['path' => $this->mediaPath.'/subdir/back-in-black.mp3']); } - /** @test */ + /** + * @test + * + * @throws getid3_exception + */ public function html_entities_in_tags_are_recognized_and_saved_properly() { $getID3 = m::mock(getID3::class, [ @@ -264,17 +275,16 @@ class MediaTest extends TestCase /** * @test * - * @throws \Exception + * @throws Exception */ public function hidden_files_can_optionally_be_ignored_when_syncing() { config(['koel.ignore_dot_files' => false]); - $media = new Media(); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); $this->seeInDatabase('albums', ['name' => 'Hidden Album']); config(['koel.ignore_dot_files' => true]); - $media->sync($this->mediaPath); + $this->mediaService->sync($this->mediaPath); $this->notSeeInDatabase('albums', ['name' => 'Hidden Album']); } } diff --git a/tests/Feature/TestCase.php b/tests/Feature/TestCase.php index 183c86e8..f1dd87ec 100644 --- a/tests/Feature/TestCase.php +++ b/tests/Feature/TestCase.php @@ -6,15 +6,16 @@ use App\Models\Album; use App\Models\Artist; use App\Models\Song; use App\Models\User; +use Exception; use JWTAuth; use Laravel\BrowserKitTesting\DatabaseTransactions; use Laravel\BrowserKitTesting\TestCase as BaseTestCase; -use Mockery as m; use Tests\CreatesApplication; +use Tests\Traits\InteractsWithIoc; abstract class TestCase extends BaseTestCase { - use CreatesApplication, DatabaseTransactions; + use CreatesApplication, DatabaseTransactions, InteractsWithIoc; public function setUp() { @@ -24,6 +25,7 @@ abstract class TestCase extends BaseTestCase /** * Create a sample media set, with a complete artist+album+song trio. + * @throws Exception */ protected function createSampleMediaSet() { @@ -86,19 +88,4 @@ abstract class TestCase extends BaseTestCase 'Authorization' => 'Bearer '.JWTAuth::fromUser($user), ]); } - - /** - * Mock an IOC dependency, for example an injected service in controllers. - * - * @param string $abstract - * @param array $args - * - * @return m\MockInterface - */ - protected function mockIocDependency($abstract, ...$args) - { - return tap(m::mock($abstract, ...$args), function ($mocked) use ($abstract) { - $this->instance($abstract, $mocked); - }); - } } diff --git a/tests/Integration/Listeners/DownloadAlbumCoverTest.php b/tests/Integration/Listeners/DownloadAlbumCoverTest.php new file mode 100644 index 00000000..64b5a647 --- /dev/null +++ b/tests/Integration/Listeners/DownloadAlbumCoverTest.php @@ -0,0 +1,48 @@ +mediaMetaDataService = $this->mockIocDependency(MediaMetadataService::class); + } + + public function testHandle() + { + $album = factory(Album::class)->make(['cover' => null]); + $event = new AlbumInformationFetched($album, ['image' => 'https://foo.bar/baz.jpg']); + + $this->mediaMetaDataService + ->shouldReceive('downloadAlbumCover') + ->once() + ->with($album, 'https://foo.bar/baz.jpg'); + + event($event); + } + } +} diff --git a/tests/Integration/Listeners/DownloadArtistImageTest.php b/tests/Integration/Listeners/DownloadArtistImageTest.php new file mode 100644 index 00000000..56e659e3 --- /dev/null +++ b/tests/Integration/Listeners/DownloadArtistImageTest.php @@ -0,0 +1,48 @@ +mediaMetaDataService = $this->mockIocDependency(MediaMetadataService::class); + } + + public function testHandle() + { + $artist = factory(Artist::class)->make(['image' => null]); + $event = new ArtistInformationFetched($artist, ['image' => 'https://foo.bar/baz.jpg']); + + $this->mediaMetaDataService + ->shouldReceive('downloadArtistImage') + ->once() + ->with($artist, 'https://foo.bar/baz.jpg'); + + event($event); + } + } +} diff --git a/tests/Integration/Models/AlbumTest.php b/tests/Integration/Models/AlbumTest.php index 63d6bb6e..b8034f26 100644 --- a/tests/Integration/Models/AlbumTest.php +++ b/tests/Integration/Models/AlbumTest.php @@ -75,43 +75,4 @@ class AlbumTest extends TestCase $this->assertTrue($album->artist->is_various); } - /** @test */ - public function it_can_write_a_cover_file_and_update_itself_with_the_cover_file() - { - // Given there's an album and a cover file content - /** @var Album $album */ - $album = factory(Album::class)->create(); - $coverContent = 'dummy'; - $root = vfsStream::setup('home'); - $coverPath = vfsStream::url('home/foo.jpg'); - - // When I call the method to write the cover file - $album->writeCoverFile($coverContent, 'jpg', $coverPath); - - // Then I see the cover file is generated - $this->assertTrue($root->hasChild('foo.jpg')); - - // And the album's cover attribute is updated - $this->assertEquals('http://localhost/public/img/covers/foo.jpg', Album::find($album->id)->cover); - } - - /** @test */ - public function it_can_copy_a_cover_file_and_update_itself_with_the_cover_file() - { - // Given there's an album and an original image file - /** @var Album $album */ - $album = factory(Album::class)->create(); - $root = vfsStream::setup('home'); - $imageFile = vfsStream::newFile('foo.jpg')->at($root)->setContent('foo'); - $coverPath = vfsStream::url('home/bar.jpg'); - - // When I call the method to copy the image file as the cover file - $album->copyCoverFile($imageFile->url(), $coverPath); - - // Then I see the cover file is copied - $this->assertTrue($root->hasChild('bar.jpg')); - - // And the album's cover attribute is updated - $this->assertEquals('http://localhost/public/img/covers/bar.jpg', Album::find($album->id)->cover); - } } diff --git a/tests/Integration/Models/ArtistTest.php b/tests/Integration/Models/ArtistTest.php index d875e6b9..28e7b200 100644 --- a/tests/Integration/Models/ArtistTest.php +++ b/tests/Integration/Models/ArtistTest.php @@ -51,26 +51,6 @@ class ArtistTest extends TestCase $this->assertTrue($artist->is_unknown); } - /** @test */ - public function it_can_write_an_image_file_and_update_itself_with_the_image() - { - // Given there's an artist and an image file content - /** @var Artist $artist */ - $artist = factory(Artist::class)->create(); - $imageContent = 'dummy'; - $root = vfsStream::setup('home'); - $imagePath = vfsStream::url('home/foo.jpg'); - - // When I call the method to write the image file - $artist->writeImageFile($imageContent, 'jpg', $imagePath); - - // Then I see the image file is generated - $this->assertTrue($root->hasChild('foo.jpg')); - - // And the artist's image attribute is updated - $this->assertEquals('http://localhost/public/img/artists/foo.jpg', Artist::find($artist->id)->image); - } - /** @test */ public function artists_with_name_in_utf16_encoding_are_retrieved_correctly() { diff --git a/tests/Integration/Services/MediaInformationServiceTest.php b/tests/Integration/Services/MediaInformationServiceTest.php index 8be82891..45086674 100644 --- a/tests/Integration/Services/MediaInformationServiceTest.php +++ b/tests/Integration/Services/MediaInformationServiceTest.php @@ -2,10 +2,13 @@ namespace Tests\Integration\Services; +use App\Events\AlbumInformationFetched; +use App\Events\ArtistInformationFetched; use App\Models\Album; use App\Models\Artist; use App\Services\LastfmService; use App\Services\MediaInformationService; +use Exception; use Mockery as m; use Mockery\MockInterface; use Tests\TestCase; @@ -30,9 +33,13 @@ class MediaInformationServiceTest extends TestCase $this->mediaInformationService = new MediaInformationService($this->lastFmService); } - /** @test */ - public function it_gets_album_information() + /** + * @throws Exception + */ + public function testGetAlbumInformation() { + $this->expectsEvents(AlbumInformationFetched::class); + /** @var Album $album */ $album = factory(Album::class)->create(); @@ -44,12 +51,19 @@ class MediaInformationServiceTest extends TestCase $info = $this->mediaInformationService->getAlbumInformation($album); - self::assertEquals(['foo' => 'bar'], $info); + self::assertEquals([ + 'foo' => 'bar', + 'cover' => $album->cover, + ], $info); } - /** @test */ - public function it_gets_artist_information() + /** + * @throws Exception + */ + public function testGetArtistInformation() { + $this->expectsEvents(ArtistInformationFetched::class); + /** @var Artist $artist */ $artist = factory(Artist::class)->create(); @@ -61,6 +75,9 @@ class MediaInformationServiceTest extends TestCase $info = $this->mediaInformationService->getArtistInformation($artist); - self::assertEquals(['foo' => 'bar'], $info); + self::assertEquals([ + 'foo' => 'bar', + 'image' => $artist->image, + ], $info); } } diff --git a/tests/Integration/Services/MediaMetadataServiceTest.php b/tests/Integration/Services/MediaMetadataServiceTest.php new file mode 100644 index 00000000..f1317982 --- /dev/null +++ b/tests/Integration/Services/MediaMetadataServiceTest.php @@ -0,0 +1,64 @@ +mediaMetadataService = new MediaMetadataService(); + } + + public function testCopyAlbumCover() + { + /** @var Album $album */ + $album = factory(Album::class)->create(); + $root = vfsStream::setup('home'); + $imageFile = vfsStream::newFile('foo.jpg')->at($root)->setContent('foo'); + $coverPath = vfsStream::url('home/bar.jpg'); + + $this->mediaMetadataService->copyAlbumCover($album, $imageFile->url(), $coverPath); + + $this->assertTrue($root->hasChild('bar.jpg')); + $this->assertEquals('http://localhost/public/img/covers/bar.jpg', Album::find($album->id)->cover); + } + + public function testWriteAlbumCover() + { + /** @var Album $album */ + $album = factory(Album::class)->create(); + $coverContent = 'dummy'; + $root = vfsStream::setup('home'); + $coverPath = vfsStream::url('home/foo.jpg'); + + $this->mediaMetadataService->writeAlbumCover($album, $coverContent, 'jpg', $coverPath); + + $this->assertTrue($root->hasChild('foo.jpg')); + $this->assertEquals('http://localhost/public/img/covers/foo.jpg', Album::find($album->id)->cover); + } + + public function testWriteArtistImage() + { + /** @var Artist $artist */ + $artist = factory(Artist::class)->create(); + $imageContent = 'dummy'; + $root = vfsStream::setup('home'); + $imagePath = vfsStream::url('home/foo.jpg'); + + $this->mediaMetadataService->writeArtistImage($artist, $imageContent, 'jpg', $imagePath); + + $this->assertTrue($root->hasChild('foo.jpg')); + $this->assertEquals('http://localhost/public/img/artists/foo.jpg', Artist::find($artist->id)->image); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 27c55c72..90bb4e63 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,10 +4,11 @@ namespace Tests; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Foundation\Testing\TestCase as BaseTestCase; +use Tests\Traits\InteractsWithIoc; abstract class TestCase extends BaseTestCase { - use DatabaseTransactions, CreatesApplication; + use DatabaseTransactions, CreatesApplication, InteractsWithIoc; public function setUp() { diff --git a/tests/Traits/InteractsWithIoc.php b/tests/Traits/InteractsWithIoc.php new file mode 100644 index 00000000..e18e622c --- /dev/null +++ b/tests/Traits/InteractsWithIoc.php @@ -0,0 +1,24 @@ +instance($abstract, $mocked); + }); + } +}