Refactor and use extending request classes

This commit is contained in:
Phan An 2017-12-09 19:34:27 +01:00
parent 3270879031
commit bca8668ace
30 changed files with 378 additions and 76 deletions

View file

@ -20,15 +20,8 @@ class AuthController extends Controller
*/
public function login(UserLoginRequest $request)
{
try {
if (!$token = JWTAuth::attempt($request->only('email', 'password'))) {
return response()->json(['error' => 'invalid_credentials'], 401);
}
} catch (JWTException $e) {
Log::error($e);
return response()->json(['error' => 'could_not_create_token'], 500);
}
$token = JWTAuth::attempt($request->only('email', 'password'));
abort_unless($token, 401, 'Invalid credentials');
return response()->json(compact('token'));
}

View file

@ -35,7 +35,8 @@ class DataController extends Controller
'useYouTube' => YouTube::enabled(),
'useiTunes' => iTunes::used(),
'allowDownload' => config('koel.download.allow'),
'supportsTranscoding' => config('koel.streaming.ffmpeg_path') && is_executable(config('koel.streaming.ffmpeg_path')),
'supportsTranscoding' => config('koel.streaming.ffmpeg_path')
&& is_executable(config('koel.streaming.ffmpeg_path')),
'cdnUrl' => app()->staticUrl(),
'currentVersion' => Application::KOEL_VERSION,
'latestVersion' => $request->user()->is_admin ? app()->getLatestVersion() : Application::KOEL_VERSION,

View file

