fix: do not remove S3-hosted songs post-sync (#1390)

app/Events/MediaSyncCompleted.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace App\Events;
+
+use App\Values\SyncResult;
+use Illuminate\Queue\SerializesModels;
+
+class MediaSyncCompleted extends Event
+{
+    use SerializesModels;
+
+    public SyncResult $result;
+
+    public function __construct(SyncResult $result)
+    {
+        $this->result = $result;
+    }
+}

app/Events/SongsBatchUnliked.php

@@ -6,7 +6,7 @@ use App\Models\User;
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Support\Collection;
 
-class SongsBatchUnliked
+class SongsBatchUnliked extends Event
 {
     use SerializesModels;
 

app/Exceptions/SongPathNotFoundException.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+use Throwable;
+
+class SongPathNotFoundException extends Exception
+{
+    private function __construct($message = '', $code = 0, ?Throwable $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+    }
+
+    public static function create(string $path): self
+    {
+        return new static(sprintf('The song at path %s cannot be found.', $path));
+    }
+}

app/Http/Controllers/API/ObjectStorage/S3/SongController.php

@@ -2,76 +2,47 @@
 
 namespace App\Http\Controllers\API\ObjectStorage\S3;
 
-use App\Events\LibraryChanged;
+use App\Exceptions\SongPathNotFoundException;
 use App\Http\Requests\API\ObjectStorage\S3\PutSongRequest;
 use App\Http\Requests\API\ObjectStorage\S3\RemoveSongRequest;
-use App\Models\Album;
-use App\Models\Artist;
-use App\Models\Song;
-use App\Repositories\SongRepository;
-use App\Services\HelperService;
-use App\Services\MediaMetadataService;
+use App\Services\S3Service;
 use Illuminate\Http\Response;
 
 class SongController extends Controller
 {
-    private MediaMetadataService $mediaMetadataService;
-    private SongRepository $songRepository;
-    private HelperService $helperService;
+    private S3Service $s3Service;
 
-    public function __construct(
-        MediaMetadataService $mediaMetadataService,
-        HelperService $helperService,
-        SongRepository $songRepository
-    ) {
-        $this->mediaMetadataService = $mediaMetadataService;
-        $this->songRepository = $songRepository;
-        $this->helperService = $helperService;
+    public function __construct(S3Service $s3Service)
+    {
+        $this->s3Service = $s3Service;
     }
 
     public function put(PutSongRequest $request)
     {
-        $path = "s3://$request->bucket/$request->key";
-
-        $tags = $request->tags;
-        $artist = Artist::getOrCreate(array_get($tags, 'artist'));
-
-        $compilation = (bool) trim(array_get($tags, 'albumartist'));
-        $album = Album::getOrCreate($artist, array_get($tags, 'album'), $compilation);
-        $cover = array_get($tags, 'cover');
-
-        if ($cover) {
-            $this->mediaMetadataService->writeAlbumCover(
-                $album,
-                base64_decode($cover['data'], true),
-                $cover['extension']
-            );
-        }
-
-        $song = Song::updateOrCreate(['id' => $this->helperService->getFileHash($path)], [
-            'path' => $path,
-            'album_id' => $album->id,
-            'artist_id' => $artist->id,
-            'title' => trim(array_get($tags, 'title', '')),
-            'length' => array_get($tags, 'duration', 0) ?: 0,
-            'track' => (int) array_get($tags, 'track'),
-            'lyrics' => array_get($tags, 'lyrics', '') ?: '',
-            'mtime' => time(),
-        ]);
-
-        event(new LibraryChanged());
+        $song = $this->s3Service->createSongEntry(
+            $request->bucket,
+            $request->key,
+            array_get($request->tags, 'artist'),
+            array_get($request->tags, 'album'),
+            (bool) trim(array_get($request->tags, 'albumartist')),
+            array_get($request->tags, 'cover'),
+            trim(array_get($request->tags, 'title', '')),
+            (int) array_get($request->tags, 'duration', 0),
+            (int) array_get($request->tags, 'track'),
+            (string) array_get($request->tags, 'lyrics', '')
+        );
 
         return response()->json($song);
     }
 
     public function remove(RemoveSongRequest $request)
     {
-        $song = $this->songRepository->getOneByPath("s3://$request->bucket/$request->key");
-        abort_unless((bool) $song, Response::HTTP_NOT_FOUND);
+        try {
+            $this->s3Service->deleteSongEntry($request->bucket, $request->key);
+        } catch (SongPathNotFoundException $exception) {
+            abort(Response::HTTP_NOT_FOUND);
+        }
 
-        $song->delete();
-        event(new LibraryChanged());
-
-        return response()->json(null, Response::HTTP_NO_CONTENT);
+        return response()->noContent();
     }
 }

app/Listeners/DeleteNonExistingRecordsPostSync.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace App\Listeners;
+
+use App\Events\MediaSyncCompleted;
+use App\Models\Song;
+use App\Repositories\SongRepository;
+use App\Services\Helper;
+
+class DeleteNonExistingRecordsPostSync
+{
+    private SongRepository $songRepository;
+    private Helper $helper;
+
+    public function __construct(SongRepository $songRepository, Helper $helper)
+    {
+        $this->songRepository = $songRepository;
+        $this->helper = $helper;
+    }
+
+    public function handle(MediaSyncCompleted $event): void
+    {
+        $hashes = $event->result
+            ->validEntries()
+            ->map(fn (string $path): string => $this->helper->getFileHash($path))
+            ->merge($this->songRepository->getAllHostedOnS3()->pluck('id'))
+            ->toArray();
+
+        Song::deleteWhereIDsNotIn($hashes);
+    }
+}

app/Models/Album.php

@@ -2,7 +2,6 @@
 
 namespace App\Models;
 
-use App\Traits\SupportsDeleteWhereIDsNotIn;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Factories\HasFactory;

app/Models/Artist.php

@@ -3,7 +3,6 @@
 namespace App\Models;
 
 use App\Facades\Util;
-use App\Traits\SupportsDeleteWhereIDsNotIn;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Factories\HasFactory;

app/Models/Interaction.php

@@ -2,7 +2,6 @@
 
 namespace App\Models;
 
-use App\Traits\CanFilterByUser;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
@@ -22,7 +21,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
  */
 class Interaction extends Model
 {
-    use CanFilterByUser;
     use HasFactory;
 
     protected $casts = [

app/Models/Playlist.php

@@ -3,7 +3,6 @@
 namespace App\Models;
 
 use App\Casts\SmartPlaylistRulesCast;
-use App\Traits\CanFilterByUser;
 use App\Values\SmartPlaylistRuleGroup;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -27,7 +26,6 @@ use Laravel\Scout\Searchable;
 class Playlist extends Model
 {
     use Searchable;
-    use CanFilterByUser;
     use HasFactory;
 
     protected $hidden = ['user_id', 'created_at', 'updated_at'];

app/Models/Song.php

@@ -3,7 +3,6 @@
 namespace App\Models;
 
 use App\Events\LibraryChanged;
-use App\Traits\SupportsDeleteWhereIDsNotIn;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
@@ -18,7 +17,6 @@ use Laravel\Scout\Searchable;
  * @property string $title
  * @property Album $album
  * @property Artist $artist
- * @property array<string> $s3_params
  * @property float $length
  * @property string $lyrics
  * @property int $track
@@ -43,6 +41,7 @@ class Song extends Model
     use HasFactory;
     use Searchable;
     use SupportsDeleteWhereIDsNotIn;
+    use SupportsS3;
 
     public $incrementing = false;
     protected $guarded = [];
@@ -226,22 +225,6 @@ class Song extends Model
         return str_replace(["\r\n", "\r", "\n"], '<br />', $value);
     }
 
-    /**
-     * Get the bucket and key name of an S3 object.
-     *
-     * @return array<string>|null
-     */
-    public function getS3ParamsAttribute(): ?array
-    {
-        if (!preg_match('/^s3:\\/\\/(.*)/', $this->path, $matches)) {
-            return null;
-        }
-
-        [$bucket, $key] = explode('/', $matches[1], 2);
-
-        return compact('bucket', 'key');
-    }
-
     /** @return array<mixed> */
     public function toSearchableArray(): array
     {

app/Models/SupportsDeleteWhereIDsNotIn.php

@@ -1,10 +1,9 @@
 <?php
 
-namespace App\Traits;
+namespace App\Models;
 
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Support\Facades\DB;
-use Throwable;
 
 /**
  * With reference to GitHub issue #463.
@@ -59,17 +58,10 @@ trait SupportsDeleteWhereIDsNotIn
      */
     public static function deleteByChunk(array $ids, string $key = 'id', int $chunkSize = 65535): void
     {
-        DB::beginTransaction();
-
-        try {
+        DB::transaction(static function () use ($ids, $key, $chunkSize): void {
             foreach (array_chunk($ids, $chunkSize) as $chunk) {
                 static::whereIn($key, $chunk)->delete();
             }
-
-            DB::commit();
-        } catch (Throwable $e) {
-            DB::rollBack();
-            throw $e;
-        }
+        });
     }
 }

app/Models/SupportsS3.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Builder;
+
+/**
+ * @property array<string> $s3_params
+ *
+ * @method static Builder hostedOnS3()
+ */
+trait SupportsS3
+{
+    /**
+     * Get the bucket and key name of an S3 object.
+     *
+     * @return array<string>|null
+     */
+    public function getS3ParamsAttribute(): ?array
+    {
+        if (!preg_match('/^s3:\\/\\/(.*)/', $this->path, $matches)) {
+            return null;
+        }
+
+        [$bucket, $key] = explode('/', $matches[1], 2);
+
+        return compact('bucket', 'key');
+    }
+
+    public static function getPathFromS3BucketAndKey(string $bucket, string $key): string
+    {
+        return "s3://$bucket/$key";
+    }
+
+    public function scopeHostedOnS3(Builder $query): Builder
+    {
+        return $query->where('path', 'LIKE', 's3://%');
+    }
+}

app/Observers/SongObserver.php

@@ -3,15 +3,15 @@
 namespace App\Observers;
 
 use App\Models\Song;
-use App\Services\HelperService;
+use App\Services\Helper;
 
 class SongObserver
 {
-    private HelperService $helperService;
+    private Helper $helper;
 
-    public function __construct(HelperService $helperService)
+    public function __construct(Helper $helper)
     {
-        $this->helperService = $helperService;
+        $this->helper = $helper;
     }
 
     public function creating(Song $song): void
@@ -21,6 +21,6 @@ class SongObserver
 
     private function setFileHashAsId(Song $song): void
     {
-        $song->id = $this->helperService->getFileHash($song->path);
+        $song->id = $this->helper->getFileHash($song->path);
     }
 }

app/Providers/EventServiceProvider.php

@@ -6,11 +6,13 @@ use App\Events\AlbumInformationFetched;
 use App\Events\ArtistInformationFetched;
 use App\Events\LibraryChanged;
 use App\Events\MediaCacheObsolete;
+use App\Events\MediaSyncCompleted;
 use App\Events\SongLikeToggled;
 use App\Events\SongsBatchLiked;
 use App\Events\SongsBatchUnliked;
 use App\Events\SongStartedPlaying;
 use App\Listeners\ClearMediaCache;
+use App\Listeners\DeleteNonExistingRecordsPostSync;
 use App\Listeners\DownloadAlbumCover;
 use App\Listeners\DownloadArtistImage;
 use App\Listeners\LoveMultipleTracksOnLastfm;
@@ -59,6 +61,10 @@ class EventServiceProvider extends ServiceProvider
         ArtistInformationFetched::class => [
             DownloadArtistImage::class,
         ],
+
+        MediaSyncCompleted::class => [
+            DeleteNonExistingRecordsPostSync::class,
+        ],
     ];
 
     public function boot(): void

app/Repositories/SongRepository.php

@@ -4,23 +4,30 @@ namespace App\Repositories;
 
 use App\Models\Song;
 use App\Repositories\Traits\Searchable;
-use App\Services\HelperService;
+use App\Services\Helper;
+use Illuminate\Support\Collection;
 
 class SongRepository extends AbstractRepository
 {
     use Searchable;
 
-    private HelperService $helperService;
+    private Helper $helper;
 
-    public function __construct(HelperService $helperService)
+    public function __construct(Helper $helper)
     {
         parent::__construct();
 
-        $this->helperService = $helperService;
+        $this->helper = $helper;
     }
 
     public function getOneByPath(string $path): ?Song
     {
-        return $this->getOneById($this->helperService->getFileHash($path));
+        return $this->getOneById($this->helper->getFileHash($path));
+    }
+
+    /** @return Collection|array<Song> */
+    public function getAllHostedOnS3(): Collection
+    {
+        return Song::hostedOnS3()->get();
     }
 }

app/Services/FileSynchronizer.php

@@ -22,7 +22,7 @@ class FileSynchronizer
 
     private getID3 $getID3;
     private MediaMetadataService $mediaMetadataService;
-    private HelperService $helperService;
+    private Helper $helper;
     private SongRepository $songRepository;
     private Cache $cache;
     private Finder $finder;
@@ -45,14 +45,14 @@ class FileSynchronizer
     public function __construct(
         getID3 $getID3,
         MediaMetadataService $mediaMetadataService,
-        HelperService $helperService,
+        Helper $helper,
         SongRepository $songRepository,
         Cache $cache,
         Finder $finder
     ) {
         $this->getID3 = $getID3;
         $this->mediaMetadataService = $mediaMetadataService;
-        $this->helperService = $helperService;
+        $this->helper = $helper;
         $this->songRepository = $songRepository;
         $this->cache = $cache;
         $this->finder = $finder;
@@ -73,7 +73,7 @@ class FileSynchronizer
         }
 
         $this->filePath = $splFileInfo->getPathname();
-        $this->fileHash = $this->helperService->getFileHash($this->filePath);
+        $this->fileHash = $this->helper->getFileHash($this->filePath);
         $this->song = $this->songRepository->getOneById($this->fileHash); // @phpstan-ignore-line
         $this->syncError = null;
 

app/Services/Helper.php

@@ -2,7 +2,7 @@
 
 namespace App\Services;
 
-class HelperService
+class Helper
 {
     /**
      * Get a unique hash from a file path.

app/Services/MediaSyncService.php

@@ -4,6 +4,7 @@ namespace App\Services;
 
 use App\Console\Commands\SyncCommand;
 use App\Events\LibraryChanged;
+use App\Events\MediaSyncCompleted;
 use App\Libraries\WatchRecord\WatchRecordInterface;
 use App\Models\Album;
 use App\Models\Artist;
@@ -12,6 +13,7 @@ use App\Models\Song;
 use App\Repositories\AlbumRepository;
 use App\Repositories\ArtistRepository;
 use App\Repositories\SongRepository;
+use App\Values\SyncResult;
 use Psr\Log\LoggerInterface;
 use SplFileInfo;
 use Symfony\Component\Finder\Finder;
@@ -36,7 +38,6 @@ class MediaSyncService
     ];
 
     private SongRepository $songRepository;
-    private HelperService $helperService;
     private FileSynchronizer $fileSynchronizer;
     private Finder $finder;
     private ArtistRepository $artistRepository;
@@ -47,13 +48,11 @@ class MediaSyncService
         SongRepository $songRepository,
         ArtistRepository $artistRepository,
         AlbumRepository $albumRepository,
-        HelperService $helperService,
         FileSynchronizer $fileSynchronizer,
         Finder $finder,
         LoggerInterface $logger
     ) {
         $this->songRepository = $songRepository;
-        $this->helperService = $helperService;
         $this->fileSynchronizer = $fileSynchronizer;
         $this->finder = $finder;
         $this->artistRepository = $artistRepository;
@@ -73,7 +72,7 @@ class MediaSyncService
      * Only taken into account for existing records.
      * New records will have all tags synced in regardless.
      * @param bool $force Whether to force syncing even unchanged files
-     * @param SyncCommand $syncCommand the SyncMedia command object, to log to console if executed by artisan
+     * @param SyncCommand $syncCommand The SyncMedia command object, to log to console if executed by artisan
      */
     public function sync(
         ?string $mediaPath = null,
@@ -84,11 +83,7 @@ class MediaSyncService
         $this->setSystemRequirements();
         $this->setTags($tags);
 
-        $results = [
-            'success' => [],
-            'bad_files' => [],
-            'unmodified' => [],
-        ];
+        $syncResult = SyncResult::init();
 
         $songPaths = $this->gatherFiles($mediaPath ?: Setting::get('media_path'));
 
@@ -101,15 +96,15 @@ class MediaSyncService
 
             switch ($result) {
                 case FileSynchronizer::SYNC_RESULT_SUCCESS:
-                    $results['success'][] = $path;
+                    $syncResult->success->add($path);
                     break;
 
                 case FileSynchronizer::SYNC_RESULT_UNMODIFIED:
-                    $results['unmodified'][] = $path;
+                    $syncResult->unmodified->add($path);
                     break;
 
                 default:
-                    $results['bad_files'][] = $path;
+                    $syncResult->bad->add($path);
                     break;
             }
 
@@ -119,13 +114,7 @@ class MediaSyncService
             }
         }
 
-        // Delete non-existing songs.
-        $hashes = array_map(
-            fn (string $path): string => $this->helperService->getFileHash($path),
-            array_merge($results['unmodified'], $results['success'])
-        );
-
-        Song::deleteWhereIDsNotIn($hashes);
+        event(new MediaSyncCompleted($syncResult));
 
         // Trigger LibraryChanged, so that TidyLibrary handler is fired to, erm, tidy our library.
         event(new LibraryChanged());

app/Services/S3Service.php

@@ -2,7 +2,12 @@
 
 namespace App\Services;
 
+use App\Events\LibraryChanged;
+use App\Exceptions\SongPathNotFoundException;
+use App\Models\Album;
+use App\Models\Artist;
 use App\Models\Song;
+use App\Repositories\SongRepository;
 use Aws\S3\S3ClientInterface;
 use Illuminate\Cache\Repository as Cache;
 
@@ -10,11 +15,22 @@ class S3Service implements ObjectStorageInterface
 {
     private ?S3ClientInterface $s3Client;
     private Cache $cache;
+    private MediaMetadataService $mediaMetadataService;
+    private SongRepository $songRepository;
+    private Helper $helper;
 
-    public function __construct(?S3ClientInterface $s3Client, Cache $cache)
-    {
+    public function __construct(
+        ?S3ClientInterface $s3Client,
+        Cache $cache,
+        MediaMetadataService $mediaMetadataService,
+        SongRepository $songRepository,
+        Helper $helper
+    ) {
         $this->s3Client = $s3Client;
         $this->cache = $cache;
+        $this->mediaMetadataService = $mediaMetadataService;
+        $this->songRepository = $songRepository;
+        $this->helper = $helper;
     }
 
     public function getSongPublicUrl(Song $song): string
@@ -31,4 +47,58 @@ class S3Service implements ObjectStorageInterface
             return (string) $request->getUri();
         });
     }
+
+    public function createSongEntry(
+        string $bucket,
+        string $key,
+        string $artistName,
+        string $albumName,
+        bool $compilation,
+        ?array $cover,
+        string $title,
+        float $duration,
+        int $track,
+        string $lyrics
+    ): Song {
+        $path = Song::getPathFromS3BucketAndKey($bucket, $key);
+
+        $artist = Artist::getOrCreate($artistName);
+        $album = Album::getOrCreate($artist, $albumName, $compilation);
+
+        if ($cover) {
+            $this->mediaMetadataService->writeAlbumCover(
+                $album,
+                base64_decode($cover['data'], true),
+                $cover['extension']
+            );
+        }
+
+        $song = Song::updateOrCreate(['id' => $this->helper->getFileHash($path)], [
+            'path' => $path,
+            'album_id' => $album->id,
+            'artist_id' => $artist->id,
+            'title' => $title,
+            'length' => $duration,
+            'track' => $track,
+            'lyrics' => $lyrics,
+            'mtime' => time(),
+        ]);
+
+        event(new LibraryChanged());
+
+        return $song;
+    }
+
+    public function deleteSongEntry(string $bucket, string $key): void
+    {
+        $path = Song::getPathFromS3BucketAndKey($bucket, $key);
+        $song = $this->songRepository->getOneByPath($path);
+
+        if (!$song) {
+            throw SongPathNotFoundException::create($path);
+        }
+
+        $song->delete();
+        event(new LibraryChanged());
+    }
 }

app/Traits/CanFilterByUser.php

@@ -1,16 +0,0 @@
-<?php
-
-namespace App\Traits;
-
-use Illuminate\Database\Eloquent\Builder;
-
-/**
- * Indicate that a (Model) object collection can be filtered by the current authenticated user.
- */
-trait CanFilterByUser
-{
-    public function scopeByCurrentUser(Builder $query): Builder
-    {
-        return $query->where('user_id', auth()->user()->id);
-    }
-}

app/Values/SyncResult.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace App\Values;
+
+use Illuminate\Support\Collection;
+
+final class SyncResult
+{
+    /** @var Collection|array<string> */
+    public Collection $success;
+
+    /** @var Collection|array<string> */
+    public Collection $bad;
+
+    /** @var Collection|array<string> */
+    public Collection $unmodified;
+
+    private function __construct(Collection $success, Collection $bad, Collection $unmodified)
+    {
+        $this->success = $success;
+        $this->bad = $bad;
+        $this->unmodified = $unmodified;
+    }
+
+    public static function init(): self
+    {
+        return new self(collect(), collect(), collect());
+    }
+
+    /** @return Collection|array<string> */
+    public function validEntries(): Collection
+    {
+        return $this->success->merge($this->unmodified);
+    }
+}

tests/Feature/ObjectStorage/S3Test.php

@@ -33,7 +33,15 @@ class S3Test extends TestCase
             ],
         ]);
 
