From a501613052a8c6f8ee7d76d8bcddcd6e5b7c70e3 Mon Sep 17 00:00:00 2001
From: Phan An <me@phanan.net>
Date: Fri, 22 Mar 2024 16:22:29 +0100
Subject: [PATCH] feat: use Saloon for iTunes integration

---
 .../ViewSongOnITunesController.php            |  2 +-
 .../Integrations/iTunes/ITunesConnector.php   | 16 +++
 .../iTunes/Requests/GetTrackRequest.php       | 46 +++++++++
 app/Services/ApiClients/LastfmClient.php      | 95 ------------------
 .../ApiClients/LemonSqueezyApiClient.php      | 34 -------
 app/Services/ApiClients/YouTubeClient.php     | 21 ----
 app/Services/ITunesService.php                | 38 +++-----
 .../Services/ApiClients/ApiClientTest.php     | 59 -----------
 .../Services/ApiClients/LastfmClientTest.php  | 25 -----
 .../Services/ApiClients/SpotifyClientTest.php | 97 -------------------
 10 files changed, 76 insertions(+), 357 deletions(-)
 create mode 100644 app/Http/Integrations/iTunes/ITunesConnector.php
 create mode 100644 app/Http/Integrations/iTunes/Requests/GetTrackRequest.php
 delete mode 100644 app/Services/ApiClients/LastfmClient.php
 delete mode 100644 app/Services/ApiClients/LemonSqueezyApiClient.php
 delete mode 100644 app/Services/ApiClients/YouTubeClient.php
 delete mode 100644 tests/Unit/Services/ApiClients/ApiClientTest.php
 delete mode 100644 tests/Unit/Services/ApiClients/LastfmClientTest.php
 delete mode 100644 tests/Unit/Services/ApiClients/SpotifyClientTest.php

diff --git a/app/Http/Controllers/ViewSongOnITunesController.php b/app/Http/Controllers/ViewSongOnITunesController.php
index 0366324d..15d5eb30 100644
--- a/app/Http/Controllers/ViewSongOnITunesController.php
+++ b/app/Http/Controllers/ViewSongOnITunesController.php
@@ -21,7 +21,7 @@ class ViewSongOnITunesController extends Controller
             Response::HTTP_UNAUTHORIZED
         );
 
-        $url = $iTunesService->getTrackUrl($request->q, $album->name, $album->artist->name);
+        $url = $iTunesService->getTrackUrl($request->q, $album);
         abort_unless((bool) $url, Response::HTTP_NOT_FOUND, "Koel can't find such a song on iTunes Store.");
 
         return redirect($url);
