From aa7ddd9d94da318d2ae767eaa0764d02fa0d248f Mon Sep 17 00:00:00 2001 From: Phan An Date: Sat, 9 Nov 2024 15:56:48 +0100 Subject: [PATCH] refactor: avoid leadking database keys (#1874) --- .../Commands/Admin/ChangePasswordCommand.php | 2 +- app/Console/Commands/ScanCommand.php | 4 +- .../UserAlreadySubscribedToPodcast.php | 2 +- .../API/PrivatizeSongsController.php | 3 ++ .../API/PublicizeSongsController.php | 3 ++ .../API/UploadAlbumCoverController.php | 2 +- .../API/UploadArtistImageController.php | 2 +- .../Requests/API/ProfileUpdateRequest.php | 2 +- app/Models/Album.php | 1 - app/Models/Playlist.php | 11 ++--- app/Models/PlaylistFolder.php | 3 +- app/Models/Song.php | 1 + app/Models/User.php | 2 +- app/Policies/SongPolicy.php | 1 + app/Providers/MacroProvider.php | 2 +- app/Repositories/AlbumRepository.php | 2 +- app/Repositories/ArtistRepository.php | 3 +- .../Contracts/RepositoryInterface.php | 5 +-- app/Repositories/PodcastRepository.php | 2 +- app/Repositories/Repository.php | 7 ++-- app/Repositories/SongRepository.php | 11 +++-- app/Rules/AllPlaylistsAreAccessibleBy.php | 2 +- app/Services/DownloadService.php | 4 +- app/Services/InteractionService.php | 28 ++++++++----- app/Services/LastfmService.php | 4 +- app/Services/MediaInformationService.php | 22 +++++----- app/Services/PlaylistService.php | 5 ++- app/Services/QueueService.php | 17 ++++---- app/Services/SearchService.php | 2 +- app/Services/SmartPlaylistService.php | 2 +- app/Services/SongService.php | 40 +++++++++---------- app/Values/LicenseInstance.php | 6 +-- app/Values/Podcast/EpisodePlayable.php | 4 +- database/factories/QueueStateFactory.php | 2 +- phpstan.neon.dist | 2 + tests/Feature/AlbumCoverTest.php | 4 +- tests/Feature/AlbumInformationTest.php | 2 +- tests/Feature/AlbumSongTest.php | 3 +- tests/Feature/ArtistAlbumTest.php | 3 +- tests/Feature/ArtistImageTest.php | 6 +-- tests/Feature/ArtistInformationTest.php | 2 +- tests/Feature/ArtistSongTest.php | 2 +- tests/Feature/DownloadTest.php | 14 +++---- tests/Feature/InteractionTest.php | 2 +- tests/Feature/KoelPlus/AlbumCoverTest.php | 6 +-- tests/Feature/KoelPlus/ArtistImageTest.php | 10 +++-- tests/Feature/KoelPlus/DownloadTest.php | 6 +-- tests/Feature/KoelPlus/InteractionTest.php | 24 ++++------- .../KoelPlus/PlaylistCollaborationTest.php | 2 +- tests/Feature/KoelPlus/PlaylistCoverTest.php | 8 +++- tests/Feature/KoelPlus/PlaylistFolderTest.php | 10 ++--- tests/Feature/KoelPlus/PlaylistSongTest.php | 8 ++-- tests/Feature/KoelPlus/PlaylistTest.php | 4 +- tests/Feature/KoelPlus/SongPlayTest.php | 6 +-- tests/Feature/KoelPlus/SongTest.php | 36 +++++++---------- tests/Feature/KoelPlus/SongVisibilityTest.php | 11 ++--- tests/Feature/PlayCountTest.php | 6 +-- tests/Feature/PlaylistCoverTest.php | 12 ++++-- tests/Feature/PlaylistFolderTest.php | 28 ++++++++----- tests/Feature/PlaylistSongTest.php | 29 ++++++-------- tests/Feature/PlaylistTest.php | 22 ++++------ tests/Feature/QueueTest.php | 4 +- tests/Feature/ScrobbleTest.php | 2 +- tests/Feature/SongPlayTest.php | 6 +-- tests/Feature/SongTest.php | 17 +++----- tests/Feature/SongVisibilityTest.php | 4 +- tests/Feature/UserTest.php | 6 +-- .../Services/SmartPlaylistServiceTest.php | 4 +- .../Services/InteractionServiceTest.php | 7 ++-- .../Services/PlaylistServiceTest.php | 24 +++++------ .../Integration/Services/QueueServiceTest.php | 6 +-- .../Services/SmartPlaylistServiceTest.php | 6 +-- .../Values/EpisodePlayableTest.php | 2 +- tests/Unit/Models/AlbumTest.php | 2 +- .../Services/MediaInformationServiceTest.php | 4 +- .../Services/PlaylistFolderServiceTest.php | 8 ++-- 76 files changed, 286 insertions(+), 293 deletions(-) diff --git a/app/Console/Commands/Admin/ChangePasswordCommand.php b/app/Console/Commands/Admin/ChangePasswordCommand.php index 0e40e5e2..6bd9bd4e 100644 --- a/app/Console/Commands/Admin/ChangePasswordCommand.php +++ b/app/Console/Commands/Admin/ChangePasswordCommand.php @@ -34,7 +34,7 @@ class ChangePasswordCommand extends Command return self::FAILURE; } - $this->comment("Changing the user's password (ID: $user->id, email: $user->email)"); + $this->comment("Changing the user's password (ID: {$user->id}, email: $user->email)"); $user->password = $this->hash->make($this->askForPassword()); $user->save(); diff --git a/app/Console/Commands/ScanCommand.php b/app/Console/Commands/ScanCommand.php index 807044d4..9eefa7e1 100644 --- a/app/Console/Commands/ScanCommand.php +++ b/app/Console/Commands/ScanCommand.php @@ -173,7 +173,7 @@ class ScanCommand extends Command exit(self::INVALID); }); - $this->components->info("Setting owner to $user->name (ID $user->id)."); + $this->components->info("Setting owner to $user->name (ID {$user->id})."); return $user; } @@ -181,7 +181,7 @@ class ScanCommand extends Command $user = $this->userRepository->getDefaultAdminUser(); $this->components->warn( - "No song owner specified. Setting the first admin ($user->name, ID $user->id) as owner." + "No song owner specified. Setting the first admin ($user->name, ID {$user->id}) as owner." ); return $user; diff --git a/app/Exceptions/UserAlreadySubscribedToPodcast.php b/app/Exceptions/UserAlreadySubscribedToPodcast.php index cc104bac..74aa3aa1 100644 --- a/app/Exceptions/UserAlreadySubscribedToPodcast.php +++ b/app/Exceptions/UserAlreadySubscribedToPodcast.php @@ -10,6 +10,6 @@ final class UserAlreadySubscribedToPodcast extends Exception { public static function make(User $user, Podcast $podcast): self { - return new self("User $user->id has already subscribed to podcast $podcast->id"); + return new self("User {$user->id} has already subscribed to podcast {$podcast->id}"); } } diff --git a/app/Http/Controllers/API/PrivatizeSongsController.php b/app/Http/Controllers/API/PrivatizeSongsController.php index a63746fa..94f771d3 100644 --- a/app/Http/Controllers/API/PrivatizeSongsController.php +++ b/app/Http/Controllers/API/PrivatizeSongsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\API; +use App\Facades\License; use App\Http\Controllers\Controller; use App\Http\Requests\API\ChangeSongsVisibilityRequest; use App\Models\Song; @@ -14,6 +15,8 @@ class PrivatizeSongsController extends Controller /** @param User $user */ public function __invoke(ChangeSongsVisibilityRequest $request, SongService $songService, Authenticatable $user) { + License::requirePlus(); + $songs = Song::query()->findMany($request->songs); $songs->each(fn ($song) => $this->authorize('own', $song)); diff --git a/app/Http/Controllers/API/PublicizeSongsController.php b/app/Http/Controllers/API/PublicizeSongsController.php index 7ebd3383..210520b9 100644 --- a/app/Http/Controllers/API/PublicizeSongsController.php +++ b/app/Http/Controllers/API/PublicizeSongsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\API; +use App\Facades\License; use App\Http\Controllers\Controller; use App\Http\Requests\API\ChangeSongsVisibilityRequest; use App\Models\Song; @@ -14,6 +15,8 @@ class PublicizeSongsController extends Controller /** @param User $user */ public function __invoke(ChangeSongsVisibilityRequest $request, SongService $songService, Authenticatable $user) { + License::requirePlus(); + $songs = Song::query()->findMany($request->songs); $songs->each(fn ($song) => $this->authorize('own', $song)); diff --git a/app/Http/Controllers/API/UploadAlbumCoverController.php b/app/Http/Controllers/API/UploadAlbumCoverController.php index 3ad5cb44..4f2d6b12 100644 --- a/app/Http/Controllers/API/UploadAlbumCoverController.php +++ b/app/Http/Controllers/API/UploadAlbumCoverController.php @@ -15,7 +15,7 @@ class UploadAlbumCoverController extends Controller $this->authorize('update', $album); $metadataService->writeAlbumCover($album, $request->getFileContent()); - Cache::delete("album.info.$album->id"); + Cache::delete("album.info.{$album->id}"); return response()->json(['cover_url' => $album->cover]); } diff --git a/app/Http/Controllers/API/UploadArtistImageController.php b/app/Http/Controllers/API/UploadArtistImageController.php index 9d565451..8701e3f8 100644 --- a/app/Http/Controllers/API/UploadArtistImageController.php +++ b/app/Http/Controllers/API/UploadArtistImageController.php @@ -15,7 +15,7 @@ class UploadArtistImageController extends Controller $this->authorize('update', $artist); $metadataService->writeArtistImage($artist, $request->getFileContent()); - Cache::delete("artist.info.$artist->id"); + Cache::delete("artist.info.{$artist->id}"); return response()->json(['image_url' => $artist->image]); } diff --git a/app/Http/Requests/API/ProfileUpdateRequest.php b/app/Http/Requests/API/ProfileUpdateRequest.php index f7c397c6..685372c8 100644 --- a/app/Http/Requests/API/ProfileUpdateRequest.php +++ b/app/Http/Requests/API/ProfileUpdateRequest.php @@ -18,7 +18,7 @@ class ProfileUpdateRequest extends Request { return [ 'name' => 'required', - 'email' => 'required|email|unique:users,email,' . auth()->user()->id, + 'email' => 'required|email|unique:users,email,' . auth()->user()->getAuthIdentifier(), 'current_password' => 'sometimes|required_with:new_password', 'new_password' => ['sometimes', Password::defaults()], ]; diff --git a/app/Models/Album.php b/app/Models/Album.php index 8493e194..6fbcee16 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -19,7 +19,6 @@ use Laravel\Scout\Searchable; * @property string $cover The album cover's URL * @property string|null $cover_path The absolute path to the cover file * @property bool $has_cover If the album has a non-default cover image - * @property int $id * @property string $name Name of the album * @property Artist $artist The album's artist * @property int $artist_id diff --git a/app/Models/Playlist.php b/app/Models/Playlist.php index 81022109..987657c0 100644 --- a/app/Models/Playlist.php +++ b/app/Models/Playlist.php @@ -8,6 +8,7 @@ use App\Models\Song as Playable; use App\Values\SmartPlaylistRuleGroupCollection; use Carbon\Carbon; use Illuminate\Database\Eloquent\Casts\Attribute; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Database\Eloquent\Concerns\HasUuids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -24,16 +25,16 @@ use Laravel\Scout\Searchable; * @property bool $is_smart * @property int $user_id * @property User $user - * @property Collection $playables * @property ?SmartPlaylistRuleGroupCollection $rule_groups * @property ?SmartPlaylistRuleGroupCollection $rules * @property Carbon $created_at + * @property EloquentCollection $playables + * @property EloquentCollection $collaborators * @property bool $own_songs_only - * @property Collection $collaborators - * @property-read bool $is_collaborative * @property-read ?string $cover The playlist cover's URL * @property-read ?string $cover_path - * @property-read Collection $folders + * @property-read EloquentCollection $folders + * @property-read bool $is_collaborative */ class Playlist extends Model { @@ -107,7 +108,7 @@ class Playlist extends Model public function ownedBy(User $user): bool { - return $this->user_id === $user->id; + return $this->user->is($user); } public function inFolder(PlaylistFolder $folder): bool diff --git a/app/Models/PlaylistFolder.php b/app/Models/PlaylistFolder.php index d91ef381..9a840065 100644 --- a/app/Models/PlaylistFolder.php +++ b/app/Models/PlaylistFolder.php @@ -11,7 +11,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsToMany; /** - * @property string $id * @property string $name * @property User $user * @property Collection $playlists @@ -38,6 +37,6 @@ class PlaylistFolder extends Model public function ownedBy(User $user): bool { - return $this->user_id === $user->id; + return $this->user->is($user); } } diff --git a/app/Models/Song.php b/app/Models/Song.php index 65d400e1..44df5cea 100644 --- a/app/Models/Song.php +++ b/app/Models/Song.php @@ -162,6 +162,7 @@ class Song extends Model public function ownedBy(User $user): bool { + // Do not use $song->owner->is($user) here, as it may trigger an extra query. return $this->owner_id === $user->id; } diff --git a/app/Models/User.php b/app/Models/User.php index 0dec136e..958f8f80 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -92,7 +92,7 @@ class User extends Authenticatable public function subscribedToPodcast(Podcast $podcast): bool { - return $this->podcasts()->where('podcast_id', $podcast->id)->exists(); + return $this->podcasts()->whereKey($podcast)->exists(); } public function subscribeToPodcast(Podcast $podcast): void diff --git a/app/Policies/SongPolicy.php b/app/Policies/SongPolicy.php index 9798e3cb..7a76fab0 100644 --- a/app/Policies/SongPolicy.php +++ b/app/Policies/SongPolicy.php @@ -10,6 +10,7 @@ class SongPolicy { public function own(User $user, Song $song): bool { + // Do not use $song->owner->is($user) here, as it may trigger an extra query. return $song->owner_id === $user->id; } diff --git a/app/Providers/MacroProvider.php b/app/Providers/MacroProvider.php index 902f11e5..81199129 100644 --- a/app/Providers/MacroProvider.php +++ b/app/Providers/MacroProvider.php @@ -14,7 +14,7 @@ class MacroProvider extends ServiceProvider { public function boot(): void { - Collection::macro('orderByArray', function (array $orderBy, string $key = 'id'): Collection { + Collection::macro('orderByArray', function (array $orderBy, string $key = 'id') { /** @var Collection $this */ return $this->sortBy(static fn ($item) => array_search($item->$key, $orderBy, true))->values(); }); diff --git a/app/Repositories/AlbumRepository.php b/app/Repositories/AlbumRepository.php index dbafc0fa..1b96f245 100644 --- a/app/Repositories/AlbumRepository.php +++ b/app/Repositories/AlbumRepository.php @@ -8,8 +8,8 @@ use App\Models\Album; use App\Models\Artist; use App\Models\User; use Illuminate\Contracts\Pagination\Paginator; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Query\JoinClause; -use Illuminate\Support\Collection; /** * @extends Repository diff --git a/app/Repositories/ArtistRepository.php b/app/Repositories/ArtistRepository.php index 1dac0c88..9de20dbc 100644 --- a/app/Repositories/ArtistRepository.php +++ b/app/Repositories/ArtistRepository.php @@ -9,7 +9,6 @@ use App\Models\User; use Illuminate\Contracts\Pagination\Paginator; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Query\JoinClause; -use Illuminate\Support\Collection as BaseCollection; /** @extends Repository */ class ArtistRepository extends Repository @@ -44,7 +43,7 @@ class ArtistRepository extends Repository } /** @return Collection|array */ - public function getMany(array $ids, bool $preserveOrder = false, ?User $user = null): Collection|BaseCollection + public function getMany(array $ids, bool $preserveOrder = false, ?User $user = null): Collection { $artists = Artist::query() ->isStandard() diff --git a/app/Repositories/Contracts/RepositoryInterface.php b/app/Repositories/Contracts/RepositoryInterface.php index 7eeaac9e..1353bd06 100644 --- a/app/Repositories/Contracts/RepositoryInterface.php +++ b/app/Repositories/Contracts/RepositoryInterface.php @@ -2,9 +2,8 @@ namespace App\Repositories\Contracts; -use Illuminate\Database\Eloquent\Collection as EloquentCollection; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Collection; /** @template T of Model */ interface RepositoryInterface @@ -25,7 +24,7 @@ interface RepositoryInterface public function getMany(array $ids, bool $preserveOrder = false): Collection; /** @return Collection */ - public function getAll(): EloquentCollection; + public function getAll(): Collection; /** @return T|null */ public function findFirstWhere(...$params): ?Model; diff --git a/app/Repositories/PodcastRepository.php b/app/Repositories/PodcastRepository.php index 857ad60f..7e9b3d54 100644 --- a/app/Repositories/PodcastRepository.php +++ b/app/Repositories/PodcastRepository.php @@ -4,7 +4,7 @@ namespace App\Repositories; use App\Models\Podcast; use App\Models\User; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; /** @extends Repository */ class PodcastRepository extends Repository diff --git a/app/Repositories/Repository.php b/app/Repositories/Repository.php index 03d28b0e..cabc7cf4 100644 --- a/app/Repositories/Repository.php +++ b/app/Repositories/Repository.php @@ -4,9 +4,8 @@ namespace App\Repositories; use App\Repositories\Contracts\RepositoryInterface; use Illuminate\Contracts\Auth\Guard; -use Illuminate\Database\Eloquent\Collection as EloquentCollection; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Collection; /** * @template T of Model @@ -68,9 +67,9 @@ abstract class Repository implements RepositoryInterface } /** @inheritDoc */ // @phpcs:ignore - public function getAll(): EloquentCollection + public function getAll(): Collection { - return $this->modelClass::all(); + return $this->modelClass::all(); // @phpstan-ignore-line } /** @inheritDoc */ diff --git a/app/Repositories/SongRepository.php b/app/Repositories/SongRepository.php index e0a13baa..2186eff3 100644 --- a/app/Repositories/SongRepository.php +++ b/app/Repositories/SongRepository.php @@ -13,7 +13,7 @@ use App\Models\Song; use App\Models\User; use App\Values\Genre; use Illuminate\Contracts\Pagination\Paginator; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; /** @extends Repository */ class SongRepository extends Repository @@ -78,7 +78,10 @@ class SongRepository extends Repository return Song::query(type: PlayableType::SONG, user: $scopedUser) ->accessible() ->withMeta() - ->when($ownSongsOnly, static fn (SongBuilder $query) => $query->where('songs.owner_id', $scopedUser->id)) + ->when( + $ownSongsOnly, + static fn (SongBuilder $query) => $query->where('songs.owner_id', $scopedUser->id) + ) ->sort($sortColumns, $sortDirection) ->simplePaginate($perPage); } @@ -129,7 +132,7 @@ class SongRepository extends Repository return Song::query(user: $scopedUser ?? $this->auth->user()) ->accessible() ->withMeta() - ->where('album_id', $album->id) + ->whereBelongsTo($album) ->orderBy('songs.disc') ->orderBy('songs.track') ->orderBy('songs.title') @@ -195,7 +198,7 @@ class SongRepository extends Repository ->whereIn('songs.id', $ids) ->get(); - return $preserveOrder ? $songs->orderByArray($ids) : $songs; + return $preserveOrder ? $songs->orderByArray($ids) : $songs; // @phpstan-ignore-line } /** diff --git a/app/Rules/AllPlaylistsAreAccessibleBy.php b/app/Rules/AllPlaylistsAreAccessibleBy.php index 84eee23e..ad241000 100644 --- a/app/Rules/AllPlaylistsAreAccessibleBy.php +++ b/app/Rules/AllPlaylistsAreAccessibleBy.php @@ -22,7 +22,7 @@ final class AllPlaylistsAreAccessibleBy implements ValidationRule $accessiblePlaylists = $accessiblePlaylists->merge($this->user->collaboratedPlaylists); } - if (array_diff(Arr::wrap($value), $accessiblePlaylists->pluck('id')->toArray())) { + if (array_diff(Arr::wrap($value), $accessiblePlaylists->modelKeys())) { $fail( License::isPlus() ? 'Not all playlists are accessible by the user' diff --git a/app/Services/DownloadService.php b/app/Services/DownloadService.php index 3601d962..5a0e5d50 100644 --- a/app/Services/DownloadService.php +++ b/app/Services/DownloadService.php @@ -9,7 +9,7 @@ use App\Services\SongStorages\DropboxStorage; use App\Services\SongStorages\S3CompatibleStorage; use App\Services\SongStorages\SftpStorage; use App\Values\Podcast\EpisodePlayable; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Support\Facades\File; class DownloadService @@ -17,7 +17,7 @@ class DownloadService public function getDownloadablePath(Collection $songs): ?string { if ($songs->count() === 1) { - return $this->getLocalPath($songs->first()); + return $this->getLocalPath($songs->first()); // @phpstan-ignore-line } return (new SongZipArchive()) diff --git a/app/Services/InteractionService.php b/app/Services/InteractionService.php index fb0d90ea..4d04f1e9 100644 --- a/app/Services/InteractionService.php +++ b/app/Services/InteractionService.php @@ -8,7 +8,7 @@ use App\Events\SongLikeToggled; use App\Models\Interaction; use App\Models\Song as Playable; use App\Models\User; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; class InteractionService { @@ -57,14 +57,20 @@ class InteractionService public function likeMany(Collection $playables, User $user): Collection { $interactions = $playables->map(static function (Playable $playable) use ($user): Interaction { - return tap(Interaction::query()->firstOrCreate([ - 'song_id' => $playable->id, - 'user_id' => $user->id, - ]), static function (Interaction $interaction): void { - $interaction->play_count ??= 0; - $interaction->liked = true; - $interaction->save(); - }); + $interaction = Interaction::query()->whereBelongsTo($playable)->whereBelongsTo($user)->first(); + + if ($interaction) { + $interaction->update(['liked' => true]); + } else { + $interaction = Interaction::query()->create([ + 'song_id' => $playable->id, + 'user_id' => $user->id, + 'play_count' => 0, + 'liked' => true, + ]); + } + + return $interaction; }); event(new MultipleSongsLiked($playables, $user)); @@ -80,8 +86,8 @@ class InteractionService public function unlikeMany(Collection $playables, User $user): void { Interaction::query() - ->whereIn('song_id', $playables->pluck('id')->all()) - ->where('user_id', $user->id) + ->whereBelongsTo($playables) + ->whereBelongsTo($user) ->update(['liked' => false]); event(new MultipleSongsUnliked($playables, $user)); diff --git a/app/Services/LastfmService.php b/app/Services/LastfmService.php index d7d57e09..9a9b5a66 100644 --- a/app/Services/LastfmService.php +++ b/app/Services/LastfmService.php @@ -50,7 +50,7 @@ class LastfmService implements MusicEncyclopedia return rescue_if(static::enabled(), function () use ($artist): ?ArtistInformation { return Cache::remember( - "lastfm.artist.$artist->id", + "lastfm.artist.{$artist->id}", now()->addWeek(), fn () => $this->connector->send(new GetArtistInfoRequest($artist))->dto() ); @@ -65,7 +65,7 @@ class LastfmService implements MusicEncyclopedia return rescue_if(static::enabled(), function () use ($album): ?AlbumInformation { return Cache::remember( - "lastfm.album.$album->id", + "lastfm.album.{$album->id}", now()->addWeek(), fn () => $this->connector->send(new GetAlbumInfoRequest($album))->dto() ); diff --git a/app/Services/MediaInformationService.php b/app/Services/MediaInformationService.php index 8cdff3f8..0cb41cb1 100644 --- a/app/Services/MediaInformationService.php +++ b/app/Services/MediaInformationService.php @@ -23,16 +23,20 @@ class MediaInformationService return null; } - return Cache::remember("album.info.$album->id", now()->addWeek(), function () use ($album): AlbumInformation { - $info = $this->encyclopedia->getAlbumInformation($album) ?: AlbumInformation::make(); + return Cache::remember( + "album.info.{$album->id}", + now()->addWeek(), + function () use ($album): AlbumInformation { + $info = $this->encyclopedia->getAlbumInformation($album) ?: AlbumInformation::make(); - rescue_unless($album->has_cover, function () use ($info, $album): void { - $this->mediaMetadataService->tryDownloadAlbumCover($album); - $info->cover = $album->cover; - }); + rescue_unless($album->has_cover, function () use ($info, $album): void { + $this->mediaMetadataService->tryDownloadAlbumCover($album); + $info->cover = $album->cover; + }); - return $info; - }); + return $info; + } + ); } public function getArtistInformation(Artist $artist): ?ArtistInformation @@ -42,7 +46,7 @@ class MediaInformationService } return Cache::remember( - "artist.info.$artist->id", + "artist.info.{$artist->id}", now()->addWeek(), function () use ($artist): ArtistInformation { $info = $this->encyclopedia->getArtistInformation($artist) ?: ArtistInformation::make(); diff --git a/app/Services/PlaylistService.php b/app/Services/PlaylistService.php index eefe4378..e9712be9 100644 --- a/app/Services/PlaylistService.php +++ b/app/Services/PlaylistService.php @@ -11,6 +11,7 @@ use App\Models\Song as Playable; use App\Models\User; use App\Repositories\SongRepository; use App\Values\SmartPlaylistRuleGroupCollection; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use InvalidArgumentException; @@ -85,12 +86,12 @@ class PlaylistService return $playlist; } - /** @return Collection */ + /** @return EloquentCollection */ public function addPlayablesToPlaylist( Playlist $playlist, Collection|Playable|array $playables, User $user - ): Collection { + ): EloquentCollection { return DB::transaction(function () use ($playlist, $playables, $user) { $playables = Collection::wrap($playables); diff --git a/app/Services/QueueService.php b/app/Services/QueueService.php index d9cf2f1c..59cd0140 100644 --- a/app/Services/QueueService.php +++ b/app/Services/QueueService.php @@ -16,11 +16,12 @@ class QueueService public function getQueueState(User $user): QueueStateDTO { - $state = QueueState::query()->where('user_id', $user->id)->firstOrCreate([ - 'user_id' => $user->id, - ], [ - 'song_ids' => [], - ]); + $state = QueueState::query() + ->whereBelongsTo($user) + ->firstOrCreate( + ['user_id' => $user->id], + ['song_ids' => []], + ); $currentSong = $state->current_song_id ? $this->songRepository->findOne($state->current_song_id, $user) : null; @@ -33,11 +34,7 @@ class QueueService public function updateQueueState(User $user, array $songIds): void { - QueueState::query()->updateOrCreate([ - 'user_id' => $user->id, - ], [ - 'song_ids' => $songIds, - ]); + QueueState::query()->updateOrCreate(['user_id' => $user->id], ['song_ids' => $songIds]); } public function updatePlaybackStatus(User $user, Song $song, int $position): void diff --git a/app/Services/SearchService.php b/app/Services/SearchService.php index 80b1a43b..78127d8e 100644 --- a/app/Services/SearchService.php +++ b/app/Services/SearchService.php @@ -52,7 +52,7 @@ class SearchService { try { return $repository->getMany( - ids: $repository->modelClass::search($keywords)->get()->take($count)->pluck('id')->all(), // @phpstan-ignore-line + ids: $repository->modelClass::search($keywords)->get()->take($count)->modelKeys(), // @phpstan-ignore-line preserveOrder: true, ); } catch (Throwable $e) { diff --git a/app/Services/SmartPlaylistService.php b/app/Services/SmartPlaylistService.php index 5db01176..54e3226e 100644 --- a/app/Services/SmartPlaylistService.php +++ b/app/Services/SmartPlaylistService.php @@ -12,7 +12,7 @@ use App\Models\User; use App\Values\SmartPlaylistRule as Rule; use App\Values\SmartPlaylistRuleGroup as RuleGroup; use App\Values\SmartPlaylistSqlElements as SqlElements; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; class SmartPlaylistService { diff --git a/app/Services/SongService.php b/app/Services/SongService.php index 901ca514..72f0c76c 100644 --- a/app/Services/SongService.php +++ b/app/Services/SongService.php @@ -9,6 +9,7 @@ use App\Models\Song; use App\Repositories\SongRepository; use App\Services\SongStorages\SongStorage; use App\Values\SongUpdateData; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; @@ -97,34 +98,31 @@ class SongService return $this->songRepository->getOne($song->id); } - public function markSongsAsPublic(Collection $songs): void + public function markSongsAsPublic(EloquentCollection $songs): void { - Song::query()->whereIn('id', $songs->pluck('id'))->update(['is_public' => true]); + $songs->toQuery()->update(['is_public' => true]); } /** @return array IDs of songs that are marked as private */ - public function markSongsAsPrivate(Collection $songs): array + public function markSongsAsPrivate(EloquentCollection $songs): array { - if (License::isPlus()) { - // Songs that are in collaborative playlists can't be marked as private. - /** - * @var Collection $collaborativeSongs - */ - $collaborativeSongs = Song::query() - ->whereIn('songs.id', $songs->pluck('id')) - ->join('playlist_song', 'songs.id', '=', 'playlist_song.song_id') - ->join('playlist_collaborators', 'playlist_song.playlist_id', '=', 'playlist_collaborators.playlist_id') - ->select('songs.id') - ->distinct() - ->pluck('songs.id') - ->all(); + License::requirePlus(); - $applicableSongIds = $songs->whereNotIn('id', $collaborativeSongs)->pluck('id')->all(); - } else { - $applicableSongIds = $songs->pluck('id')->all(); - } + // Songs that are in collaborative playlists can't be marked as private. + /** + * @var Collection $collaborativeSongs + */ + $collaborativeSongs = $songs->toQuery() + ->join('playlist_song', 'songs.id', '=', 'playlist_song.song_id') + ->join('playlist_collaborators', 'playlist_song.playlist_id', '=', 'playlist_collaborators.playlist_id') + ->select('songs.id') + ->distinct() + ->pluck('songs.id') + ->all(); - Song::query()->whereIn('id', $applicableSongIds)->update(['is_public' => false]); + $applicableSongIds = $songs->whereNotIn('id', $collaborativeSongs)->modelKeys(); + + Song::query()->whereKey($applicableSongIds)->update(['is_public' => false]); return $applicableSongIds; } diff --git a/app/Values/LicenseInstance.php b/app/Values/LicenseInstance.php index e4410a0c..39d1ea6d 100644 --- a/app/Values/LicenseInstance.php +++ b/app/Values/LicenseInstance.php @@ -24,9 +24,9 @@ final class LicenseInstance implements Arrayable, Jsonable public static function fromJsonObject(object $json): self { return new self( - id: $json->id, - name: $json->name, - createdAt: Carbon::parse($json->created_at), + id: object_get($json, 'id'), + name: object_get($json, 'name'), + createdAt: Carbon::parse(object_get($json, 'created_at')), ); } diff --git a/app/Values/Podcast/EpisodePlayable.php b/app/Values/Podcast/EpisodePlayable.php index 905908f9..32a39a03 100644 --- a/app/Values/Podcast/EpisodePlayable.php +++ b/app/Values/Podcast/EpisodePlayable.php @@ -28,7 +28,7 @@ final class EpisodePlayable implements Arrayable, Jsonable public static function getForEpisode(Episode $episode): ?self { /** @var self|null $cached */ - $cached = Cache::get("episode-playable.$episode->id"); + $cached = Cache::get("episode-playable.{$episode->id}"); return $cached?->valid() ? $cached : self::createForEpisode($episode); } @@ -44,7 +44,7 @@ final class EpisodePlayable implements Arrayable, Jsonable } $playable = new self($file, md5_file($file)); - Cache::forever("episode-playable.$episode->id", $playable); + Cache::forever("episode-playable.{$episode->id}", $playable); return $playable; } diff --git a/database/factories/QueueStateFactory.php b/database/factories/QueueStateFactory.php index 46d01f97..3110566f 100644 --- a/database/factories/QueueStateFactory.php +++ b/database/factories/QueueStateFactory.php @@ -13,7 +13,7 @@ class QueueStateFactory extends Factory { return [ 'user_id' => User::factory(), - 'song_ids' => Song::factory()->count(3)->create()->pluck('id')->toArray(), + 'song_ids' => Song::factory()->count(3)->create()->modelKeys(), 'current_song_id' => null, 'playback_position' => 0, ]; diff --git a/phpstan.neon.dist b/phpstan.neon.dist index b9525845..90bfda64 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -26,10 +26,12 @@ parameters: # Laravel factories allow declaration of dynamic methods as "states" - '#Call to an undefined method Illuminate\\Database\\Eloquent\\Factories\\Factory::#' - '#expects App\\Models\\User\|null, Illuminate\\Database\\Eloquent\\Collection\|Illuminate\\Database\\Eloquent\\Model given#' + - '#expects App\\Models\\[a-zA-Z]*, Illuminate\\Database\\Eloquent\\Model\|null given#' - '#Method App\\Models\\.*::query\(\) should return App\\Builders\\.*Builder but returns Illuminate\\Database\\Eloquent\\Builder#' - '#Parameter \#1 \$callback of method Illuminate\\Support\\Collection::each\(\) expects callable\(Illuminate\\Database\\Eloquent\\Model, int\)#' - '#Access to an undefined property Illuminate\\Database\\Eloquent\\Model::#' - '#Unknown parameter \$(type|user) in call to static method App\\Models\\Song::query\(\)#' + - "#Called 'modelKeys' on Laravel collection, but could have been retrieved as a query#" excludePaths: diff --git a/tests/Feature/AlbumCoverTest.php b/tests/Feature/AlbumCoverTest.php index acc34d8d..38e655bb 100644 --- a/tests/Feature/AlbumCoverTest.php +++ b/tests/Feature/AlbumCoverTest.php @@ -33,7 +33,7 @@ class AlbumCoverTest extends TestCase ->once() ->with(Mockery::on(static fn (Album $target) => $target->is($album)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/album/$album->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_admin()) + $this->putAs("api/album/{$album->id}/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_admin()) ->assertOk(); } @@ -44,7 +44,7 @@ class AlbumCoverTest extends TestCase $this->mediaMetadataService->shouldNotReceive('writeAlbumCover'); - $this->putAs('api/album/' . $album->id . '/cover', ['cover' => 'data:image/jpeg;base64,Rm9v'], create_user()) + $this->putAs("api/album/{$album->id}/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_user()) ->assertForbidden(); } } diff --git a/tests/Feature/AlbumInformationTest.php b/tests/Feature/AlbumInformationTest.php index ae6fba92..8664f042 100644 --- a/tests/Feature/AlbumInformationTest.php +++ b/tests/Feature/AlbumInformationTest.php @@ -43,7 +43,7 @@ class AlbumInformationTest extends TestCase ] )); - $this->getAs('api/albums/' . $album->id . '/information') + $this->getAs("api/albums/{$album->id}/information") ->assertJsonStructure(AlbumInformation::JSON_STRUCTURE); } diff --git a/tests/Feature/AlbumSongTest.php b/tests/Feature/AlbumSongTest.php index 4a085251..bebdca4b 100644 --- a/tests/Feature/AlbumSongTest.php +++ b/tests/Feature/AlbumSongTest.php @@ -14,10 +14,9 @@ class AlbumSongTest extends TestCase public function index(): void { $album = Album::factory()->create(); - Song::factory(5)->for($album)->create(); - $this->getAs('api/albums/' . $album->id . '/songs') + $this->getAs("api/albums/{$album->id}/songs") ->assertJsonStructure(['*' => SongResource::JSON_STRUCTURE]); } } diff --git a/tests/Feature/ArtistAlbumTest.php b/tests/Feature/ArtistAlbumTest.php index 9ff1458e..8cd5f439 100644 --- a/tests/Feature/ArtistAlbumTest.php +++ b/tests/Feature/ArtistAlbumTest.php @@ -14,10 +14,9 @@ class ArtistAlbumTest extends TestCase public function index(): void { $artist = Artist::factory()->create(); - Album::factory(5)->for($artist)->create(); - $this->getAs('api/artists/' . $artist->id . '/albums') + $this->getAs("api/artists/{$artist->id}/albums") ->assertJsonStructure(['*' => AlbumResource::JSON_STRUCTURE]); } } diff --git a/tests/Feature/ArtistImageTest.php b/tests/Feature/ArtistImageTest.php index c9a8e3e0..cf5e61a9 100644 --- a/tests/Feature/ArtistImageTest.php +++ b/tests/Feature/ArtistImageTest.php @@ -32,18 +32,18 @@ class ArtistImageTest extends TestCase ->once() ->with(Mockery::on(static fn (Artist $target) => $target->is($artist)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/artist/$artist->id/image", ['image' => 'data:image/jpeg;base64,Rm9v'], create_admin()) + $this->putAs("api/artist/{$artist->id}/image", ['image' => 'data:image/jpeg;base64,Rm9v'], create_admin()) ->assertOk(); } #[Test] public function updateNotAllowedForNormalUsers(): void { - Artist::factory()->create(['id' => 9999]); + $artist = Artist::factory()->create(); $this->mediaMetadataService->shouldNotReceive('writeArtistImage'); - $this->putAs('api/artist/9999/image', ['image' => 'data:image/jpeg;base64,Rm9v']) + $this->putAs("api/artist/{$artist->id}/image", ['image' => 'data:image/jpeg;base64,Rm9v']) ->assertForbidden(); } } diff --git a/tests/Feature/ArtistInformationTest.php b/tests/Feature/ArtistInformationTest.php index 3b658ef9..483c9e70 100644 --- a/tests/Feature/ArtistInformationTest.php +++ b/tests/Feature/ArtistInformationTest.php @@ -31,7 +31,7 @@ class ArtistInformationTest extends TestCase ], )); - $this->getAs('api/artists/' . $artist->id . '/information') + $this->getAs("api/artists/{$artist->id}/information") ->assertJsonStructure(ArtistInformation::JSON_STRUCTURE); } diff --git a/tests/Feature/ArtistSongTest.php b/tests/Feature/ArtistSongTest.php index 42146051..48cc80a4 100644 --- a/tests/Feature/ArtistSongTest.php +++ b/tests/Feature/ArtistSongTest.php @@ -17,7 +17,7 @@ class ArtistSongTest extends TestCase Song::factory(5)->for($artist)->create(); - $this->getAs('api/artists/' . $artist->id . '/songs') + $this->getAs("api/artists/{$artist->id}/songs") ->assertJsonStructure(['*' => SongResource::JSON_STRUCTURE]); } } diff --git a/tests/Feature/DownloadTest.php b/tests/Feature/DownloadTest.php index aaac8c13..05e718e7 100644 --- a/tests/Feature/DownloadTest.php +++ b/tests/Feature/DownloadTest.php @@ -8,7 +8,7 @@ use App\Models\Interaction; use App\Models\Playlist; use App\Models\Song; use App\Services\DownloadService; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; use Mockery; use Mockery\MockInterface; use PHPUnit\Framework\Attributes\Test; @@ -47,7 +47,7 @@ class DownloadTest extends TestCase ->shouldReceive('getDownloadablePath') ->once() ->with(Mockery::on(static function (Collection $retrievedSongs) use ($song) { - return $retrievedSongs->count() === 1 && $retrievedSongs->first()->id === $song->id; + return $retrievedSongs->count() === 1 && $retrievedSongs->first()->is($song); })) ->andReturn(test_path('songs/blank.mp3')); @@ -65,7 +65,7 @@ class DownloadTest extends TestCase ->shouldReceive('getDownloadablePath') ->once() ->with(Mockery::on(static function (Collection $retrievedSongs) use ($songs): bool { - self::assertEqualsCanonicalizing($retrievedSongs->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($retrievedSongs->modelKeys(), $songs->modelKeys()); return true; })) @@ -89,7 +89,7 @@ class DownloadTest extends TestCase ->shouldReceive('getDownloadablePath') ->once() ->with(Mockery::on(static function (Collection $retrievedSongs) use ($songs): bool { - self::assertEqualsCanonicalizing($retrievedSongs->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($retrievedSongs->modelKeys(), $songs->modelKeys()); return true; })) @@ -110,7 +110,7 @@ class DownloadTest extends TestCase ->shouldReceive('getDownloadablePath') ->once() ->with(Mockery::on(static function (Collection $retrievedSongs) use ($songs): bool { - self::assertEqualsCanonicalizing($retrievedSongs->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($retrievedSongs->modelKeys(), $songs->modelKeys()); return true; })) @@ -133,7 +133,7 @@ class DownloadTest extends TestCase $this->downloadService ->shouldReceive('getDownloadablePath') ->with(Mockery::on(static function (Collection $retrievedSongs) use ($songs): bool { - self::assertEqualsCanonicalizing($retrievedSongs->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($retrievedSongs->modelKeys(), $songs->modelKeys()); return true; })) @@ -162,7 +162,7 @@ class DownloadTest extends TestCase $this->downloadService ->shouldReceive('getDownloadablePath') ->with(Mockery::on(static function (Collection $songs) use ($favorites): bool { - self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $favorites->pluck('song_id')->all()); + self::assertEqualsCanonicalizing($songs->modelKeys(), $favorites->pluck('song_id')->all()); return true; })) diff --git a/tests/Feature/InteractionTest.php b/tests/Feature/InteractionTest.php index 1fe2206a..75e3844d 100644 --- a/tests/Feature/InteractionTest.php +++ b/tests/Feature/InteractionTest.php @@ -76,7 +76,7 @@ class InteractionTest extends TestCase $user = create_user(); $songs = Song::factory(2)->create(); - $songIds = $songs->pluck('id')->all(); + $songIds = $songs->modelKeys(); $this->postAs('api/interaction/batch/like', ['songs' => $songIds], $user); diff --git a/tests/Feature/KoelPlus/AlbumCoverTest.php b/tests/Feature/KoelPlus/AlbumCoverTest.php index 99274ad4..a486cd71 100644 --- a/tests/Feature/KoelPlus/AlbumCoverTest.php +++ b/tests/Feature/KoelPlus/AlbumCoverTest.php @@ -38,7 +38,7 @@ class AlbumCoverTest extends PlusTestCase ->once() ->with(Mockery::on(static fn (Album $target) => $target->is($album)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/albums/$album->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $user) + $this->putAs("api/albums/{$album->id}/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $user) ->assertOk(); } @@ -56,7 +56,7 @@ class AlbumCoverTest extends PlusTestCase ->shouldReceive('writeAlbumCover') ->never(); - $this->putAs("api/albums/$album->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $user) + $this->putAs("api/albums/{$album->id}/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $user) ->assertForbidden(); } @@ -74,7 +74,7 @@ class AlbumCoverTest extends PlusTestCase ->once() ->with(Mockery::on(static fn (Album $target) => $target->is($album)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/albums/$album->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_admin()) + $this->putAs("api/albums/{$album->id}/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_admin()) ->assertOk(); } } diff --git a/tests/Feature/KoelPlus/ArtistImageTest.php b/tests/Feature/KoelPlus/ArtistImageTest.php index 1ddcd450..b754240f 100644 --- a/tests/Feature/KoelPlus/ArtistImageTest.php +++ b/tests/Feature/KoelPlus/ArtistImageTest.php @@ -38,7 +38,7 @@ class ArtistImageTest extends PlusTestCase ->once() ->with(Mockery::on(static fn (Artist $target) => $target->is($artist)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/artists/$artist->id/image", ['image' => 'data:image/jpeg;base64,Rm9v'], $user) + $this->putAs("api/artists/{$artist->id}/image", ['image' => 'data:image/jpeg;base64,Rm9v'], $user) ->assertOk(); } @@ -56,7 +56,7 @@ class ArtistImageTest extends PlusTestCase ->shouldReceive('writeArtistImage') ->never(); - $this->putAs("api/artists/$artist->id/image", ['image' => 'data:image/jpeg;base64,Rm9v'], $user) + $this->putAs("api/artists/{$artist->id}/image", ['image' => 'data:image/jpeg;base64,Rm9v'], $user) ->assertForbidden(); } @@ -74,7 +74,11 @@ class ArtistImageTest extends PlusTestCase ->once() ->with(Mockery::on(static fn (Artist $target) => $target->is($artist)), 'data:image/jpeg;base64,Rm9v'); - $this->putAs("api/artists/$artist->id/image", ['image' => 'data:image/jpeg;base64,Rm9v'], create_admin()) + $this->putAs( + "api/artists/{$artist->id}/image", + ['image' => 'data:image/jpeg;base64,Rm9v'], + create_admin() + ) ->assertOk(); } } diff --git a/tests/Feature/KoelPlus/DownloadTest.php b/tests/Feature/KoelPlus/DownloadTest.php index cced694d..88b665f8 100644 --- a/tests/Feature/KoelPlus/DownloadTest.php +++ b/tests/Feature/KoelPlus/DownloadTest.php @@ -21,7 +21,7 @@ class DownloadTest extends PlusTestCase // Can't download a private song that doesn't belong to the user /** @var Song $externalPrivateSong */ $externalPrivateSong = Song::factory()->private()->create(); - $this->get("download/songs?songs[]=$externalPrivateSong->id&api_token=" . $apiToken) + $this->get("download/songs?songs[]={$externalPrivateSong->id}&api_token=" . $apiToken) ->assertForbidden(); // Can download a public song that doesn't belong to the user @@ -33,7 +33,7 @@ class DownloadTest extends PlusTestCase ->once() ->andReturn(test_path('songs/blank.mp3')); - $this->get("download/songs?songs[]=$externalPublicSong->id&api_token=" . $apiToken) + $this->get("download/songs?songs[]={$externalPublicSong->id}&api_token=" . $apiToken) ->assertOk(); // Can download a private song that belongs to the user @@ -42,7 +42,7 @@ class DownloadTest extends PlusTestCase $downloadService->shouldReceive('getDownloadablePath') ->once() ->andReturn(test_path('songs/blank.mp3')); - $this->get("download/songs?songs[]=$ownSong->id&api_token=" . $apiToken) + $this->get("download/songs?songs[]={$ownSong->id}&api_token=" . $apiToken) ->assertOk(); } } diff --git a/tests/Feature/KoelPlus/InteractionTest.php b/tests/Feature/KoelPlus/InteractionTest.php index d6aae86b..4ade4817 100644 --- a/tests/Feature/KoelPlus/InteractionTest.php +++ b/tests/Feature/KoelPlus/InteractionTest.php @@ -6,7 +6,6 @@ use App\Events\MultipleSongsLiked; use App\Events\MultipleSongsUnliked; use App\Events\SongLikeToggled; use App\Models\Song; -use Illuminate\Support\Collection; use Illuminate\Support\Facades\Event; use PHPUnit\Framework\Attributes\Test; use Tests\PlusTestCase; @@ -23,7 +22,6 @@ class InteractionTest extends PlusTestCase $owner = create_user(); // Can't increase play count of a private song that doesn't belong to the user - /** @var Song $externalPrivateSong */ $externalPrivateSong = Song::factory()->private()->create(); $this->postAs('api/interaction/play', ['song' => $externalPrivateSong->id], $owner) ->assertForbidden(); @@ -75,26 +73,23 @@ class InteractionTest extends PlusTestCase $owner = create_user(); // Can't batch like private songs that don't belong to the user - /** @var Collection $externalPrivateSongs */ $externalPrivateSongs = Song::factory()->count(3)->private()->create(); - $this->postAs('api/interaction/batch/like', ['songs' => $externalPrivateSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/like', ['songs' => $externalPrivateSongs->modelKeys()], $owner) ->assertForbidden(); // Can batch like public songs that don't belong to the user - /** @var Collection $externalPublicSongs */ $externalPublicSongs = Song::factory()->count(3)->public()->create(); - $this->postAs('api/interaction/batch/like', ['songs' => $externalPublicSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/like', ['songs' => $externalPublicSongs->modelKeys()], $owner) ->assertSuccessful(); // Can batch like private songs that belong to the user - /** @var Collection $ownPrivateSongs */ $ownPrivateSongs = Song::factory()->count(3)->private()->for($owner, 'owner')->create(); - $this->postAs('api/interaction/batch/like', ['songs' => $ownPrivateSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/like', ['songs' => $ownPrivateSongs->modelKeys()], $owner) ->assertSuccessful(); // Can't batch like a mix of inaccessible and accessible songs $mixedSongs = $externalPrivateSongs->merge($externalPublicSongs); - $this->postAs('api/interaction/batch/like', ['songs' => $mixedSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/like', ['songs' => $mixedSongs->modelKeys()], $owner) ->assertForbidden(); } @@ -106,26 +101,23 @@ class InteractionTest extends PlusTestCase $owner = create_user(); // Can't batch unlike private songs that don't belong to the user - /** @var Collection $externalPrivateSongs */ $externalPrivateSongs = Song::factory()->count(3)->private()->create(); - $this->postAs('api/interaction/batch/unlike', ['songs' => $externalPrivateSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/unlike', ['songs' => $externalPrivateSongs->modelKeys()], $owner) ->assertForbidden(); // Can batch unlike public songs that don't belong to the user - /** @var Collection $externalPublicSongs */ $externalPublicSongs = Song::factory()->count(3)->public()->create(); - $this->postAs('api/interaction/batch/unlike', ['songs' => $externalPublicSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/unlike', ['songs' => $externalPublicSongs->modelKeys()], $owner) ->assertSuccessful(); // Can batch unlike private songs that belong to the user - /** @var Collection $ownPrivateSongs */ $ownPrivateSongs = Song::factory()->count(3)->private()->for($owner, 'owner')->create(); - $this->postAs('api/interaction/batch/unlike', ['songs' => $ownPrivateSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/unlike', ['songs' => $ownPrivateSongs->modelKeys()], $owner) ->assertSuccessful(); // Can't batch unlike a mix of inaccessible and accessible songs $mixedSongs = $externalPrivateSongs->merge($externalPublicSongs); - $this->postAs('api/interaction/batch/unlike', ['songs' => $mixedSongs->pluck('id')->all()], $owner) + $this->postAs('api/interaction/batch/unlike', ['songs' => $mixedSongs->modelKeys()], $owner) ->assertForbidden(); } } diff --git a/tests/Feature/KoelPlus/PlaylistCollaborationTest.php b/tests/Feature/KoelPlus/PlaylistCollaborationTest.php index 8eaa86d7..fe6c8827 100644 --- a/tests/Feature/KoelPlus/PlaylistCollaborationTest.php +++ b/tests/Feature/KoelPlus/PlaylistCollaborationTest.php @@ -19,7 +19,7 @@ class PlaylistCollaborationTest extends PlusTestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(); - $this->postAs("api/playlists/$playlist->id/collaborators/invite", [], $playlist->user) + $this->postAs("api/playlists/{$playlist->id}/collaborators/invite", [], $playlist->user) ->assertJsonStructure(PlaylistCollaborationTokenResource::JSON_STRUCTURE); } diff --git a/tests/Feature/KoelPlus/PlaylistCoverTest.php b/tests/Feature/KoelPlus/PlaylistCoverTest.php index d0a7c8b0..ad5b3e68 100644 --- a/tests/Feature/KoelPlus/PlaylistCoverTest.php +++ b/tests/Feature/KoelPlus/PlaylistCoverTest.php @@ -18,7 +18,11 @@ class PlaylistCoverTest extends PlusTestCase $collaborator = create_user(); $playlist->addCollaborator($collaborator); - $this->putAs("api/playlists/$playlist->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $collaborator) + $this->putAs( + "api/playlists/{$playlist->id}/cover", + ['cover' => 'data:image/jpeg;base64,Rm9v'], + $collaborator + ) ->assertForbidden(); } @@ -30,7 +34,7 @@ class PlaylistCoverTest extends PlusTestCase $collaborator = create_user(); $playlist->addCollaborator($collaborator); - $this->deleteAs("api/playlists/$playlist->id/cover", [], $collaborator) + $this->deleteAs("api/playlists/{$playlist->id}/cover", [], $collaborator) ->assertForbidden(); } } diff --git a/tests/Feature/KoelPlus/PlaylistFolderTest.php b/tests/Feature/KoelPlus/PlaylistFolderTest.php index 07020ff4..4d685593 100644 --- a/tests/Feature/KoelPlus/PlaylistFolderTest.php +++ b/tests/Feature/KoelPlus/PlaylistFolderTest.php @@ -22,7 +22,7 @@ class PlaylistFolderTest extends PlusTestCase /** @var PlaylistFolder $ownerFolder */ $ownerFolder = PlaylistFolder::factory()->for($playlist->user)->create(); - $ownerFolder->playlists()->attach($playlist->id); + $ownerFolder->playlists()->attach($playlist); self::assertTrue($playlist->refresh()->getFolder($playlist->user)?->is($ownerFolder)); /** @var PlaylistFolder $collaboratorFolder */ @@ -30,7 +30,7 @@ class PlaylistFolderTest extends PlusTestCase self::assertNull($playlist->getFolder($collaborator)); $this->postAs( - "api/playlist-folders/$collaboratorFolder->id/playlists", + "api/playlist-folders/{$collaboratorFolder->id}/playlists", ['playlists' => [$playlist->id]], $collaborator ) @@ -54,17 +54,17 @@ class PlaylistFolderTest extends PlusTestCase /** @var PlaylistFolder $ownerFolder */ $ownerFolder = PlaylistFolder::factory()->for($playlist->user)->create(); - $ownerFolder->playlists()->attach($playlist->id); + $ownerFolder->playlists()->attach($playlist); self::assertTrue($playlist->refresh()->getFolder($playlist->user)?->is($ownerFolder)); /** @var PlaylistFolder $collaboratorFolder */ $collaboratorFolder = PlaylistFolder::factory()->for($collaborator)->create(); - $collaboratorFolder->playlists()->attach($playlist->id); + $collaboratorFolder->playlists()->attach($playlist); self::assertTrue($playlist->refresh()->getFolder($collaborator)?->is($collaboratorFolder)); $this->deleteAs( - "api/playlist-folders/$collaboratorFolder->id/playlists", + "api/playlist-folders/{$collaboratorFolder->id}/playlists", ['playlists' => [$playlist->id]], $collaborator ) diff --git a/tests/Feature/KoelPlus/PlaylistSongTest.php b/tests/Feature/KoelPlus/PlaylistSongTest.php index a055f539..ef2286f2 100644 --- a/tests/Feature/KoelPlus/PlaylistSongTest.php +++ b/tests/Feature/KoelPlus/PlaylistSongTest.php @@ -22,7 +22,7 @@ class PlaylistSongTest extends PlusTestCase $collaborator = create_user(); $playlist->addCollaborator($collaborator); - $this->getAs("api/playlists/$playlist->id/songs", $collaborator) + $this->getAs("api/playlists/{$playlist->id}/songs", $collaborator) ->assertSuccessful() ->assertJsonStructure(['*' => CollaborativeSongResource::JSON_STRUCTURE]) ->assertJsonCount(3); @@ -42,7 +42,7 @@ class PlaylistSongTest extends PlusTestCase $collaborator = create_user(); $playlist->addCollaborator($collaborator); - $this->getAs("api/playlists/$playlist->id/songs", $collaborator) + $this->getAs("api/playlists/{$playlist->id}/songs", $collaborator) ->assertSuccessful() ->assertJsonStructure(['*' => CollaborativeSongResource::JSON_STRUCTURE]) ->assertJsonCount(3) @@ -58,7 +58,7 @@ class PlaylistSongTest extends PlusTestCase $playlist->addCollaborator($collaborator); $songs = Song::factory()->for($collaborator, 'owner')->count(3)->create(); - $this->postAs("api/playlists/$playlist->id/songs", ['songs' => $songs->pluck('id')->all()], $collaborator) + $this->postAs("api/playlists/{$playlist->id}/songs", ['songs' => $songs->modelKeys()], $collaborator) ->assertSuccessful(); $playlist->refresh(); @@ -75,7 +75,7 @@ class PlaylistSongTest extends PlusTestCase $songs = Song::factory()->for($collaborator, 'owner')->count(3)->create(); $playlist->addPlayables($songs); - $this->deleteAs("api/playlists/$playlist->id/songs", ['songs' => $songs->pluck('id')->all()], $collaborator) + $this->deleteAs("api/playlists/{$playlist->id}/songs", ['songs' => $songs->modelKeys()], $collaborator) ->assertSuccessful(); $playlist->refresh(); diff --git a/tests/Feature/KoelPlus/PlaylistTest.php b/tests/Feature/KoelPlus/PlaylistTest.php index df4a7fbf..05a38665 100644 --- a/tests/Feature/KoelPlus/PlaylistTest.php +++ b/tests/Feature/KoelPlus/PlaylistTest.php @@ -52,7 +52,7 @@ class PlaylistTest extends PlusTestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->smart()->create(); - $this->putAs("api/playlists/$playlist->id", [ + $this->putAs("api/playlists/{$playlist->id}", [ 'name' => 'Foo', 'own_songs_only' => true, 'rules' => $playlist->rules->toArray(), @@ -72,7 +72,7 @@ class PlaylistTest extends PlusTestCase $collaborator = create_user(); $playlist->addCollaborator($collaborator); - $this->putAs("api/playlists/$playlist->id", ['name' => 'Nope'], $collaborator) + $this->putAs("api/playlists/{$playlist->id}", ['name' => 'Nope'], $collaborator) ->assertForbidden(); } } diff --git a/tests/Feature/KoelPlus/SongPlayTest.php b/tests/Feature/KoelPlus/SongPlayTest.php index 8ba65b25..1d61976c 100644 --- a/tests/Feature/KoelPlus/SongPlayTest.php +++ b/tests/Feature/KoelPlus/SongPlayTest.php @@ -29,7 +29,7 @@ class SongPlayTest extends PlusTestCase ->shouldReceive('stream') ->once(); - $this->get("play/$song->id?t=$token->audioToken") + $this->get("play/{$song->id}?t=$token->audioToken") ->assertOk(); } @@ -48,7 +48,7 @@ class SongPlayTest extends PlusTestCase ->shouldReceive('stream') ->once(); - $this->get("play/$song->id?t=$token->audioToken") + $this->get("play/{$song->id}?t=$token->audioToken") ->assertOk(); } @@ -63,7 +63,7 @@ class SongPlayTest extends PlusTestCase /** @var CompositeToken $token */ $token = app(TokenManager::class)->createCompositeToken(create_user()); - $this->get("play/$song->id?t=$token->audioToken") + $this->get("play/{$song->id}?t=$token->audioToken") ->assertForbidden(); } } diff --git a/tests/Feature/KoelPlus/SongTest.php b/tests/Feature/KoelPlus/SongTest.php index f9da89dd..ea834787 100644 --- a/tests/Feature/KoelPlus/SongTest.php +++ b/tests/Feature/KoelPlus/SongTest.php @@ -3,7 +3,6 @@ namespace Tests\Feature\KoelPlus; use App\Models\Song; -use Illuminate\Support\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\PlusTestCase; @@ -18,7 +17,6 @@ class SongTest extends PlusTestCase Song::factory(2)->public()->create(); - /** @var Collection $ownSongs */ $ownSongs = Song::factory(3)->for($user, 'owner')->create(); $this->getAs('api/songs?own_songs_only=true', $user) @@ -55,19 +53,19 @@ class SongTest extends PlusTestCase $publicSong = Song::factory()->public()->create(); // We can access public songs. - $this->getAs("api/songs/$publicSong->id", $user)->assertSuccessful(); + $this->getAs("api/songs/{$publicSong->id}", $user)->assertSuccessful(); /** @var Song $ownPrivateSong */ $ownPrivateSong = Song::factory()->for($user, 'owner')->private()->create(); // We can access our own private songs. - $this->getAs('api/songs/' . $ownPrivateSong->id, $user)->assertSuccessful(); + $this->getAs("api/songs/{$ownPrivateSong->id}", $user)->assertSuccessful(); /** @var Song $externalUnownedSong */ $externalUnownedSong = Song::factory()->private()->create(); // But we can't access private songs that are not ours. - $this->getAs("api/songs/$externalUnownedSong->id", $user)->assertForbidden(); + $this->getAs("api/songs/{$externalUnownedSong->id}", $user)->assertForbidden(); } #[Test] @@ -76,12 +74,11 @@ class SongTest extends PlusTestCase $currentUser = create_user(); $anotherUser = create_user(); - /** @var Collection $externalUnownedSongs */ $externalUnownedSongs = Song::factory(3)->for($anotherUser, 'owner')->private()->create(); // We can't edit songs that are not ours. $this->putAs('api/songs', [ - 'songs' => $externalUnownedSongs->pluck('id')->toArray(), + 'songs' => $externalUnownedSongs->modelKeys(), 'data' => [ 'title' => 'New Title', ], @@ -91,7 +88,7 @@ class SongTest extends PlusTestCase $mixedSongs = $externalUnownedSongs->merge(Song::factory(2)->for($currentUser, 'owner')->create()); $this->putAs('api/songs', [ - 'songs' => $mixedSongs->pluck('id')->toArray(), + 'songs' => $mixedSongs->modelKeys(), 'data' => [ 'title' => 'New Title', ], @@ -101,7 +98,7 @@ class SongTest extends PlusTestCase $ownSongs = Song::factory(3)->for($currentUser, 'owner')->create(); $this->putAs('api/songs', [ - 'songs' => $ownSongs->pluck('id')->toArray(), + 'songs' => $ownSongs->modelKeys(), 'data' => [ 'title' => 'New Title', ], @@ -114,35 +111,33 @@ class SongTest extends PlusTestCase $currentUser = create_user(); $anotherUser = create_user(); - /** @var Collection $externalUnownedSongs */ $externalUnownedSongs = Song::factory(3)->for($anotherUser, 'owner')->private()->create(); // We can't delete songs that are not ours. - $this->deleteAs('api/songs', ['songs' => $externalUnownedSongs->pluck('id')->toArray()], $currentUser) + $this->deleteAs('api/songs', ['songs' => $externalUnownedSongs->modelKeys()], $currentUser) ->assertForbidden(); // Even if some of the songs are owned by us, we still can't delete them. $mixedSongs = $externalUnownedSongs->merge(Song::factory(2)->for($currentUser, 'owner')->create()); - $this->deleteAs('api/songs', ['songs' => $mixedSongs->pluck('id')->toArray()], $currentUser) + $this->deleteAs('api/songs', ['songs' => $mixedSongs->modelKeys()], $currentUser) ->assertForbidden(); // But we can delete our own songs. $ownSongs = Song::factory(3)->for($currentUser, 'owner')->create(); - $this->deleteAs('api/songs', ['songs' => $ownSongs->pluck('id')->toArray()], $currentUser) + $this->deleteAs('api/songs', ['songs' => $ownSongs->modelKeys()], $currentUser) ->assertSuccessful(); } #[Test] - public function publicizeSongs(): void + public function markSongsAsPublic(): void { $user = create_user(); - /** @var Song $songs */ $songs = Song::factory(3)->for($user, 'owner')->private()->create(); - $this->putAs('api/songs/publicize', ['songs' => $songs->pluck('id')->toArray()], $user) + $this->putAs('api/songs/publicize', ['songs' => $songs->modelKeys()], $user) ->assertSuccessful(); $songs->each(static function (Song $song): void { @@ -152,14 +147,13 @@ class SongTest extends PlusTestCase } #[Test] - public function privatizeSongs(): void + public function markSongsAsPrivate(): void { $user = create_user(); - /** @var Song $songs */ $songs = Song::factory(3)->for($user, 'owner')->public()->create(); - $this->putAs('api/songs/privatize', ['songs' => $songs->pluck('id')->toArray()], $user) + $this->putAs('api/songs/privatize', ['songs' => $songs->modelKeys()], $user) ->assertSuccessful(); $songs->each(static function (Song $song): void { @@ -173,12 +167,12 @@ class SongTest extends PlusTestCase { $songs = Song::factory(3)->public()->create(); - $this->putAs('api/songs/privatize', ['songs' => $songs->pluck('id')->toArray()]) + $this->putAs('api/songs/privatize', ['songs' => $songs->modelKeys()]) ->assertForbidden(); $otherSongs = Song::factory(3)->private()->create(); - $this->putAs('api/songs/publicize', ['songs' => $otherSongs->pluck('id')->toArray()]) + $this->putAs('api/songs/publicize', ['songs' => $otherSongs->modelKeys()]) ->assertForbidden(); } } diff --git a/tests/Feature/KoelPlus/SongVisibilityTest.php b/tests/Feature/KoelPlus/SongVisibilityTest.php index b319d438..eb7e595b 100644 --- a/tests/Feature/KoelPlus/SongVisibilityTest.php +++ b/tests/Feature/KoelPlus/SongVisibilityTest.php @@ -3,7 +3,6 @@ namespace Tests\Feature\KoelPlus; use App\Models\Song; -use Illuminate\Support\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\PlusTestCase; @@ -17,17 +16,16 @@ class SongVisibilityTest extends PlusTestCase $currentUser = create_user(); $anotherUser = create_user(); - /** @var Collection $externalSongs */ $externalSongs = Song::factory(3)->for($anotherUser, 'owner')->private()->create(); // We can't make public songs that are not ours. - $this->putAs('api/songs/publicize', ['songs' => $externalSongs->pluck('id')->toArray()], $currentUser) + $this->putAs('api/songs/publicize', ['songs' => $externalSongs->modelKeys()], $currentUser) ->assertForbidden(); // But we can our own songs. $ownSongs = Song::factory(3)->for($currentUser, 'owner')->create(); - $this->putAs('api/songs/publicize', ['songs' => $ownSongs->pluck('id')->toArray()], $currentUser) + $this->putAs('api/songs/publicize', ['songs' => $ownSongs->modelKeys()], $currentUser) ->assertSuccessful(); $ownSongs->each(static fn (Song $song) => self::assertTrue($song->refresh()->is_public)); @@ -39,17 +37,16 @@ class SongVisibilityTest extends PlusTestCase $currentUser = create_user(); $anotherUser = create_user(); - /** @var Collection $externalSongs */ $externalSongs = Song::factory(3)->for($anotherUser, 'owner')->public()->create(); // We can't Mark as Private songs that are not ours. - $this->putAs('api/songs/privatize', ['songs' => $externalSongs->pluck('id')->toArray()], $currentUser) + $this->putAs('api/songs/privatize', ['songs' => $externalSongs->modelKeys()], $currentUser) ->assertForbidden(); // But we can our own songs. $ownSongs = Song::factory(3)->for($currentUser, 'owner')->create(); - $this->putAs('api/songs/privatize', ['songs' => $ownSongs->pluck('id')->toArray()], $currentUser) + $this->putAs('api/songs/privatize', ['songs' => $ownSongs->modelKeys()], $currentUser) ->assertSuccessful(); $ownSongs->each(static fn (Song $song) => self::assertFalse($song->refresh()->is_public)); diff --git a/tests/Feature/PlayCountTest.php b/tests/Feature/PlayCountTest.php index fa8dc6f2..f1b5c55c 100644 --- a/tests/Feature/PlayCountTest.php +++ b/tests/Feature/PlayCountTest.php @@ -22,7 +22,7 @@ class PlayCountTest extends TestCase 'play_count' => 10, ]); - $this->postAs('/api/interaction/play', ['song' => $interaction->song->id], $interaction->user) + $this->postAs('/api/interaction/play', ['song' => $interaction->song_id], $interaction->user) ->assertJsonStructure([ 'type', 'id', @@ -53,8 +53,8 @@ class PlayCountTest extends TestCase ]); $interaction = Interaction::query() - ->where('song_id', $song->id) - ->where('user_id', $user->id) + ->whereBelongsTo($song) + ->whereBelongsTo($user) ->first(); self::assertSame(1, $interaction->play_count); diff --git a/tests/Feature/PlaylistCoverTest.php b/tests/Feature/PlaylistCoverTest.php index c72a09a4..aa41ece8 100644 --- a/tests/Feature/PlaylistCoverTest.php +++ b/tests/Feature/PlaylistCoverTest.php @@ -19,7 +19,7 @@ class PlaylistCoverTest extends TestCase self::assertNull($playlist->cover); $this->putAs( - "api/playlists/$playlist->id/cover", + "api/playlists/{$playlist->id}/cover", ['cover' => read_as_data_url(test_path('blobs/cover.png'))], $playlist->user ) @@ -33,7 +33,11 @@ class PlaylistCoverTest extends TestCase { $playlist = Playlist::factory()->create(); - $this->putAs("api/playlists/$playlist->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_user()) + $this->putAs( + "api/playlists/{$playlist->id}/cover", + ['cover' => 'data:image/jpeg;base64,Rm9v'], + create_user() + ) ->assertForbidden(); } @@ -42,7 +46,7 @@ class PlaylistCoverTest extends TestCase { $playlist = Playlist::factory()->create(['cover' => 'cover.jpg']); - $this->deleteAs("api/playlists/$playlist->id/cover", [], $playlist->user) + $this->deleteAs("api/playlists/{$playlist->id}/cover", [], $playlist->user) ->assertNoContent(); self::assertNull($playlist->refresh()->cover); @@ -53,7 +57,7 @@ class PlaylistCoverTest extends TestCase { $playlist = Playlist::factory()->create(['cover' => 'cover.jpg']); - $this->deleteAs("api/playlists/$playlist->id/cover", [], create_user()) + $this->deleteAs("api/playlists/{$playlist->id}/cover", [], create_user()) ->assertForbidden(); self::assertSame('cover.jpg', $playlist->refresh()->getRawOriginal('cover')); diff --git a/tests/Feature/PlaylistFolderTest.php b/tests/Feature/PlaylistFolderTest.php index 7c9172ca..10183aa0 100644 --- a/tests/Feature/PlaylistFolderTest.php +++ b/tests/Feature/PlaylistFolderTest.php @@ -39,7 +39,7 @@ class PlaylistFolderTest extends TestCase { $folder = PlaylistFolder::factory()->create(['name' => 'Metal']); - $this->patchAs('api/playlist-folders/' . $folder->id, ['name' => 'Classical'], $folder->user) + $this->patchAs("api/playlist-folders/{$folder->id}", ['name' => 'Classical'], $folder->user) ->assertJsonStructure(PlaylistFolderResource::JSON_STRUCTURE); self::assertSame('Classical', $folder->fresh()->name); @@ -50,7 +50,7 @@ class PlaylistFolderTest extends TestCase { $folder = PlaylistFolder::factory()->create(['name' => 'Metal']); - $this->patchAs('api/playlist-folders/' . $folder->id, ['name' => 'Classical']) + $this->patchAs("api/playlist-folders/{$folder->id}", ['name' => 'Classical']) ->assertForbidden(); self::assertSame('Metal', $folder->fresh()->name); @@ -61,7 +61,7 @@ class PlaylistFolderTest extends TestCase { $folder = PlaylistFolder::factory()->create(); - $this->deleteAs('api/playlist-folders/' . $folder->id, ['name' => 'Classical'], $folder->user) + $this->deleteAs("api/playlist-folders/{$folder->id}", ['name' => 'Classical'], $folder->user) ->assertNoContent(); self::assertModelMissing($folder); @@ -73,7 +73,7 @@ class PlaylistFolderTest extends TestCase /** @var PlaylistFolder $folder */ $folder = PlaylistFolder::factory()->create(); - $this->deleteAs('api/playlist-folders/' . $folder->id, ['name' => 'Classical']) + $this->deleteAs("api/playlist-folders/{$folder->id}", ['name' => 'Classical']) ->assertForbidden(); self::assertModelExists($folder); @@ -89,7 +89,11 @@ class PlaylistFolderTest extends TestCase $playlist = Playlist::factory()->for($folder->user)->create(); self::assertNull($playlist->getFolderId($folder->user)); - $this->postAs("api/playlist-folders/$folder->id/playlists", ['playlists' => [$playlist->id]], $folder->user) + $this->postAs( + "api/playlist-folders/{$folder->id}/playlists", + ['playlists' => [$playlist->id]], + $folder->user + ) ->assertSuccessful(); self::assertTrue($playlist->fresh()->getFolder($folder->user)->is($folder)); @@ -105,7 +109,7 @@ class PlaylistFolderTest extends TestCase $playlist = Playlist::factory()->for($folder->user)->create(); self::assertNull($playlist->getFolderId($folder->user)); - $this->postAs("api/playlist-folders/$folder->id/playlists", ['playlists' => [$playlist->id]]) + $this->postAs("api/playlist-folders/{$folder->id}/playlists", ['playlists' => [$playlist->id]]) ->assertUnprocessable(); self::assertNull($playlist->fresh()->getFolder($folder->user)); @@ -120,10 +124,14 @@ class PlaylistFolderTest extends TestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->for($folder->user)->create(); - $folder->playlists()->attach($playlist->id); + $folder->playlists()->attach($playlist); self::assertTrue($playlist->refresh()->getFolder($folder->user)->is($folder)); - $this->deleteAs("api/playlist-folders/$folder->id/playlists", ['playlists' => [$playlist->id]], $folder->user) + $this->deleteAs( + "api/playlist-folders/{$folder->id}/playlists", + ['playlists' => [$playlist->id]], + $folder->user + ) ->assertSuccessful(); self::assertNull($playlist->fresh()->getFolder($folder->user)); @@ -138,10 +146,10 @@ class PlaylistFolderTest extends TestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->for($folder->user)->create(); - $folder->playlists()->attach($playlist->id); + $folder->playlists()->attach($playlist); self::assertTrue($playlist->refresh()->getFolder($folder->user)->is($folder)); - $this->deleteAs("api/playlist-folders/$folder->id/playlists", ['playlists' => [$playlist->id]]) + $this->deleteAs("api/playlist-folders/{$folder->id}/playlists", ['playlists' => [$playlist->id]]) ->assertUnprocessable(); self::assertTrue($playlist->refresh()->getFolder($folder->user)->is($folder)); diff --git a/tests/Feature/PlaylistSongTest.php b/tests/Feature/PlaylistSongTest.php index 3ded971d..515fb152 100644 --- a/tests/Feature/PlaylistSongTest.php +++ b/tests/Feature/PlaylistSongTest.php @@ -5,7 +5,6 @@ namespace Tests\Feature; use App\Http\Resources\SongResource; use App\Models\Playlist; use App\Models\Song; -use Illuminate\Support\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; @@ -20,7 +19,7 @@ class PlaylistSongTest extends TestCase $playlist = Playlist::factory()->create(); $playlist->addPlayables(Song::factory(5)->create()); - $this->getAs("api/playlists/$playlist->id/songs", $playlist->user) + $this->getAs("api/playlists/{$playlist->id}/songs", $playlist->user) ->assertJsonStructure(['*' => SongResource::JSON_STRUCTURE]); } @@ -46,7 +45,7 @@ class PlaylistSongTest extends TestCase ], ]); - $this->getAs("api/playlists/$playlist->id/songs", $playlist->user) + $this->getAs("api/playlists/{$playlist->id}/songs", $playlist->user) ->assertJsonStructure(['*' => SongResource::JSON_STRUCTURE]); } @@ -57,7 +56,7 @@ class PlaylistSongTest extends TestCase $playlist = Playlist::factory()->for(create_user())->create(); $playlist->addPlayables(Song::factory(5)->create()); - $this->getAs("api/playlists/$playlist->id/songs") + $this->getAs("api/playlists/{$playlist->id}/songs") ->assertForbidden(); } @@ -67,13 +66,12 @@ class PlaylistSongTest extends TestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(); - /** @var Collection $songs */ $songs = Song::factory(2)->create(); - $this->postAs("api/playlists/$playlist->id/songs", ['songs' => $songs->pluck('id')->all()], $playlist->user) + $this->postAs("api/playlists/{$playlist->id}/songs", ['songs' => $songs->modelKeys()], $playlist->user) ->assertSuccessful(); - self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->playables->pluck('id')->all()); + self::assertEqualsCanonicalizing($songs->modelKeys(), $playlist->playables->modelKeys()); } #[Test] @@ -84,7 +82,6 @@ class PlaylistSongTest extends TestCase $toRemainSongs = Song::factory(5)->create(); - /** @var Collection $toBeRemovedSongs */ $toBeRemovedSongs = Song::factory(2)->create(); $playlist->addPlayables($toRemainSongs->merge($toBeRemovedSongs)); @@ -92,15 +89,15 @@ class PlaylistSongTest extends TestCase self::assertCount(7, $playlist->playables); $this->deleteAs( - "api/playlists/$playlist->id/songs", - ['songs' => $toBeRemovedSongs->pluck('id')->all()], + "api/playlists/{$playlist->id}/songs", + ['songs' => $toBeRemovedSongs->modelKeys()], $playlist->user ) ->assertNoContent(); $playlist->refresh(); - self::assertEqualsCanonicalizing($toRemainSongs->pluck('id')->all(), $playlist->playables->pluck('id')->all()); + self::assertEqualsCanonicalizing($toRemainSongs->modelKeys(), $playlist->playables->modelKeys()); } #[Test] @@ -112,10 +109,10 @@ class PlaylistSongTest extends TestCase /** @var Song $song */ $song = Song::factory()->create(); - $this->postAs("api/playlists/$playlist->id/songs", ['songs' => [$song->id]]) + $this->postAs("api/playlists/{$playlist->id}/songs", ['songs' => [$song->id]]) ->assertForbidden(); - $this->deleteAs("api/playlists/$playlist->id/songs", ['songs' => [$song->id]]) + $this->deleteAs("api/playlists/{$playlist->id}/songs", ['songs' => [$song->id]]) ->assertForbidden(); } @@ -139,12 +136,12 @@ class PlaylistSongTest extends TestCase ], ]); - $songs = Song::factory(2)->create()->pluck('id')->all(); + $songs = Song::factory(2)->create()->modelKeys(); - $this->postAs("api/playlists/$playlist->id/songs", ['songs' => $songs], $playlist->user) + $this->postAs("api/playlists/{$playlist->id}/songs", ['songs' => $songs], $playlist->user) ->assertForbidden(); - $this->deleteAs("api/playlists/$playlist->id/songs", ['songs' => $songs], $playlist->user) + $this->deleteAs("api/playlists/{$playlist->id}/songs", ['songs' => $songs], $playlist->user) ->assertForbidden(); } } diff --git a/tests/Feature/PlaylistTest.php b/tests/Feature/PlaylistTest.php index 84985dbb..c7ab9744 100644 --- a/tests/Feature/PlaylistTest.php +++ b/tests/Feature/PlaylistTest.php @@ -6,7 +6,6 @@ use App\Http\Resources\PlaylistResource; use App\Models\Playlist; use App\Models\Song; use App\Values\SmartPlaylistRule; -use Illuminate\Support\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; @@ -30,23 +29,21 @@ class PlaylistTest extends TestCase { $user = create_user(); - /** @var array|Collection $songs */ $songs = Song::factory(4)->create(); $this->postAs('api/playlists', [ 'name' => 'Foo Bar', - 'songs' => $songs->pluck('id')->all(), + 'songs' => $songs->modelKeys(), 'rules' => [], ], $user) ->assertJsonStructure(PlaylistResource::JSON_STRUCTURE); - /** @var Playlist $playlist */ $playlist = Playlist::query()->latest()->first(); self::assertSame('Foo Bar', $playlist->name); self::assertTrue($playlist->ownedBy($user)); self::assertNull($playlist->getFolder()); - self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->playables->pluck('id')->all()); + self::assertEqualsCanonicalizing($songs->modelKeys(), $playlist->playables->modelKeys()); } #[Test] @@ -70,7 +67,6 @@ class PlaylistTest extends TestCase ], ], $user)->assertJsonStructure(PlaylistResource::JSON_STRUCTURE); - /** @var Playlist $playlist */ $playlist = Playlist::query()->latest()->first(); self::assertSame('Smart Foo Bar', $playlist->name); @@ -98,7 +94,7 @@ class PlaylistTest extends TestCase ], ], ], - 'songs' => Song::factory(3)->create()->pluck('id')->all(), + 'songs' => Song::factory(3)->create()->modelKeys(), ])->assertUnprocessable(); } @@ -116,10 +112,9 @@ class PlaylistTest extends TestCase #[Test] public function updatePlaylistName(): void { - /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(['name' => 'Foo']); - $this->putAs("api/playlists/$playlist->id", ['name' => 'Bar'], $playlist->user) + $this->putAs("api/playlists/{$playlist->id}", ['name' => 'Bar'], $playlist->user) ->assertJsonStructure(PlaylistResource::JSON_STRUCTURE); self::assertSame('Bar', $playlist->refresh()->name); @@ -128,20 +123,18 @@ class PlaylistTest extends TestCase #[Test] public function nonOwnerCannotUpdatePlaylist(): void { - /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(['name' => 'Foo']); - $this->putAs("api/playlists/$playlist->id", ['name' => 'Qux'])->assertForbidden(); + $this->putAs("api/playlists/{$playlist->id}", ['name' => 'Qux'])->assertForbidden(); self::assertSame('Foo', $playlist->refresh()->name); } #[Test] public function deletePlaylist(): void { - /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(); - $this->deleteAs("api/playlists/$playlist->id", [], $playlist->user); + $this->deleteAs("api/playlists/{$playlist->id}", [], $playlist->user); self::assertModelMissing($playlist); } @@ -149,10 +142,9 @@ class PlaylistTest extends TestCase #[Test] public function nonOwnerCannotDeletePlaylist(): void { - /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(); - $this->deleteAs("api/playlists/$playlist->id")->assertForbidden(); + $this->deleteAs("api/playlists/{$playlist->id}")->assertForbidden(); self::assertModelExists($playlist); } diff --git a/tests/Feature/QueueTest.php b/tests/Feature/QueueTest.php index 1c6b8917..991937eb 100644 --- a/tests/Feature/QueueTest.php +++ b/tests/Feature/QueueTest.php @@ -45,13 +45,13 @@ class QueueTest extends TestCase self::assertDatabaseMissing(QueueState::class, ['user_id' => $user->id]); - $songIds = Song::factory(3)->create()->pluck('id')->toArray(); + $songIds = Song::factory(3)->create()->modelKeys(); $this->putAs('api/queue/state', ['songs' => $songIds], $user) ->assertNoContent(); /** @var QueueState $queue */ - $queue = QueueState::query()->where('user_id', $user->id)->firstOrFail(); + $queue = QueueState::query()->whereBelongsTo($user)->firstOrFail(); self::assertEqualsCanonicalizing($songIds, $queue->song_ids); } diff --git a/tests/Feature/ScrobbleTest.php b/tests/Feature/ScrobbleTest.php index e9d1eb2e..3ffc8034 100644 --- a/tests/Feature/ScrobbleTest.php +++ b/tests/Feature/ScrobbleTest.php @@ -30,7 +30,7 @@ class ScrobbleTest extends TestCase ) ->once(); - $this->postAs("/api/songs/$song->id/scrobble", ['timestamp' => 100], $user) + $this->postAs("/api/songs/{$song->id}/scrobble", ['timestamp' => 100], $user) ->assertNoContent(); } } diff --git a/tests/Feature/SongPlayTest.php b/tests/Feature/SongPlayTest.php index 87a0a31f..db89296b 100644 --- a/tests/Feature/SongPlayTest.php +++ b/tests/Feature/SongPlayTest.php @@ -33,7 +33,7 @@ class SongPlayTest extends TestCase ->shouldReceive('stream') ->once(); - $this->get("play/$song->id?t=$token->audioToken") + $this->get("play/{$song->id}?t=$token->audioToken") ->assertOk(); } @@ -58,7 +58,7 @@ class SongPlayTest extends TestCase ->shouldReceive('stream') ->once(); - $this->get("play/$song->id?t=$token->audioToken") + $this->get("play/{$song->id}?t=$token->audioToken") ->assertOk(); config(['koel.streaming.transcode_flac' => false]); @@ -79,7 +79,7 @@ class SongPlayTest extends TestCase ->shouldReceive('stream') ->once(); - $this->get("play/$song->id/1?t=$token->audioToken") + $this->get("play/{$song->id}/1?t=$token->audioToken") ->assertOk(); } } diff --git a/tests/Feature/SongTest.php b/tests/Feature/SongTest.php index 446c4635..8fb0d5f0 100644 --- a/tests/Feature/SongTest.php +++ b/tests/Feature/SongTest.php @@ -35,10 +35,9 @@ class SongTest extends TestCase #[Test] public function destroy(): void { - /** @var Collection $songs */ $songs = Song::factory(3)->create(); - $this->deleteAs('api/songs', ['songs' => $songs->pluck('id')->all()], create_admin()) + $this->deleteAs('api/songs', ['songs' => $songs->modelKeys()], create_admin()) ->assertNoContent(); $songs->each(fn (Song $song) => $this->assertModelMissing($song)); @@ -47,10 +46,9 @@ class SongTest extends TestCase #[Test] public function unauthorizedDelete(): void { - /** @var Collection $songs */ $songs = Song::factory(3)->create(); - $this->deleteAs('api/songs', ['songs' => $songs->pluck('id')->all()]) + $this->deleteAs('api/songs', ['songs' => $songs->modelKeys()]) ->assertForbidden(); $songs->each(fn (Song $song) => $this->assertModelExists($song)); @@ -122,7 +120,7 @@ class SongTest extends TestCase #[Test] public function multipleUpdateNoCompilation(): void { - $songIds = Song::factory(3)->create()->pluck('id')->all(); + $songIds = Song::factory(3)->create()->modelKeys(); $this->putAs('/api/songs', [ 'songs' => $songIds, @@ -159,9 +157,8 @@ class SongTest extends TestCase #[Test] public function multipleUpdateCreatingNewAlbumsAndArtists(): void { - /** @var Collection $originalSongs */ $originalSongs = Song::factory(3)->create(); - $originalSongIds = $originalSongs->pluck('id')->all(); + $originalSongIds = $originalSongs->modelKeys(); $originalAlbumNames = $originalSongs->pluck('album.name')->all(); $originalAlbumIds = $originalSongs->pluck('album_id')->all(); @@ -177,7 +174,6 @@ class SongTest extends TestCase ], create_admin()) ->assertOk(); - /** @var Collection $songs */ $songs = Song::query()->whereIn('id', $originalSongIds)->get()->orderByArray($originalSongIds); // Even though the album name doesn't change, a new artist should have been created @@ -263,10 +259,7 @@ class SongTest extends TestCase { Song::factory(5)->create(); - self::assertNotSame(0, Song::query()->count()); - $ids = Song::query()->select('id')->get()->pluck('id')->all(); - - Song::deleteByChunk($ids, 1); + Song::deleteByChunk(Song::query()->get()->modelKeys(), 1); self::assertSame(0, Song::query()->count()); } diff --git a/tests/Feature/SongVisibilityTest.php b/tests/Feature/SongVisibilityTest.php index b20a6fff..cb46afcc 100644 --- a/tests/Feature/SongVisibilityTest.php +++ b/tests/Feature/SongVisibilityTest.php @@ -16,10 +16,10 @@ class SongVisibilityTest extends TestCase $owner = create_admin(); Song::factory(3)->create(); - $this->putAs('api/songs/publicize', ['songs' => Song::query()->pluck('id')->all()], $owner) + $this->putAs('api/songs/publicize', ['songs' => Song::query()->get()->modelKeys()], $owner) ->assertForbidden(); - $this->putAs('api/songs/privatize', ['songs' => Song::query()->pluck('id')->all()], $owner) + $this->putAs('api/songs/privatize', ['songs' => Song::query()->get()->modelKeys()], $owner) ->assertForbidden(); } } diff --git a/tests/Feature/UserTest.php b/tests/Feature/UserTest.php index 42273fe5..07b7809c 100644 --- a/tests/Feature/UserTest.php +++ b/tests/Feature/UserTest.php @@ -51,7 +51,7 @@ class UserTest extends TestCase $admin = create_admin(); $user = create_admin(['password' => 'secret']); - $this->putAs("api/user/$user->id", [ + $this->putAs("api/user/{$user->id}", [ 'name' => 'Foo', 'email' => 'bar@baz.com', 'password' => 'new-secret', @@ -72,7 +72,7 @@ class UserTest extends TestCase { $user = create_user(); - $this->deleteAs("api/user/$user->id", [], create_admin()); + $this->deleteAs("api/user/{$user->id}", [], create_admin()); self::assertModelMissing($user); } @@ -81,7 +81,7 @@ class UserTest extends TestCase { $admin = create_admin(); - $this->deleteAs("api/user/$admin->id", [], $admin)->assertForbidden(); + $this->deleteAs("api/user/{$admin->id}", [], $admin)->assertForbidden(); self::assertModelExists($admin); } } diff --git a/tests/Integration/KoelPlus/Services/SmartPlaylistServiceTest.php b/tests/Integration/KoelPlus/Services/SmartPlaylistServiceTest.php index d54f5404..cfa23969 100644 --- a/tests/Integration/KoelPlus/Services/SmartPlaylistServiceTest.php +++ b/tests/Integration/KoelPlus/Services/SmartPlaylistServiceTest.php @@ -52,8 +52,8 @@ class SmartPlaylistServiceTest extends PlusTestCase ]); self::assertEqualsCanonicalizing( - $matches->pluck('id')->all(), - $this->service->getSongs($playlist, $owner)->pluck('id')->all() + $matches->modelKeys(), + $this->service->getSongs($playlist, $owner)->modelKeys() ); } } diff --git a/tests/Integration/Services/InteractionServiceTest.php b/tests/Integration/Services/InteractionServiceTest.php index acf86610..67077ef6 100644 --- a/tests/Integration/Services/InteractionServiceTest.php +++ b/tests/Integration/Services/InteractionServiceTest.php @@ -66,8 +66,8 @@ class InteractionServiceTest extends TestCase $songs->each(static function (Song $song) use ($user): void { /** @var Interaction $interaction */ $interaction = Interaction::query() - ->where('song_id', $song->id) - ->where('user_id', $user->id) + ->whereBelongsTo($song) + ->whereBelongsTo($user) ->first(); self::assertTrue($interaction->liked); @@ -82,10 +82,9 @@ class InteractionServiceTest extends TestCase Event::fake(MultipleSongsUnliked::class); $user = create_user(); - /** @var Collection $interactions */ $interactions = Interaction::factory(3)->for($user)->create(['liked' => true]); - $this->interactionService->unlikeMany($interactions->map(static fn (Interaction $i) => $i->song), $user); + $this->interactionService->unlikeMany($interactions->map(static fn (Interaction $i) => $i->song), $user); // @phpstan-ignore-line $interactions->each(static function (Interaction $interaction): void { self::assertFalse($interaction->refresh()->liked); diff --git a/tests/Integration/Services/PlaylistServiceTest.php b/tests/Integration/Services/PlaylistServiceTest.php index ba30abe4..aee6696c 100644 --- a/tests/Integration/Services/PlaylistServiceTest.php +++ b/tests/Integration/Services/PlaylistServiceTest.php @@ -8,7 +8,6 @@ use App\Models\Podcast; use App\Models\Song; use App\Services\PlaylistService; use App\Values\SmartPlaylistRuleGroupCollection; -use Illuminate\Support\Collection; use InvalidArgumentException as BaseInvalidArgumentException; use PHPUnit\Framework\Attributes\Test; use Tests\PlusTestCase; @@ -43,17 +42,15 @@ class PlaylistServiceTest extends TestCase #[Test] public function createPlaylistWithSongs(): void { - /** @var Collection $songs */ $songs = Song::factory(3)->create(); - $user = create_user(); - $playlist = $this->service->createPlaylist('foo', $user, null, $songs->pluck('id')->all()); + $playlist = $this->service->createPlaylist('foo', $user, null, $songs->modelKeys()); self::assertSame('foo', $playlist->name); self::assertTrue($user->is($playlist->user)); self::assertFalse($playlist->is_smart); - self::assertEqualsCanonicalizing($playlist->playables->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($playlist->playables->modelKeys(), $songs->modelKeys()); } #[Test] @@ -213,7 +210,7 @@ class PlaylistServiceTest extends TestCase self::assertCount(2, $addedSongs); self::assertCount(5, $playlist->playables); - self::assertEqualsCanonicalizing($addedSongs->pluck('id')->all(), $songs->pluck('id')->all()); + self::assertEqualsCanonicalizing($addedSongs->modelKeys(), $songs->modelKeys()); $songs->each(static fn (Song $song) => self::assertTrue($playlist->playables->contains($song))); } @@ -235,7 +232,7 @@ class PlaylistServiceTest extends TestCase self::assertCount(2, $addedEpisodes); self::assertCount(5, $playlist->playables); - self::assertEqualsCanonicalizing($addedEpisodes->pluck('id')->all(), $episodes->pluck('id')->all()); + self::assertEqualsCanonicalizing($addedEpisodes->modelKeys(), $episodes->modelKeys()); } #[Test] @@ -252,7 +249,7 @@ class PlaylistServiceTest extends TestCase self::assertCount(4, $addedEpisodes); self::assertCount(7, $playlist->playables); - self::assertEqualsCanonicalizing($addedEpisodes->pluck('id')->all(), $playables->pluck('id')->all()); + self::assertEqualsCanonicalizing($addedEpisodes->modelKeys(), $playables->modelKeys()); } #[Test] @@ -292,23 +289,22 @@ class PlaylistServiceTest extends TestCase /** @var Playlist $playlist */ $playlist = Playlist::factory()->create(); - /** @var Collection $songs */ $songs = Song::factory(4)->create(); - $ids = $songs->pluck('id')->all(); + $ids = $songs->modelKeys(); $playlist->addPlayables($songs); $this->service->movePlayablesInPlaylist($playlist, [$ids[2], $ids[3]], $ids[0], 'after'); - self::assertSame([$ids[0], $ids[2], $ids[3], $ids[1]], $playlist->refresh()->playables->pluck('id')->all()); + self::assertSame([$ids[0], $ids[2], $ids[3], $ids[1]], $playlist->refresh()->playables->modelKeys()); $this->service->movePlayablesInPlaylist($playlist, [$ids[0]], $ids[3], 'before'); - self::assertSame([$ids[2], $ids[0], $ids[3], $ids[1]], $playlist->refresh()->playables->pluck('id')->all()); + self::assertSame([$ids[2], $ids[0], $ids[3], $ids[1]], $playlist->refresh()->playables->modelKeys()); // move to the first position $this->service->movePlayablesInPlaylist($playlist, [$ids[0], $ids[1]], $ids[2], 'before'); - self::assertSame([$ids[0], $ids[1], $ids[2], $ids[3]], $playlist->refresh()->playables->pluck('id')->all()); + self::assertSame([$ids[0], $ids[1], $ids[2], $ids[3]], $playlist->refresh()->playables->modelKeys()); // move to the last position $this->service->movePlayablesInPlaylist($playlist, [$ids[0], $ids[1]], $ids[3], 'after'); - self::assertSame([$ids[2], $ids[3], $ids[0], $ids[1]], $playlist->refresh()->playables->pluck('id')->all()); + self::assertSame([$ids[2], $ids[3], $ids[0], $ids[1]], $playlist->refresh()->playables->modelKeys()); } } diff --git a/tests/Integration/Services/QueueServiceTest.php b/tests/Integration/Services/QueueServiceTest.php index 8217ffbd..3c0a8e2d 100644 --- a/tests/Integration/Services/QueueServiceTest.php +++ b/tests/Integration/Services/QueueServiceTest.php @@ -49,11 +49,11 @@ class QueueServiceTest extends TestCase 'user_id' => $user->id, ]); - $songIds = Song::factory()->count(3)->create()->pluck('id')->toArray(); + $songIds = Song::factory()->count(3)->create()->modelKeys(); $this->service->updateQueueState($user, $songIds); /** @var QueueState $queueState */ - $queueState = QueueState::query()->where('user_id', $user->id)->firstOrFail(); + $queueState = QueueState::query()->whereBelongsTo($user)->firstOrFail(); self::assertEqualsCanonicalizing($songIds, $queueState->song_ids); self::assertNull($queueState->current_song_id); self::assertSame(0, $queueState->playback_position); @@ -65,7 +65,7 @@ class QueueServiceTest extends TestCase /** @var QueueState $state */ $state = QueueState::factory()->create(); - $songIds = Song::factory()->count(3)->create()->pluck('id')->toArray(); + $songIds = Song::factory()->count(3)->create()->modelKeys(); $this->service->updateQueueState($state->user, $songIds); $state->refresh(); diff --git a/tests/Integration/Services/SmartPlaylistServiceTest.php b/tests/Integration/Services/SmartPlaylistServiceTest.php index a2a911a9..a2c0fce1 100644 --- a/tests/Integration/Services/SmartPlaylistServiceTest.php +++ b/tests/Integration/Services/SmartPlaylistServiceTest.php @@ -9,7 +9,7 @@ use App\Models\Playlist; use App\Models\Song; use App\Models\User; use App\Services\SmartPlaylistService; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; @@ -564,8 +564,8 @@ class SmartPlaylistServiceTest extends TestCase $playlist = Playlist::factory()->for($owner ?? create_admin())->create(['rules' => $rules]); self::assertEqualsCanonicalizing( - $matches->pluck('id')->all(), - $this->service->getSongs($playlist)->pluck('id')->all() + $matches->modelKeys(), + $this->service->getSongs($playlist)->modelKeys() ); } } diff --git a/tests/Integration/Values/EpisodePlayableTest.php b/tests/Integration/Values/EpisodePlayableTest.php index 38831229..16d80723 100644 --- a/tests/Integration/Values/EpisodePlayableTest.php +++ b/tests/Integration/Values/EpisodePlayableTest.php @@ -28,7 +28,7 @@ class EpisodePlayableTest extends TestCase Http::assertSentCount(1); self::assertSame('acbd18db4cc2f85cedef654fccc4a4d8', $playable->checksum); - self::assertTrue(Cache::has("episode-playable.$episode->id")); + self::assertTrue(Cache::has("episode-playable.{$episode->id}")); $retrieved = EpisodePlayable::getForEpisode($episode); diff --git a/tests/Unit/Models/AlbumTest.php b/tests/Unit/Models/AlbumTest.php index 91bd7004..23d9137d 100644 --- a/tests/Unit/Models/AlbumTest.php +++ b/tests/Unit/Models/AlbumTest.php @@ -26,7 +26,7 @@ class AlbumTest extends TestCase $artist = Artist::factory()->create(); $name = 'Foo'; - self::assertNull(Album::query()->where('artist_id', $artist->id)->where('name', $name)->first()); + self::assertNull(Album::query()->whereBelongsTo($artist)->where('name', $name)->first()); $album = Album::getOrCreate($artist, $name); self::assertSame('Foo', $album->name); diff --git a/tests/Unit/Services/MediaInformationServiceTest.php b/tests/Unit/Services/MediaInformationServiceTest.php index 7f3c2909..fe9b8ca9 100644 --- a/tests/Unit/Services/MediaInformationServiceTest.php +++ b/tests/Unit/Services/MediaInformationServiceTest.php @@ -49,7 +49,7 @@ class MediaInformationServiceTest extends TestCase ->shouldNotReceive('tryDownloadAlbumCover'); self::assertSame($info, $this->mediaInformationService->getAlbumInformation($album)); - self::assertNotNull(cache()->get('album.info.' . $album->id)); + self::assertNotNull(cache()->get("album.info.{$album->id}")); } #[Test] @@ -93,7 +93,7 @@ class MediaInformationServiceTest extends TestCase ->shouldNotReceive('tryDownloadArtistImage'); self::assertSame($info, $this->mediaInformationService->getArtistInformation($artist)); - self::assertNotNull(cache()->get('artist.info.' . $artist->id)); + self::assertNotNull(cache()->get("artist.info.{$artist->id}")); } #[Test] diff --git a/tests/Unit/Services/PlaylistFolderServiceTest.php b/tests/Unit/Services/PlaylistFolderServiceTest.php index 0ab05189..5a73a67b 100644 --- a/tests/Unit/Services/PlaylistFolderServiceTest.php +++ b/tests/Unit/Services/PlaylistFolderServiceTest.php @@ -5,7 +5,7 @@ namespace Tests\Unit\Services; use App\Models\Playlist; use App\Models\PlaylistFolder; use App\Services\PlaylistFolderService; -use Illuminate\Support\Collection; +use Illuminate\Database\Eloquent\Collection; use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; @@ -57,7 +57,7 @@ class PlaylistFolderServiceTest extends TestCase /** @var PlaylistFolder $folder */ $folder = PlaylistFolder::factory()->for($user)->create(); - $this->service->addPlaylistsToFolder($folder, $playlists->pluck('id')->all()); + $this->service->addPlaylistsToFolder($folder, $playlists->modelKeys()); self::assertCount(3, $folder->playlists); } @@ -70,9 +70,9 @@ class PlaylistFolderServiceTest extends TestCase /** @var Collection $playlists */ $playlists = Playlist::factory()->count(3)->create(); - $folder->playlists()->attach($playlists->pluck('id')->all()); + $folder->playlists()->attach($playlists); - $this->service->movePlaylistsToRootLevel($folder, $playlists->pluck('id')->all()); + $this->service->movePlaylistsToRootLevel($folder, $playlists->modelKeys()); self::assertCount(0, $folder->playlists);