-        self::assertDatabaseHas('songs', ['path' => 's3://koel/sample.mp3']);
+        /** @var Song $song */
+        $song = Song::where('path', 's3://koel/sample.mp3')->firstOrFail();
+
+        self::assertSame('A Koel Song', $song->title);
+        self::assertSame('Koel Testing Vol. 1', $song->album->name);
+        self::assertSame('Koel', $song->artist->name);
+        self::assertSame('When you wake up, turn your radio on, and you\'ll hear this simple song', $song->lyrics);
+        self::assertEquals(10, $song->length);
+        self::assertSame(5, $song->track);
     }
 
     public function testRemovingASong(): void
@@ -49,6 +57,6 @@ class S3Test extends TestCase
             'key' => 'sample.mp3',
         ]);
 
-        self::assertDatabaseMissing('songs', ['path' => 's3://koel/sample.mp3']);
+        self::assertDatabaseMissing(Song::class, ['path' => 's3://koel/sample.mp3']);
     }
 }

tests/Integration/Listeners/DeleteNonExistingRecordsPostSyncTest.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace Tests\Integration\Listeners;
+
+use App\Events\MediaSyncCompleted;
+use App\Listeners\DeleteNonExistingRecordsPostSync;
+use App\Models\Song;
+use App\Values\SyncResult;
+use Illuminate\Database\Eloquent\Collection;
+use Tests\TestCase;
+
+class DeleteNonExistingRecordsPostSyncTest extends TestCase
+{
+    private DeleteNonExistingRecordsPostSync $listener;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->listener = app(DeleteNonExistingRecordsPostSync::class);
+    }
+
+    public function testHandleDoesNotDeleteS3Entries(): void
+    {
+        $song = Song::factory()->create(['path' => 's3://do-not/delete-me.mp3']);
+        $this->listener->handle(new MediaSyncCompleted(SyncResult::init()));
+
+        self::assertModelExists($song);
+    }
+
+    public function testHandle(): void
+    {
+        /** @var Collection|array<Song> $songs */
+        $songs = Song::factory(4)->create();
+
+        self::assertCount(4, Song::all());
+
+        $syncResult = SyncResult::init();
+        $syncResult->success->add($songs[0]->path);
+        $syncResult->unmodified->add($songs[3]->path);
+
+        $this->listener->handle(new MediaSyncCompleted($syncResult));
+
+        self::assertModelMissing($songs[1]);
+        self::assertModelMissing($songs[2]);
+    }
+}