diff --git a/app/Http/Integrations/iTunes/ITunesConnector.php b/app/Http/Integrations/iTunes/ITunesConnector.php
new file mode 100644
index 00000000..5524ede5
--- /dev/null
+++ b/app/Http/Integrations/iTunes/ITunesConnector.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Http\Integrations\iTunes;
+
+use Saloon\Http\Connector;
+use Saloon\Traits\Plugins\AcceptsJson;
+
+class ITunesConnector extends Connector
+{
+    use AcceptsJson;
+
+    public function resolveBaseUrl(): string
+    {
+        return config('koel.itunes.endpoint');
+    }
+}
diff --git a/app/Http/Integrations/iTunes/Requests/GetTrackRequest.php b/app/Http/Integrations/iTunes/Requests/GetTrackRequest.php
new file mode 100644
index 00000000..14690be5
--- /dev/null
+++ b/app/Http/Integrations/iTunes/Requests/GetTrackRequest.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace App\Http\Integrations\iTunes\Requests;
+
+use App\Models\Album;
+use App\Models\Artist;
+use Saloon\Enums\Method;
+use Saloon\Http\Request;
+
+class GetTrackRequest extends Request
+{
+    protected Method $method = Method::GET;
+
+    public function __construct(private string $trackName, private Album $album)
+    {
+    }
+
+    /** @return array<mixed> */
+    protected function defaultQuery(): array
+    {
+        $term = $this->trackName;
+
+        if ($this->album->name !== Album::UNKNOWN_NAME) {
+            $term .= ' ' . $this->album->name;
+        }
+
+        if (
+            $this->album->artist->name !== Artist::UNKNOWN_NAME
+            && $this->album->artist->name !== Artist::VARIOUS_NAME
+        ) {
+            $term .= ' ' . $this->album->artist->name;
+        }
+
+        return [
+            'term' => $term,
+            'media' => 'music',
+            'entity' => 'song',
+            'limit' => 1,
+        ];
+    }
+
+    public function resolveEndpoint(): string
+    {
+        return '/';
+    }
+}
diff --git a/app/Services/ApiClients/LastfmClient.php b/app/Services/ApiClients/LastfmClient.php
deleted file mode 100644
index 4892c5b0..00000000
--- a/app/Services/ApiClients/LastfmClient.php
+++ /dev/null
@@ -1,95 +0,0 @@
-<?php
-
-namespace App\Services\ApiClients;
-
-use GuzzleHttp\Promise\Promise;
-
-class LastfmClient extends ApiClient
-{
-    protected string $keyParam = 'api_key';
-
-    public function post($uri, array $data = [], bool $appendKey = true): mixed
-    {
-        return parent::post($uri, $this->buildAuthCallParams($data), $appendKey);
-    }
-
-    public function postAsync($uri, array $data = [], bool $appendKey = true): Promise
-    {
-        return parent::postAsync($uri, $this->buildAuthCallParams($data), $appendKey);
-    }
-
-    /**
-     * Get Last.fm's session key for the authenticated user using a token.
-     *
-     * @param string $token The token after successfully connecting to Last.fm
-     *
-     * @see http://www.last.fm/api/webauth#4
-     */
-    public function getSessionKey(string $token): ?string
-    {
-        $query = $this->buildAuthCallParams([
-            'method' => 'auth.getSession',
-            'token' => $token,
-        ], true);
-
-
-        return attempt(fn () => $this->get("/?$query&format=json", [], false)->session->key);
-    }
-
-    /**
-     * Build the parameters to use for _authenticated_ Last.fm API calls.
-     * Such calls require:
-     * - The API key (api_key)
-     * - The API signature (api_sig).
-     *
-     * @see http://www.last.fm/api/webauth#5
-     *
-     * @param array $params The array of parameters
-     * @param bool $toString Whether to turn the array into a query string
-     *
-     * @return array<mixed>|string
-     */
-    private function buildAuthCallParams(array $params, bool $toString = false): array|string
-    {
-        $params['api_key'] = $this->getKey();
-        ksort($params);
-
-        // Generate the API signature.
-        // @link http://www.last.fm/api/webauth#6
-        $str = '';
-
-        foreach ($params as $name => $value) {
-            $str .= $name . $value;
-        }
-
-        $str .= $this->getSecret();
-        $params['api_sig'] = md5($str);
-
-        if (!$toString) {
-            return $params;
-        }
-
-        $query = '';
-
-        foreach ($params as $key => $value) {
-            $query .= "$key=$value&";
-        }
-
-        return rtrim($query, '&');
-    }
-
-    public function getKey(): ?string
-    {
-        return config('koel.lastfm.key');
-    }
-
-    public function getEndpoint(): ?string
-    {
-        return config('koel.lastfm.endpoint');
-    }
-
-    public function getSecret(): ?string
-    {
-        return config('koel.lastfm.secret');
-    }
-}
diff --git a/app/Services/ApiClients/LemonSqueezyApiClient.php b/app/Services/ApiClients/LemonSqueezyApiClient.php
deleted file mode 100644
index 12ffd1e5..00000000
--- a/app/Services/ApiClients/LemonSqueezyApiClient.php
+++ /dev/null
@@ -1,34 +0,0 @@
-<?php
-
-namespace App\Services\ApiClients;
-
-class LemonSqueezyApiClient extends ApiClient
-{
-    protected array $headers = [
-        'Accept' => 'application/json',
-    ];
-
-    public function post($uri, array $data = [], bool $appendKey = true, array $headers = []): mixed
-    {
-        // LemonSquzzey requires the Content-Type header to be set to application/x-www-form-urlencoded
-        // @see https://docs.lemonsqueezy.com/help/licensing/license-api#requests
-        $headers['Content-Type'] = 'application/x-www-form-urlencoded';
-
-        return parent::post($uri, $data, $appendKey, $headers);
-    }
-
-    public function getKey(): ?string
-    {
-        return null;
-    }
-
-    public function getSecret(): ?string
-    {
-        return null;
-    }
-
-    public function getEndpoint(): ?string
-    {
-        return 'https://api.lemonsqueezy.com/v1/';
-    }
-}
diff --git a/app/Services/ApiClients/YouTubeClient.php b/app/Services/ApiClients/YouTubeClient.php
deleted file mode 100644
index df950a0a..00000000
--- a/app/Services/ApiClients/YouTubeClient.php
+++ /dev/null
@@ -1,21 +0,0 @@
-<?php
-
-namespace App\Services\ApiClients;
-
-class YouTubeClient extends ApiClient
-{
-    public function getKey(): ?string
-    {
-        return config('koel.youtube.key');
-    }
-
-    public function getSecret(): ?string
-    {
-        return null;
-    }
-
-    public function getEndpoint(): ?string
-    {
-        return config('koel.youtube.endpoint');
-    }
-}
diff --git a/app/Services/ITunesService.php b/app/Services/ITunesService.php
index 5541fa8e..da039b5e 100644
--- a/app/Services/ITunesService.php
+++ b/app/Services/ITunesService.php
@@ -2,45 +2,33 @@
 
 namespace App\Services;
 