@ -2,20 +2,20 @@
namespace App\Http\Controllers\API\Interaction;
use App\Http\Requests\API\SongLikeRequest;
use App\Models\Interaction;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
class LikeController extends Controller
{
/**
* Like or unlike a song as the currently authenticated user.
*
* @param Request $request
* @param SongLikeRequest $request
*
* @return JsonResponse
*/
public function store(Request $request)
public function store(SongLikeRequest $request)
{
return response()->json(Interaction::toggleLike($request->song, $request->user()));
}

View file

@ -3,20 +3,20 @@
namespace App\Http\Controllers\API\Interaction;
use App\Events\SongStartedPlaying;
use App\Http\Requests\API\Interaction\StorePlayCountRequest;
use App\Models\Interaction;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
class PlayCountController extends Controller
{
/**
* Increase a song's play count as the currently authenticated user.
*
* @param Request $request
* @param StorePlayCountRequest $request
*
* @return JsonResponse
*/
public function store(Request $request)
public function store(StorePlayCountRequest $request)
{
$interaction = Interaction::increasePlayCount($request->song, $request->user());
if ($interaction) {

View file

@ -2,11 +2,12 @@
namespace App\Http\Controllers\API;
use App\Http\Requests\API\LastfmCallbackRequest;
use App\Http\Requests\API\LastfmSetSessionKeyRequest;
use App\Services\Lastfm;
use Illuminate\Contracts\Auth\Guard;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Routing\Redirector;
use Tymon\JWTAuth\JWTAuth;
@ -60,17 +61,15 @@ class LastfmController extends Controller
/**
* Serve the callback request from Last.fm.
*
* @param Request $request
* @param Lastfm $lastfm
* @param LastfmCallbackRequest $request
* @param Lastfm $lastfm
*
* @return Response
*/
public function callback(Request $request, Lastfm $lastfm)
public function callback(LastfmCallbackRequest $request, Lastfm $lastfm)
{
abort_unless($token = $request->token, 500, 'Something wrong happened.');
// Get the session key using the obtained token.
abort_unless($sessionKey = $lastfm->getSessionKey($token), 500, 'Invalid token key.');
abort_unless($sessionKey = $lastfm->getSessionKey($request->token), 500, 'Invalid token key.');
$this->auth->user()->savePreference('lastfm_session_key', $sessionKey);
@ -80,11 +79,11 @@ class LastfmController extends Controller
/**
* Set the Last.fm session key of the current user.
*
* @param Request $request
* @param LastfmSetSessionKeyRequest $request
*
* @return JsonResponse
*/
public function setSessionKey(Request $request)
public function setSessionKey(LastfmSetSessionKeyRequest $request)
{
$this->auth->user()->savePreference('lastfm_session_key', trim($request->key));

View file

@ -3,6 +3,7 @@
namespace App\Http\Controllers\API;
use App\Http\Requests\API\PlaylistStoreRequest;
use App\Http\Requests\API\PlaylistSyncRequest;
use App\Models\Playlist;
use Exception;
use Illuminate\Auth\Access\AuthorizationException;
@ -61,14 +62,13 @@ class PlaylistController extends Controller
* Sync a playlist with songs.
* Any songs that are not populated here will be removed from the playlist.
*
* @param Request $request
* @param Playlist $playlist
*
* @throws AuthorizationException
* @param PlaylistSyncRequest $request
* @param Playlist $playlist
*
* @return JsonResponse
* @throws AuthorizationException
*/
public function sync(Request $request, Playlist $playlist)
public function sync(PlaylistSyncRequest $request, Playlist $playlist)
{
$this->authorize('owner', $playlist);
@ -80,12 +80,12 @@ class PlaylistController extends Controller
/**
* Get a playlist's all songs.
*
* @param Request $request
* @param Playlist $playlist
*
* @return JsonResponse
* @throws AuthorizationException
*/
public function getSongs(Request $request, Playlist $playlist)
public function getSongs(Playlist $playlist)
{
$this->authorize('owner', $playlist);

View file

@ -2,6 +2,7 @@
namespace App\Http\Controllers\API;
use App\Http\Requests\API\SongPlayRequest;
use App\Http\Requests\API\SongUpdateRequest;
use App\Models\Song;
use App\Services\Streamers\PHPStreamer;
@ -11,7 +12,6 @@ use App\Services\Streamers\XAccelRedirectStreamer;
use App\Services\Streamers\XSendFileStreamer;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Routing\Redirector;
class SongController extends Controller
@ -21,16 +21,16 @@ class SongController extends Controller
*
* @link https://github.com/phanan/koel/wiki#streaming-music
*
* @param Request $request
* @param Song $song The song to stream.
* @param null|bool $transcode Whether to force transcoding the song.
* If this is omitted, by default Koel will transcode FLAC.
* @param null|int $bitRate The target bit rate to transcode, defaults to OUTPUT_BIT_RATE.
* Only taken into account if $transcode is truthy.
* @param SongPlayRequest $request
* @param Song $song The song to stream.
* @param null|bool $transcode Whether to force transcoding the song.
* If this is omitted, by default Koel will transcode FLAC.
* @param null|int $bitRate The target bit rate to transcode, defaults to OUTPUT_BIT_RATE.
* Only taken into account if $transcode is truthy.
*
* @return RedirectResponse|Redirector
*/
public function play(Request $request, Song $song, $transcode = null, $bitRate = null)
public function play(SongPlayRequest $request, Song $song, $transcode = null, $bitRate = null)
{
if ($song->s3_params) {
return (new S3Streamer($song))->stream();

View file

@ -2,22 +2,21 @@
namespace App\Http\Controllers\API;
use App\Http\Requests\API\YouTubeSearchRequest;
use App\Models\Song;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use YouTube;
class YouTubeController extends Controller
{
/**
* Search for YouTube videos related to a song (using its title and artist name).
*
* @param Request $request
* @param Song $song
* @param YouTubeSearchRequest $request
* @param Song $song
*
* @return JsonResponse
*/
public function searchVideosRelatedToSong(Request $request, Song $song)
public function searchVideosRelatedToSong(YouTubeSearchRequest $request, Song $song)
{
return response()->json($song->getRelatedYouTubeVideos($request->pageToken));
}

View file

@ -2,7 +2,7 @@
namespace App\Http\Controllers\API;
use App\Http\Requests\API\ViewSongRequest;
use App\Http\Requests\API\ViewSongOnITunesRequest;
use App\Models\Album;
use Illuminate\Http\RedirectResponse;
use iTunes;
@ -12,12 +12,12 @@ class iTunesController extends Controller
/**
* View a song on iTunes store.
*
* @param ViewSongRequest $request
* @param Album $album
* @param ViewSongOnITunesRequest $request
* @param Album $album
*
* @return RedirectResponse
*/
public function viewSong(ViewSongRequest $request, Album $album)
public function viewSong(ViewSongOnITunesRequest $request, Album $album)
{
$url = iTunes::getTrackUrl($request->q, $album->name, $album->artist->name);
abort_unless($url, 404, "Koel can't find such a song on iTunes Store.");

View file

@ -2,6 +2,9 @@
namespace App\Http\Requests\API;
/**
* @property array songs
*/
class BatchInteractionRequest extends Request
{
/**

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API\Interaction;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property string song
*/
class StorePlayCountRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'song' => 'required',
];
}
}

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property string token
*/
class LastfmCallbackRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'token' => 'required',
];
}
}

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property string key
*/
class LastfmSetSessionKeyRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'key' => 'required',
];
}
}

View file

@ -2,6 +2,9 @@
namespace App\Http\Requests\API;
/**
* @property array songs
*/
class PlaylistStoreRequest extends Request
{
/**

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property string songs
*/
class PlaylistSyncRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'songs' => 'required',
];
}
}

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use App\Models\Song;
/**
* @property Song song
*/
class SongLikeRequest extends Request
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
];
}
}

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property float time
*/
class SongPlayRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'time' => 'required',
];
}
}