tests/Integration/Repositories/SongRepositoryTest.php

@@ -4,7 +4,7 @@ namespace Tests\Integration\Repositories;
 
 use App\Models\Song;
 use App\Repositories\SongRepository;
-use App\Services\HelperService;
+use App\Services\Helper;
 use Tests\TestCase;
 
 class SongRepositoryTest extends TestCase
@@ -15,7 +15,7 @@ class SongRepositoryTest extends TestCase
     {
         parent::setUp();
 
-        $this->songRepository = new SongRepository(new HelperService());
+        $this->songRepository = new SongRepository(new Helper());
     }
 
     public function testGetOneByPath(): void

tests/Integration/Services/MediaSyncServiceTest.php

@@ -1,8 +1,9 @@
 <?php
 
-namespace Tests\Feature;
+namespace Tests\Integration\Services;
 
 use App\Events\LibraryChanged;
+use App\Events\MediaSyncCompleted;
 use App\Libraries\WatchRecord\InotifyWatchRecord;
 use App\Models\Album;
 use App\Models\Artist;
@@ -12,8 +13,9 @@ use App\Services\MediaSyncService;
 use getID3;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
 use Mockery;
+use Tests\Feature\TestCase;
 
-class MediaSyncTest extends TestCase
+class MediaSyncServiceTest extends TestCase
 {
     use WithoutMiddleware;
 
@@ -29,12 +31,12 @@ class MediaSyncTest extends TestCase
 
     public function testSync(): void
     {
-        $this->expectsEvents(LibraryChanged::class);
+        $this->expectsEvents(LibraryChanged::class, MediaSyncCompleted::class);
 
         $this->mediaService->sync($this->mediaPath);
 
         // Standard mp3 files under root path should be recognized
-        self::assertDatabaseHas('songs', [
+        self::assertDatabaseHas(Song::class, [
             'path' => $this->mediaPath . '/full.mp3',
             // Track # should be recognized
             'track' => 5,
@@ -90,11 +92,12 @@ class MediaSyncTest extends TestCase
 
     public function testForceSync(): void
     {
-        $this->expectsEvents(LibraryChanged::class);
+        $this->expectsEvents(LibraryChanged::class, MediaSyncCompleted::class);
 
         $this->mediaService->sync($this->mediaPath);
 
         // Make some modification to the records
+        /** @var Song $song */
         $song = Song::orderBy('id', 'desc')->first();
         $originalTitle = $song->title;
         $originalLyrics = $song->lyrics;
@@ -108,6 +111,7 @@ class MediaSyncTest extends TestCase
         $this->mediaService->sync($this->mediaPath);
 
         // Validate that the changes are not lost
+        /** @var Song $song */
         $song = Song::orderBy('id', 'desc')->first();
         self::assertEquals("It's John Cena!", $song->title);
         self::assertEquals('Booom Wroooom', $song->lyrics);
@@ -116,6 +120,7 @@ class MediaSyncTest extends TestCase
         $this->mediaService->sync($this->mediaPath, [], true);
 
         // All is lost.
+        /** @var Song $song */
         $song = Song::orderBy('id', 'desc')->first();
         self::assertEquals($originalTitle, $song->title);
         self::assertEquals($originalLyrics, $song->lyrics);
@@ -123,11 +128,12 @@ class MediaSyncTest extends TestCase
 
     public function testSelectiveSync(): void
     {
-        $this->expectsEvents(LibraryChanged::class);
+        $this->expectsEvents(LibraryChanged::class, MediaSyncCompleted::class);
 
         $this->mediaService->sync($this->mediaPath);
 
         // Make some modification to the records
+        /** @var Song $song */
         $song = Song::orderBy('id', 'desc')->first();
         $originalTitle = $song->title;
 
@@ -189,11 +195,11 @@ class MediaSyncTest extends TestCase
 
     public function testSyncDeletedDirectoryViaWatch(): void
     {
-        $this->expectsEvents(LibraryChanged::class);
+        $this->expectsEvents(LibraryChanged::class, MediaSyncCompleted::class);
 
         $this->mediaService->sync($this->mediaPath);
 
-        $this->mediaService->syncByWatchRecord(new InotifyWatchRecord("MOVED_FROM,ISDIR {$this->mediaPath}/subdir"));
+        $this->mediaService->syncByWatchRecord(new InotifyWatchRecord("MOVED_FROM,ISDIR $this->mediaPath/subdir"));
 
         self::assertDatabaseMissing('songs', ['path' => $this->mediaPath . '/subdir/sic.mp3']);
         self::assertDatabaseMissing('songs', ['path' => $this->mediaPath . '/subdir/no-name.mp3']);

tests/Unit/Listeners/LoveTrackOnLastFmTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tests\Integration\Listeners;
+namespace Tests\Unit\Listeners;
 
 use App\Events\SongLikeToggled;
 use App\Listeners\LoveTrackOnLastfm;

tests/Unit/Listeners/UpdateLastfmNowPlayingTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tests\Integration\Listeners;
+namespace Tests\Unit\Listeners;
 
 use App\Events\SongStartedPlaying;
 use App\Listeners\UpdateLastfmNowPlaying;

tests/Unit/Services/S3ServiceTest.php

@@ -1,8 +1,11 @@
 <?php
 
-namespace Tests\Integration\Services;
+namespace Tests\Unit\Services;
 
 use App\Models\Song;
+use App\Repositories\SongRepository;
+use App\Services\Helper;
+use App\Services\MediaMetadataService;
 use App\Services\S3Service;
 use Aws\CommandInterface;
 use Aws\S3\S3ClientInterface;
@@ -15,6 +18,9 @@ class S3ServiceTest extends TestCase
 {
     private $s3Client;
     private $cache;
+    private $metadataService;
+    private $songRepository;
+    private $helper;
     private S3Service $s3Service;
 
     public function setUp(): void
@@ -23,7 +29,17 @@ class S3ServiceTest extends TestCase
 
         $this->s3Client = Mockery::mock(S3ClientInterface::class);
         $this->cache = Mockery::mock(Cache::class);
-        $this->s3Service = new S3Service($this->s3Client, $this->cache);
+        $this->metadataService = Mockery::mock(MediaMetadataService::class);
+        $this->songRepository = Mockery::mock(SongRepository::class);
+        $this->helper = Mockery::mock(Helper::class);
+
+        $this->s3Service = new S3Service(
+            $this->s3Client,
+            $this->cache,
+            $this->metadataService,
+            $this->songRepository,
+            $this->helper
+        );
     }
 
     public function testGetSongPublicUrl(): void