-use App\Services\ApiClients\ITunesClient;
+use App\Http\Integrations\iTunes\ITunesConnector;
+use App\Http\Integrations\iTunes\Requests\GetTrackRequest;
+use App\Models\Album;
 use Illuminate\Cache\Repository as Cache;
 
 class ITunesService
 {
-    public function __construct(private ITunesClient $client, private Cache $cache)
+    public function __construct(private ITunesConnector $connector, private Cache $cache)
     {
     }
 
-    /**
-     * Determines whether to use iTunes services.
-     */
     public static function used(): bool
     {
         return (bool) config('koel.itunes.enabled');
     }
 
-    /**
-     * Search for a track on iTunes Store with the given information and get its URL.
-     *
-     * @param string $term The main query string (should be the track's name)
-     * @param string $album The album's name, if available
-     * @param string $artist The artist's name, if available
-     */
-    public function getTrackUrl(string $term, string $album = '', string $artist = ''): ?string
+    public function getTrackUrl(string $trackName, Album $album): ?string
     {
-        return attempt(function () use ($term, $album, $artist): ?string {
-            return $this->cache->remember(
-                md5("itunes_track_url_$term$album$artist"),
-                24 * 60 * 7,
-                function () use ($term, $album, $artist): ?string {
-                    $params = [
-                        'term' => $term . ($album ? " $album" : '') . ($artist ? " $artist" : ''),
-                        'media' => 'music',
-                        'entity' => 'song',
-                        'limit' => 1,
-                    ];
+        return attempt(function () use ($trackName, $album): ?string {
+            $request = new GetTrackRequest($trackName, $album);
+            $hash = md5(serialize($request->query()));
 
-                    $response = $this->client->get('/', ['query' => $params]);
+            return $this->cache->remember(
+                "itunes:track:$hash",
+                now()->addWeek(),
+                function () use ($request): ?string {
+                    $response = $this->connector->send($request)->object();
 
                     if (!$response->resultCount) {
                         return null;
diff --git a/tests/Unit/Services/ApiClients/ApiClientTest.php b/tests/Unit/Services/ApiClients/ApiClientTest.php
deleted file mode 100644
index 76d56aac..00000000
--- a/tests/Unit/Services/ApiClients/ApiClientTest.php
+++ /dev/null
@@ -1,59 +0,0 @@
-<?php
-
-namespace Tests\Unit\Services\ApiClients;
-
-use GuzzleHttp\Client;
-use GuzzleHttp\Psr7\Response;
-use Illuminate\Foundation\Testing\WithoutMiddleware;
-use Mockery;
-use Mockery\LegacyMockInterface;
-use Mockery\MockInterface;
-use Tests\TestCase;
-use Tests\Unit\Stubs\ConcreteApiClient;
-
-class ApiClientTest extends TestCase
-{
-    use WithoutMiddleware;
-
-    private Client|LegacyMockInterface|MockInterface $wrapped;
-
-    public function setUp(): void
-    {
-        parent::setUp();
-
-        $this->wrapped = Mockery::mock(Client::class);
-    }
-
-    public function testBuildUri(): void
-    {
-        $api = new ConcreteApiClient($this->wrapped);
-
-        self::assertSame('https://foo.com/get/param?key=bar', $api->buildUrl('get/param'));
-        self::assertSame('https://foo.com/get/param?baz=moo&key=bar', $api->buildUrl('/get/param?baz=moo'));
-        self::assertSame('https://baz.com/?key=bar', $api->buildUrl('https://baz.com/'));
-    }
-
-    /** @return array<mixed> */
-    public function provideRequestData(): array
-    {
-        return [
-            ['get', '{"foo":"bar"}'],
-            ['post', '{"foo":"bar"}'],
-            ['put', '{"foo":"bar"}'],
-            ['delete', '{"foo":"bar"}'],
-        ];
-    }
-
-    /** @dataProvider provideRequestData */
-    public function testRequest(string $method, string $responseBody): void
-    {
-        /** @var Client $client */
-        $client = Mockery::mock(Client::class, [
-            $method => new Response(200, [], $responseBody),
-        ]);
-
-        $api = new ConcreteApiClient($client);
-
-        self::assertSame((array) json_decode($responseBody), (array) $api->$method('/'));
-    }
-}
diff --git a/tests/Unit/Services/ApiClients/LastfmClientTest.php b/tests/Unit/Services/ApiClients/LastfmClientTest.php
deleted file mode 100644
index 1311ba72..00000000
--- a/tests/Unit/Services/ApiClients/LastfmClientTest.php
+++ /dev/null
@@ -1,25 +0,0 @@
-<?php
-
-namespace Tests\Unit\Services\ApiClients;
-
-use App\Services\ApiClients\LastfmClient;
-use GuzzleHttp\Client as GuzzleHttpClient;
-use GuzzleHttp\Handler\MockHandler;
-use GuzzleHttp\HandlerStack;
-use GuzzleHttp\Psr7\Response;
-use Illuminate\Support\Facades\File;
-use Tests\TestCase;
-
-use function Tests\test_path;
-
-class LastfmClientTest extends TestCase
-{
-    public function testGetSessionKey(): void
-    {
-        $mock = new MockHandler([new Response(200, [], File::get(test_path('blobs/lastfm/session-key.json')))]);
-
-        $client = new LastfmClient(new GuzzleHttpClient(['handler' => HandlerStack::create($mock)]));
-
-        self::assertSame('foo', $client->getSessionKey('bar'));
-    }
-}
diff --git a/tests/Unit/Services/ApiClients/SpotifyClientTest.php b/tests/Unit/Services/ApiClients/SpotifyClientTest.php
deleted file mode 100644
index 4c72ced3..00000000
--- a/tests/Unit/Services/ApiClients/SpotifyClientTest.php
+++ /dev/null
@@ -1,97 +0,0 @@
-<?php
-
-namespace Tests\Unit\Services\ApiClients;
-
-use App\Exceptions\SpotifyIntegrationDisabledException;
-use App\Services\ApiClients\SpotifyClient;
-use Illuminate\Cache\Repository as Cache;
-use Mockery;
-use Mockery\LegacyMockInterface;
-use Mockery\MockInterface;
-use SpotifyWebAPI\Session as SpotifySession;
-use SpotifyWebAPI\SpotifyWebAPI;
-use Tests\TestCase;
-
-class SpotifyClientTest extends TestCase
-{
-    private SpotifySession|LegacyMockInterface|MockInterface $session;
-    private SpotifyWebAPI|LegacyMockInterface|MockInterface $wrapped;
-    private Cache|LegacyMockInterface|MockInterface $cache;
-
-    private SpotifyClient $client;
-
-    public function setUp(): void
-    {
-        parent::setUp();
-
-        config([
-            'koel.spotify.client_id' => 'fake-client-id',
-            'koel.spotify.client_secret' => 'fake-client-secret',
-        ]);
-
-        $this->session = Mockery::mock(SpotifySession::class);
-        $this->wrapped = Mockery::mock(SpotifyWebAPI::class);
-        $this->cache = Mockery::mock(Cache::class);
-    }
-
-    public function testAccessTokenIsSetUponInitialization(): void
-    {
-        $this->mockSetAccessToken();
-
-        $this->client = new SpotifyClient($this->wrapped, $this->session, $this->cache);
-        self::addToAssertionCount(1);
-    }
-
-    public function testAccessTokenIsRetrievedFromCacheWhenApplicable(): void
-    {
-        $this->wrapped->shouldReceive('setOptions')->with(['return_assoc' => true]);
-        $this->cache->shouldReceive('get')->with('spotify.access_token')->andReturn('fake-access-token');
-        $this->session->shouldNotReceive('requestCredentialsToken');
-        $this->session->shouldNotReceive('getAccessToken');
-        $this->cache->shouldNotReceive('put');
-        $this->wrapped->shouldReceive('setAccessToken')->with('fake-access-token');
-
-        $this->client = new SpotifyClient($this->wrapped, $this->session, $this->cache);
-    }
-
-    public function testCallForwarding(): void
-    {
-        $this->mockSetAccessToken();
-        $this->wrapped->shouldReceive('search')->with('foo', 'track')->andReturn('bar');
-
-        $this->client = new SpotifyClient($this->wrapped, $this->session, $this->cache);
-
-        self::assertSame('bar', $this->client->search('foo', 'track'));
-    }
-
-    public function testCallForwardingThrowsIfIntegrationIsDisabled(): void
-    {
-        config([
-            'koel.spotify.client_id' => null,
-            'koel.spotify.client_secret' => null,
-        ]);
-
-        self::expectException(SpotifyIntegrationDisabledException::class);
-        (new SpotifyClient($this->wrapped, $this->session, $this->cache))->search('foo', 'track');
-    }
-
-    private function mockSetAccessToken(): void
-    {
-        $this->wrapped->shouldReceive('setOptions')->with(['return_assoc' => true]);
-        $this->cache->shouldReceive('get')->with('spotify.access_token')->andReturnNull();
-        $this->session->shouldReceive('requestCredentialsToken');
-        $this->session->shouldReceive('getAccessToken')->andReturn('fake-access-token');
-        $this->cache->shouldReceive('put')->with('spotify.access_token', 'fake-access-token', 3_540);
-        $this->wrapped->shouldReceive('setAccessToken')->with('fake-access-token');
-    }
-
-    protected function tearDown(): void
-    {
-        config([
-            'koel.spotify.client_id' => null,
-            'koel.spotify.client_secret' => null,
-        ]);
-
-        parent::tearDown();
-    }
-}