Refactor S3

app/Models/Song.php

@@ -18,7 +18,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsToMany;
  * @property string title
  * @property Album  album
  * @property Artist artist
- * @property string s3_params
+ * @property string[] s3_params
  * @property float  length
  * @property string lyrics
  * @property int    track
@@ -195,30 +195,6 @@ class Song extends Model
         return $query->where('path', 'LIKE', "$path%");
     }
 
-    /**
-     * Get the song's Object Storage url for streaming or downloading.
-     */
-    public function getObjectStoragePublicUrl(AwsClient $s3 = null): string
-    {
-        return Cache::remember("OSUrl/{$this->id}", 60, static function () use ($s3) {
-            if (!$s3) {
-                $s3 = AWS::createClient('s3');
-            }
-
-            $cmd = $s3->getCommand('GetObject', [
-                'Bucket' => $this->s3_params['bucket'],
-                'Key' => $this->s3_params['key'],
-            ]);
-
-            // Here we specify that the request is valid for 1 hour.
-            // We'll also cache the public URL for future reuse.
-            $request = $s3->createPresignedRequest($cmd, '+1 hour');
-            $url = (string) $request->getUri();
-
-            return $url;
-        });
-    }
-
     /**
      * Sometimes the tags extracted from getID3 are HTML entity encoded.
      * This makes sure they are always sane.

app/Providers/AppServiceProvider.php

@@ -38,7 +38,7 @@ class AppServiceProvider extends ServiceProvider
      *
      * @return void
      */
