From 3a3a84164d0babd505bfbd23e71dc652c4e1245d Mon Sep 17 00:00:00 2001
From: Phan An <me@phanan.net>
Date: Mon, 15 Jan 2024 14:32:44 +0100
Subject: [PATCH] fix: updating songs from S3 might create duplicates

---
 app/Services/S3Service.php            |  5 +-
 tests/Unit/Services/S3ServiceTest.php | 86 ++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/app/Services/S3Service.php b/app/Services/S3Service.php
index a4f84156..82753671 100644
--- a/app/Services/S3Service.php
+++ b/app/Services/S3Service.php
@@ -49,7 +49,6 @@ class S3Service implements ObjectStorageInterface
         string $lyrics
     ): Song {
         $path = Song::getPathFromS3BucketAndKey($bucket, $key);
-
         $artist = Artist::getOrCreate($artistName);
 
         $albumArtist = $albumArtistName && $albumArtistName !== $artistName
@@ -66,8 +65,8 @@ class S3Service implements ObjectStorageInterface
             );
         }
 
-        $song = Song::query()->updateOrCreate(['id' => Helper::getFileHash($path)], [
-            'path' => $path,
+        /** @var Song $song */
+        $song = Song::query()->updateOrCreate(['path' => $path], [
             'album_id' => $album->id,
             'artist_id' => $artist->id,
             'title' => $title,
diff --git a/tests/Unit/Services/S3ServiceTest.php b/tests/Unit/Services/S3ServiceTest.php
index f2bfd957..7a1dc25c 100644
--- a/tests/Unit/Services/S3ServiceTest.php
+++ b/tests/Unit/Services/S3ServiceTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Unit\Services;
 
+use App\Events\LibraryChanged;
 use App\Models\Song;
 use App\Repositories\SongRepository;
 use App\Services\MediaMetadataService;
@@ -19,6 +20,7 @@ class S3ServiceTest extends TestCase
 {
     private S3ClientInterface|LegacyMockInterface|MockInterface $s3Client;
     private Cache|LegacyMockInterface|MockInterface $cache;
+    private SongRepository|LegacyMockInterface|MockInterface $songRepository;
     private S3Service $s3Service;
 
     public function setUp(): void
@@ -29,9 +31,89 @@ class S3ServiceTest extends TestCase
         $this->cache = Mockery::mock(Cache::class);
 
         $metadataService = Mockery::mock(MediaMetadataService::class);
-        $songRepository = Mockery::mock(SongRepository::class);
+        $this->songRepository = Mockery::mock(SongRepository::class);
 
-        $this->s3Service = new S3Service($this->s3Client, $this->cache, $metadataService, $songRepository);
+        $this->s3Service = new S3Service($this->s3Client, $this->cache, $metadataService, $this->songRepository);
+    }
+
+    public function testCreateSongEntry(): void
+    {
+        $this->expectsEvents(LibraryChanged::class);
+
+        $song = $this->s3Service->createSongEntry(
+            bucket: 'foo',
+            key: 'bar',
+            artistName: 'Queen',
+            albumName: 'A Night at the Opera',
+            albumArtistName: 'Queen',
+            cover: [],
+            title: 'Bohemian Rhapsody',
+            duration: 355.5,
+            track: 1,
+            lyrics: 'Is this the real life?'
+        );
+
+        self::assertSame('Queen', $song->artist->name);
+        self::assertSame('A Night at the Opera', $song->album->name);
+        self::assertSame('Queen', $song->album_artist->name);
+        self::assertSame('Bohemian Rhapsody', $song->title);
+        self::assertSame(355.5, $song->length);
+        self::assertSame('Is this the real life?', $song->lyrics);
+        self::assertSame(1, $song->track);
+    }
+
+    public function testUpdateSongEntry(): void
+    {
+        $this->expectsEvents(LibraryChanged::class);
+
+        /** @var Song $song */
+        $song = Song::factory()->create([
+            'path' => 's3://foo/bar',
+        ]);
+
+        $this->s3Service->createSongEntry(
+            bucket: 'foo',
+            key: 'bar',
+            artistName: 'Queen',
+            albumName: 'A Night at the Opera',
+            albumArtistName: 'Queen',
+            cover: [],
+            title: 'Bohemian Rhapsody',
+            duration: 355.5,
+            track: 1,
+            lyrics: 'Is this the real life?'
+        );
+
+        self::assertSame(1, Song::query()->count());
+
+        $song->refresh();
+
+        self::assertSame('Queen', $song->artist->name);
+        self::assertSame('A Night at the Opera', $song->album->name);
+        self::assertSame('Queen', $song->album_artist->name);
+        self::assertSame('Bohemian Rhapsody', $song->title);
+        self::assertSame(355.5, $song->length);
+        self::assertSame('Is this the real life?', $song->lyrics);
+        self::assertSame(1, $song->track);
+    }
+
+    public function testDeleteSong(): void
+    {
+        $this->expectsEvents(LibraryChanged::class);
+
+        /** @var Song $song */
+        $song = Song::factory()->create([
+            'path' => 's3://foo/bar',
+        ]);
+
+        $this->songRepository->shouldReceive('getOneByPath')
+            ->with('s3://foo/bar')
+            ->once()
+            ->andReturn($song);
+
+        $this->s3Service->deleteSongEntry('foo', 'bar');
+
+        self::assertModelMissing($song);
     }
 
     public function testGetSongPublicUrl(): void