View file

@ -7,7 +7,7 @@ use Illuminate\Foundation\Http\FormRequest;
/**
* @property string q
*/
class ViewSongRequest extends FormRequest
class ViewSongOnITunesRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Requests\API;
use Illuminate\Foundation\Http\FormRequest;
/**
* @property string pageToken
*/
class YouTubeSearchRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
];
}
}

View file

@ -4,6 +4,10 @@ namespace App\Models;
use Illuminate\Database\Eloquent\Model;
/**
* @property string $key
* @property mixed $value
*/
class Setting extends Model
{
protected $primaryKey = 'key';

View file

@ -34,7 +34,7 @@ class Download
} elseif ($mixed instanceof Playlist) {
return $this->fromPlaylist($mixed);
} else {
throw new Exception('Unsupport download type.');
throw new Exception('Unsupported download type.');
}
}
@ -71,7 +71,9 @@ class Download
// For those with high-byte characters in names, we copy it into a safe name
// as a workaround.
$newPath = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.utf8_decode(basename($song->path));
$newPath = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR)
.DIRECTORY_SEPARATOR
.utf8_decode(basename($song->path));
if ($s3Params) {
// If the file is downloaded from S3, we rename it directly.
@ -106,16 +108,34 @@ class Download
->getPath();
}
/**
* @param Playlist $playlist
*
* @return string
* @throws Exception
*/
protected function fromPlaylist(Playlist $playlist)
{
return $this->fromMultipleSongs($playlist->songs);
}
/**
* @param Album $album
*
* @return string
* @throws Exception
*/
protected function fromAlbum(Album $album)
{
return $this->fromMultipleSongs($album->songs);
}
/**
* @param Artist $artist
*
* @return string
* @throws Exception
*/
protected function fromArtist(Artist $artist)
{
return $this->fromMultipleSongs($artist->songs);

View file

@ -10,6 +10,7 @@ use App\Models\Artist;
use App\Models\File;
use App\Models\Setting;
use App\Models\Song;
use Exception;
use getID3;
use Illuminate\Support\Facades\Log;
use Symfony\Component\Finder\Finder;
@ -51,6 +52,8 @@ class Media
* New records will have all tags synced in regardless.
* @param bool $force Whether to force syncing even unchanged files
* @param SyncMedia $syncCommand The SyncMedia command object, to log to console if executed by artisan.
*
* @throws Exception
*/
public function sync($mediaPath = null, $tags = [], $force = false, SyncMedia $syncCommand = null)
{
@ -131,6 +134,8 @@ class Media
* Sync media using a watch record.
*
* @param WatchRecordInterface $record The watch record.
*
* @throws Exception
*/
public function syncByWatchRecord(WatchRecordInterface $record)
{
@ -142,6 +147,8 @@ class Media
* Sync a file's watch record.
*
* @param WatchRecordInterface $record
*
* @throws Exception
*/
private function syncFileRecord(WatchRecordInterface $record)
{
@ -230,6 +237,7 @@ class Media
/**
* Tidy up the library by deleting empty albums and artists.
* @throws Exception
*/
public function tidy()
{

View file

@ -68,12 +68,8 @@ class iTunes
}
$trackUrl = $response->results[0]->trackViewUrl;
if (parse_url($trackUrl, PHP_URL_QUERY)) {
$trackUrl .= '&at='.config('koel.itunes.affiliate_id');
} else {
$trackUrl .= '?at='.config('koel.itunes.affiliate_id');
}
$connector = parse_url($trackUrl, PHP_URL_QUERY) ? '&' : '?';
$trackUrl .= "{$connector}at=".config('koel.itunes.affiliate_id');
return $trackUrl;
}

View file

@ -42,7 +42,10 @@ class InteractionTest extends TestCase
]);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function user_can_like_and_unlike_a_song()
{
$this->expectsEvents(SongLikeToggled::class);
@ -68,7 +71,10 @@ class InteractionTest extends TestCase
]);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function user_can_like_and_unlike_songs_in_batch()
{
$this->expectsEvents(SongLikeToggled::class);

View file

@ -5,6 +5,7 @@ namespace Tests\Feature;
use App\Events\SongLikeToggled;
use App\Events\SongStartedPlaying;
use App\Http\Controllers\API\LastfmController;
use App\Http\Requests\API\LastfmCallbackRequest;
use App\Listeners\LoveTrackOnLastfm;
use App\Listeners\UpdateLastfmNowPlaying;
use App\Models\Interaction;
@ -15,7 +16,6 @@ use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Response;
use Illuminate\Contracts\Auth\Guard;
use Illuminate\Foundation\Testing\WithoutMiddleware;
use Illuminate\Http\Request;
use Illuminate\Routing\Redirector;
use Mockery as m;
use Tymon\JWTAuth\JWTAuth;
@ -53,9 +53,11 @@ class LastfmTest extends TestCase
/** @test */
public function user_can_connect_to_lastfm()
{
/** @var Redirector|m\MockInterface $redirector */
$redirector = m::mock(Redirector::class);
$redirector->shouldReceive('to')->once();
/** @var Guard|m\MockInterface $guard */
$guard = m::mock(Guard::class, ['user' => factory(User::class)->create()]);
$auth = m::mock(JWTAuth::class, [
'parseToken' => '',
@ -68,11 +70,14 @@ class LastfmTest extends TestCase
/** @test */
public function lastfm_session_key_can_be_retrieved_and_stored()
{
$request = m::mock(Request::class);
/** @var LastfmCallbackRequest|m\MockInterface $request */
$request = m::mock(LastfmCallbackRequest::class);
$request->token = 'foo';
/** @var Lastfm|m\MockInterface $lastfm */
$lastfm = m::mock(Lastfm::class, ['getSessionKey' => 'bar']);
$user = factory(User::class)->create();
/** @var Guard|m\MockInterface $guard */
$guard = m::mock(Guard::class, ['user' => $user]);
(new LastfmController($guard))->callback($request, $lastfm);
@ -102,6 +107,7 @@ class LastfmTest extends TestCase
'song_id' => Song::first()->id,
]);
/** @var Lastfm|m\MockInterface $lastfm */
$lastfm = m::mock(Lastfm::class, ['enabled' => true]);
$lastfm->shouldReceive('toggleLoveTrack')
->with($interaction->song->title, $interaction->song->album->artist->name, 'bar', false);
@ -118,6 +124,7 @@ class LastfmTest extends TestCase
$user = factory(User::class)->create(['preferences' => ['lastfm_session_key' => 'bar']]);
$song = Song::first();
/** @var Lastfm|m\MockInterface $lastfm */
$lastfm = m::mock(Lastfm::class, ['enabled' => true]);
$lastfm->shouldReceive('updateNowPlaying')
->with($song->album->artist->name, $song->title, $song->album->name, $song->length, 'bar');

View file

@ -9,6 +9,7 @@ use App\Models\Artist;
use App\Models\File;
use App\Models\Song;
use App\Services\Media;
use getID3;
use Illuminate\Foundation\Testing\WithoutMiddleware;
use Mockery as m;
@ -22,7 +23,10 @@ class MediaTest extends TestCase
parent::tearDown();
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function songs_can_be_synced()
{
$this->expectsEvents(LibraryChanged::class);
@ -85,7 +89,10 @@ class MediaTest extends TestCase
$this->assertEquals($currentCover, Album::find($album->id)->cover);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function songs_can_be_force_synced()
{
$this->expectsEvents(LibraryChanged::class);
@ -120,7 +127,10 @@ class MediaTest extends TestCase
$this->assertEquals($originalLyrics, $song->lyrics);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function songs_can_be_synced_with_selectively_tags()
{
$this->expectsEvents(LibraryChanged::class);
@ -146,7 +156,10 @@ class MediaTest extends TestCase
$this->assertEquals('Booom Wroooom', $song->lyrics);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function all_tags_are_catered_for_if_syncing_new_file()
{
// First we sync the test directory to get the data
@ -168,7 +181,10 @@ class MediaTest extends TestCase
$this->assertEquals($song, $addedSong);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function added_song_is_synced_when_watching()
{
$this->expectsEvents(LibraryChanged::class);
@ -180,7 +196,10 @@ class MediaTest extends TestCase
$this->seeInDatabase('songs', ['path' => $path]);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function deleted_song_is_synced_when_watching()
{
$this->expectsEvents(LibraryChanged::class);
@ -193,7 +212,10 @@ class MediaTest extends TestCase
$this->notSeeInDatabase('songs', ['id' => $song->id]);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function deleted_directory_is_synced_when_watching()
{
$this->expectsEvents(LibraryChanged::class);
@ -232,7 +254,10 @@ class MediaTest extends TestCase
$this->assertEquals('水谷広実', $info['title']);
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function hidden_files_can_optionally_be_ignored_when_syncing()
{
config(['koel.ignore_dot_files' => false]);

View file

@ -18,7 +18,7 @@ class ScrobbleTest extends TestCase
}
/** @test */
public function a_song_can_be_scrobbed_via_lastfm()
public function a_song_can_be_scrobbled_via_lastfm()
{
$this->withoutEvents();
$this->createSampleMediaSet();

View file

@ -17,8 +17,7 @@ class SettingTest extends TestCase
Media::shouldReceive('sync')->once();
$user = factory(User::class, 'admin')->create();
$this->postAsUser('/api/settings', ['media_path' => __DIR__], $user)
->seeStatusCode(200);
$this->postAsUser('/api/settings', ['media_path' => __DIR__], $user)->seeStatusCode(200);
$this->assertEquals(__DIR__, Setting::get('media_path'));
}

View file

@ -19,6 +19,9 @@ class SongTest extends TestCase
$this->createSampleMediaSet();
}
/**
* @throws \Exception
*/
public function testSingleUpdateAllInfoNoCompilation()
{
$this->expectsEvents(LibraryChanged::class);
@ -195,7 +198,6 @@ class SongTest extends TestCase
'compilationState' => 2,
],
], $user)
// ->seeText('jaja')
->seeStatusCode(200);
$compilationAlbum = Album::whereArtistIdAndName(Artist::VARIOUS_ID, 'Two by Two')->first();
@ -262,7 +264,7 @@ class SongTest extends TestCase
]);
// Case 3: Change compilation state and artist
// Remember to set the compliation state back to 1
// Remember to set the compilation state back to 1
$this->putAsUser('/api/songs', [
'songs' => [$song->id],
'data' => [
@ -299,6 +301,9 @@ class SongTest extends TestCase
]);
}
/**
* @throws \Exception
*/
public function testDeletingByChunk()
{
$this->assertNotEquals(0, Song::count());

View file

@ -16,7 +16,10 @@ class YouTubeTest extends TestCase
parent::tearDown();
}
/** @test */
/**
* @test
* @throws \Exception
*/
public function videos_can_be_searched_from_youtube()
{
$this->withoutEvents();