-    public function register()
+    public function register(): void
     {
         if (!$this->app->environment('production')) {
             $this->app->register(TinkerServiceProvider::class);

app/Providers/DownloadServiceProvider.php

@@ -7,25 +7,10 @@ use Illuminate\Support\ServiceProvider;
 
 class DownloadServiceProvider extends ServiceProvider
 {
-    /**
-     * Bootstrap the application services.
-     *
-     * @return void
-     */
-    public function boot()
+    public function register(): void
     {
-        //
-    }
-
-    /**
-     * Register the application services.
-     *
-     * @return void
-     */
-    public function register()
-    {
-        app()->singleton('Download', function () {
-            return new DownloadService();
+        app()->singleton('Download', static function (): DownloadService {
+            return app(DownloadService::class);
         });
     }
 }

app/Providers/MediaCacheServiceProvider.php

@@ -7,25 +7,10 @@ use Illuminate\Support\ServiceProvider;
 
 class MediaCacheServiceProvider extends ServiceProvider
 {
-    /**
-     * Bootstrap the application services.
-     *
-     * @return void
-     */
-    public function boot()
+    public function register(): void
     {
-        //
-    }
-
-    /**
-     * Register the application services.
-     *
-     * @return void
-     */
-    public function register()
-    {
-        app()->singleton('MediaCache', function () {
-            return new MediaCacheService();
+        app()->singleton('MediaCache', static function (): MediaCacheService {
+            return app(MediaCacheService::class);
         });
     }
 }

app/Providers/ObjectStorageServiceProvider.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace App\Providers;
+
+use AWS;
+use Aws\AwsClientInterface;
+use Aws\S3\S3ClientInterface;
+use Illuminate\Support\ServiceProvider;
+
+class ObjectStorageServiceProvider extends ServiceProvider
+{
+    public function register(): void
+    {
+        $this->app->bind(S3ClientInterface::class, static function (): AwsClientInterface {
+            return AWS::createClient('s3');
+        });
+    }
+}

app/Providers/StreamerServiceProvider.php

@@ -14,9 +14,9 @@ use Illuminate\Support\ServiceProvider;
 
 class StreamerServiceProvider extends ServiceProvider
 {
-    public function register()
+    public function register(): void
     {
-        $this->app->bind(DirectStreamerInterface::class, static function () {
+        $this->app->bind(DirectStreamerInterface::class, static function (): DirectStreamerInterface {
             switch (config('koel.streaming.method')) {
                 case 'x-sendfile':
                     return new XSendFileStreamer();

app/Providers/UtilServiceProvider.php

@@ -7,25 +7,10 @@ use Illuminate\Support\ServiceProvider;
 
 class UtilServiceProvider extends ServiceProvider
 {
-    /**
-     * Bootstrap the application services.
-     *
-     * @return void
-     */
-    public function boot()
+    public function register(): void
     {
-        //
-    }
-
-    /**
-     * Register the application services.
-     *
-     * @return void
-     */
-    public function register()
-    {
-        app()->singleton('Util', function () {
-            return new Util();
+        app()->singleton('Util', static function (): Util {
+            return app(Util::class);
         });
     }
 }

app/Providers/YouTubeServiceProvider.php

@@ -7,25 +7,10 @@ use Illuminate\Support\ServiceProvider;
 
 class YouTubeServiceProvider extends ServiceProvider
 {
-    /**
-     * Bootstrap the application services.
-     *
-     * @return void
-     */
-    public function boot()
+    public function register(): void
     {
-        //
-    }
-
-    /**
-     * Register the application services.
-     *
-     * @return void
-     */
-    public function register()
-    {
-        app()->singleton('YouTube', function () {
-            return app()->make(YouTubeService::class);
+        app()->singleton('YouTube', static function (): YouTubeService {
+            return app(YouTubeService::class);
         });
     }
 }

app/Providers/iTunesServiceProvider.php

@@ -7,15 +7,10 @@ use Illuminate\Support\ServiceProvider;
 
 class iTunesServiceProvider extends ServiceProvider
 {
-    /**
-     * Register the application services.
-     *
-     * @return void
-     */
-    public function register()
+    public function register(): void
     {
-        app()->singleton('iTunes', function () {
-            return app()->make(iTunesService::class);
+        app()->singleton('iTunes', static function (): iTunesService {
+            return app(iTunesService::class);
         });
     }
 }

app/Services/DownloadService.php

@@ -12,6 +12,13 @@ use InvalidArgumentException;
 
 class DownloadService
 {
+    private $s3Service;
+
+    public function __construct(S3Service $s3Service)
+    {
+        $this->s3Service = $s3Service;
+    }
+
     /**
      * Generic method to generate a download archive from various source types.
      *
@@ -44,8 +51,7 @@ class DownloadService
         if ($s3Params = $song->s3_params) {
             // The song is hosted on Amazon S3.
             // We download it back to our local server first.
-            $url = $song->getObjectStoragePublicUrl();
-
+            $url = $this->s3Service->getSongPublicUrl($song);
             abort_unless($url, 404);
 
             $localPath = sys_get_temp_dir().DIRECTORY_SEPARATOR.basename($s3Params['key']);

app/Services/ObjectStorageInterface.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace App\Services;
+
+use App\Models\Song;
+
+interface ObjectStorageInterface
+{
+    /**
+     * Get a song's Object Storage url for streaming or downloading.
+     */
+    public function getSongPublicUrl(Song $song): string;
+}

app/Services/S3Service.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace App\Services;
+
+use App\Models\Song;
+use Aws\S3\S3ClientInterface;
+use Illuminate\Cache\Repository as Cache;
+
+class S3Service implements ObjectStorageInterface
+{
+    private $s3Client;
+    private $cache;
+
+    public function __construct(S3ClientInterface $s3Client, Cache $cache)
+    {
+        $this->s3Client = $s3Client;
+        $this->cache = $cache;
+    }
+
+    public function getSongPublicUrl(Song $song): string
+    {
+        return $this->cache->remember("OSUrl/{$song->id}", 60, static function () use ($song): string {
+            $cmd = $this->s3Client->getCommand('GetObject', [
+                'Bucket' => $song->s3_params['bucket'],
+                'Key' => $song->s3_params['key'],
+            ]);
+
+            // Here we specify that the request is valid for 1 hour.
+            // We'll also cache the public URL for future reuse.
+            $request = $this->s3Client->createPresignedRequest($cmd, '+1 hour');
+            $url = (string) $request->getUri();
+
+            return $url;
+        });
+    }
+}

app/Services/Streamers/S3Streamer.php

@@ -2,15 +2,25 @@
 
 namespace App\Services\Streamers;
 
+use App\Services\S3Service;
+
 class S3Streamer extends Streamer implements ObjectStorageStreamerInterface
 {
+    private $s3Service;
+
+    public function __construct(S3Service $s3Service)
+    {
+        parent::__construct();
+        $this->s3Service = $s3Service;
+    }
+
     /**
      * Stream the current song through S3.
-     * Actually, we only redirect to the S3 object's location.
+     * Actually, we just redirect the request to the S3 object's location.
      */
     public function stream()
     {
         // Get and redirect to the actual presigned-url
-        return redirect($this->song->getObjectStoragePublicUrl());
+        return redirect($this->s3Service->getSongPublicUrl($this->song));
     }
 }

config/app.php

@@ -156,6 +156,7 @@ return [
         App\Providers\BroadcastServiceProvider::class,
         App\Providers\iTunesServiceProvider::class,
         App\Providers\StreamerServiceProvider::class,
+        App\Providers\ObjectStorageServiceProvider::class,
     ],
 
     /*

tests/Integration/Models/SongTest.php

@@ -10,35 +10,6 @@ use Tests\TestCase;
 
 class SongTest extends TestCase
 {
-    protected function tearDown()
-    {
-        parent::tearDown();
-        m::close();
-    }
-
-    /** @test */
-    public function it_returns_object_storage_public_url()
-    {
-        // Given there's a song hosted on Amazon S3
-        /** @var Song $song */
-        $song = factory(Song::class)->create(['path' => 's3://foo/bar']);
-        $mockedURL = 'http://aws.com/foo/bar';
-
-        // When I get the song's object storage public URL
-        $client = m::mock(AwsClient::class, [
-            'getCommand' => null,
-            'createPresignedRequest' => m::mock(Request::class, [
-                'getUri' => $mockedURL,
-            ]),
-        ]);
-
-        Cache::shouldReceive('remember')->andReturn($mockedURL);
-        $url = $song->getObjectStoragePublicUrl($client);
-
-        // Then I should receive the correct S3 public URL
-        $this->assertEquals($mockedURL, $url);
-    }
-
     /** @test */
     public function it_can_be_retrieved_using_its_path()
     {

tests/Integration/Services/S3ServiceTest.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace Tests\Integration\Services;
+
+use App\Models\Song;
+use App\Services\S3Service;
+use Aws\CommandInterface;
+use Aws\S3\S3ClientInterface;
+use GuzzleHttp\Psr7\Request;
+use Illuminate\Cache\Repository as Cache;
+use Mockery;
+use Mockery\MockInterface;
+use Tests\TestCase;
+
+class S3ServiceTest extends TestCase
+{
+    /**
+     * @var S3ClientInterface|MockInterface
+     */
+    private $s3Client;
+
+    /**
+     * @var Cache|MockInterface
+     */
+    private $cache;
+
+    /**
+     * @var S3Service
+     */
+    private $s3Service;
+
+    public function setUp()
+    {
+        parent::setUp();
+        $this->s3Client = Mockery::mock(S3ClientInterface::class);
+        $this->cache = Mockery::mock(Cache::class);
+        $this->s3Service = new S3Service($this->s3Client, $this->cache);
+    }
+
+    public function testGetSongPublicUrl(): void
+    {
+        $song = factory(Song::class)->create(['path' => 's3://foo/bar']);
+
+        $cmd = Mockery::mock(CommandInterface::class);
+        $this->s3Client->shouldReceive('getCommand')
+            ->with('GetObject', [
+                'Bucket' => 'foo',
+                'Key' => 'bar',
+            ])
+            ->andReturn($cmd);
+
+        $request = Mockery::mock(Request::class, ['getUri' => 'https://aws.com/foo.mp3']);
+
+        $this->s3Client->shouldReceive('createPresignedRequest')
+            ->with($cmd, '+1 hour')
+            ->andReturn($request);
+
+        $this->cache->shouldReceive('remember')
+            ->once()
+            ->andReturn('https://aws.com/foo.mp3');
+
+        self::assertSame('https://aws.com/foo.mp3', $this->s3Service->getSongPublicUrl($song));
+    }
+}