diff --git a/app/Models/Album.php b/app/Models/Album.php index f2080c38..f912992c 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -4,8 +4,10 @@ namespace App\Models; use App\Facades\Lastfm; use App\Traits\SupportsDeleteWhereIDsNotIn; +use Exception; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; +use Log; /** * @property string cover The path to the album's cover @@ -85,9 +87,13 @@ class Album extends Model // If our current album has no cover, and Last.fm has one, why don't we steal it? // Great artists steal for their great albums! if (!$this->has_cover && is_string($image) && ini_get('allow_url_fopen')) { - $extension = explode('.', $image); - $this->writeCoverFile(file_get_contents($image), last($extension)); - $info['cover'] = $this->cover; + try { + $extension = explode('.', $image); + $this->writeCoverFile(file_get_contents($image), last($extension)); + $info['cover'] = $this->cover; + } catch (Exception $e) { + Log::error($e); + } } return $info; diff --git a/app/Models/Artist.php b/app/Models/Artist.php index ab43f90d..67ad43d2 100644 --- a/app/Models/Artist.php +++ b/app/Models/Artist.php @@ -111,12 +111,7 @@ class Artist extends Model if (!$this->image && is_string($image) && ini_get('allow_url_fopen')) { try { $extension = explode('.', $image); - $fileName = uniqid().'.'.trim(strtolower(last($extension)), '. '); - $coverPath = app()->publicPath().'/public/img/artists/'.$fileName; - - file_put_contents($coverPath, file_get_contents($image)); - - $this->update(['image' => $fileName]); + $this->writeImageFile(file_get_contents($image), last($extension)); $info['image'] = $this->image; } catch (Exception $e) { Log::error($e); @@ -126,6 +121,35 @@ class Artist extends Model return $info; } + + /** + * 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/Song.php b/app/Models/Song.php index 64efd117..bacaf017 100644 --- a/app/Models/Song.php +++ b/app/Models/Song.php @@ -291,7 +291,7 @@ class Song extends Model * * @param string $youTubePageToken The YouTube page token, for pagination purpose. * - * @return @return object|false + * @return false|object */ public function getRelatedYouTubeVideos($youTubePageToken = '') { diff --git a/tests/BrowserKitTestCase.php b/tests/BrowserKitTestCase.php index c5b584a8..2d230e04 100644 --- a/tests/BrowserKitTestCase.php +++ b/tests/BrowserKitTestCase.php @@ -6,22 +6,12 @@ use App\Models\Album; use App\Models\Artist; use App\Models\Song; use App\Models\User; -use Illuminate\Contracts\Console\Kernel; -use Illuminate\Support\Facades\Artisan; use JWTAuth; use Laravel\BrowserKitTesting\TestCase as BaseBrowserKitTestCase; abstract class BrowserKitTestCase extends BaseBrowserKitTestCase { - protected $mediaPath; - protected $coverPath; - - public function __construct() - { - parent::__construct(); - - $this->mediaPath = __DIR__.'/songs'; - } + use CreatesApplication; public function setUp() { @@ -29,43 +19,7 @@ abstract class BrowserKitTestCase extends BaseBrowserKitTestCase $this->prepareForTests(); } - /** - * The base URL to use while testing the application. - * - * @var string - */ - protected $baseUrl = 'http://localhost'; - - /** - * Creates the application. - * - * @return \Illuminate\Foundation\Application - */ - public function createApplication() - { - $app = require __DIR__.'/../bootstrap/app.php'; - - $app->make(Kernel::class)->bootstrap(); - - $this->coverPath = $app->basePath().'/public/img/covers'; - - return $app; - } - - private function prepareForTests() - { - Artisan::call('migrate'); - - if (!User::all()->count()) { - Artisan::call('db:seed'); - } - - if (!file_exists($this->coverPath)) { - mkdir($this->coverPath, 0777, true); - } - } - - /** + /** * Create a sample media set, with a complete artist+album+song trio. */ protected function createSampleMediaSet() diff --git a/tests/CreatesApplication.php b/tests/CreatesApplication.php index aa4f6b3a..74c1d5b1 100644 --- a/tests/CreatesApplication.php +++ b/tests/CreatesApplication.php @@ -2,11 +2,14 @@ namespace Tests; +use App\Models\User; +use Artisan; use Illuminate\Contracts\Console\Kernel; trait CreatesApplication { protected $coverPath; + protected $mediaPath = __DIR__.'/songs'; /** * The base URL to use while testing the application. @@ -30,4 +33,17 @@ trait CreatesApplication return $app; } + + private function prepareForTests() + { + Artisan::call('migrate'); + + if (!User::all()->count()) { + Artisan::call('db:seed'); + } + + if (!file_exists($this->coverPath)) { + mkdir($this->coverPath, 0777, true); + } + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 2932d4a6..cdef857a 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -7,4 +7,10 @@ use Illuminate\Foundation\Testing\TestCase as BaseTestCase; abstract class TestCase extends BaseTestCase { use CreatesApplication; + + public function setUp() + { + parent::setUp(); + $this->prepareForTests(); + } } diff --git a/tests/Unit/AlbumTest.php b/tests/Unit/AlbumTest.php index df2d66b3..d6e75be8 100644 --- a/tests/Unit/AlbumTest.php +++ b/tests/Unit/AlbumTest.php @@ -2,16 +2,16 @@ namespace Tests\Unit; +use App\Facades\Lastfm; use App\Models\Album; use App\Models\Artist; -use Illuminate\Foundation\Testing\DatabaseMigrations; use Illuminate\Foundation\Testing\DatabaseTransactions; use org\bovigo\vfs\vfsStream; use Tests\TestCase; class AlbumTest extends TestCase { - use DatabaseTransactions, DatabaseMigrations; + use DatabaseTransactions; /** @test */ public function it_can_be_instantiated() @@ -115,4 +115,22 @@ class AlbumTest extends TestCase // And the album's cover attribute is updated $this->assertEquals('http://localhost/public/img/covers/bar.jpg', Album::find($album->id)->cover); } + + /** @test */ + public function extra_info_can_be_retrieved_for_an_album() + { + // Given there's an album + $album = factory(Album::class)->create(); + + // When I get the extra info for the album + Lastfm::shouldReceive('getAlbumInfo') + ->once() + ->with($album->name, $album->artist->name) + ->andReturn(['foo' => 'bar']); + + $info = $album->getInfo(); + + // Then I receive the extra info + $this->assertEquals(['foo' => 'bar'], $info); + } } diff --git a/tests/Unit/ArtistTest.php b/tests/Unit/ArtistTest.php new file mode 100644 index 00000000..2eae658f --- /dev/null +++ b/tests/Unit/ArtistTest.php @@ -0,0 +1,109 @@ +assertInstanceOf(Artist::class, new Artist()); + } + + /** @test */ + public function various_artist_can_be_retrieved() + { + // When I retrieve the Various Artist + $artist = Artist::getVariousArtist(); + + // Then I received the Various Artist + $this->assertTrue($artist->is_various); + } + + /** @test */ + public function existing_artist_can_be_retrieved_using_name() + { + // Given an existing artist with a name + $artist = factory(Artist::class)->create(['name' => 'Foo']); + + // When I get the artist by name + $gottenArtist = Artist::get('Foo'); + + // Then I get the artist + $this->assertEquals($artist->id, $gottenArtist->id); + } + + /** @test */ + public function new_artist_can_be_created_using_name() + { + // Given an artist name + $name = 'Foo'; + + // And an artist with such a name doesn't exist yet + $this->assertNull(Artist::whereName($name)->first()); + + // When I get the artist by name + $artist = Artist::get($name); + + // Then I get the newly created artist + $this->assertInstanceOf(Artist::class, $artist); + } + + /** @test */ + public function getting_artist_with_empty_name_returns_unknown_artist() + { + // Given an empty name + $name = ''; + + // When I get the artist by the empty name + $artist = Artist::get($name); + + // Then I get the artist as Unknown Artist + $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 + $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 extra_info_can_be_retrieved_for_an_artist() + { + // Given there's an artist + $artist = factory(Artist::class)->create(); + + // When I get the extra info + Lastfm::shouldReceive('getArtistInfo') + ->once() + ->with($artist->name) + ->andReturn(['foo' => 'bar']); + + $info = $artist->getInfo(); + + // Then I receive the extra info + $this->assertEquals(['foo' => 'bar'], $info); + } +}