diff --git a/app/Http/Controllers/PlayerController.php b/app/Http/Controllers/PlayerController.php index f2fb45e0..0724a0a6 100644 --- a/app/Http/Controllers/PlayerController.php +++ b/app/Http/Controllers/PlayerController.php @@ -6,7 +6,6 @@ use App\Events\PlayerWasAdded; use App\Events\PlayerWasDeleted; use App\Events\PlayerWillBeAdded; use App\Events\PlayerWillBeDeleted; -use App\Http\Middleware\CheckPlayerExist; use App\Http\Middleware\CheckPlayerOwner; use App\Models\Player; use App\Models\Texture; @@ -23,7 +22,7 @@ class PlayerController extends Controller { public function __construct() { - $this->middleware([CheckPlayerExist::class, CheckPlayerOwner::class], [ + $this->middleware([CheckPlayerOwner::class], [ 'only' => ['delete', 'rename', 'setTexture', 'clearTexture'], ]); } @@ -114,11 +113,13 @@ class PlayerController extends Controller return json(trans('user.player.add.success', ['name' => $name]), 0, $player->toArray()); } - public function delete(Dispatcher $dispatcher, Filter $filter, $pid) - { + public function delete( + Dispatcher $dispatcher, + Filter $filter, + Player $player + ) { /** @var User */ $user = auth()->user(); - $player = Player::find($pid); $playerName = $player->name; $dispatcher->dispatch('player.delete.attempt', [$player, $user]); @@ -152,7 +153,7 @@ class PlayerController extends Controller Request $request, Dispatcher $dispatcher, Filter $filter, - $pid + Player $player ) { $name = $request->validate([ 'name' => [ @@ -160,11 +161,10 @@ class PlayerController extends Controller new Rules\PlayerName(), 'min:'.option('player_name_length_min'), 'max:'.option('player_name_length_max'), - Rule::unique('players')->ignore($pid), + Rule::unique('players')->ignoreModel($player), ], ])['name']; $name = $filter->apply('new_player_name', $name); - $player = Player::find($pid); $dispatcher->dispatch('player.renaming', [$player, $name]); @@ -197,9 +197,8 @@ class PlayerController extends Controller Request $request, Dispatcher $dispatcher, Filter $filter, - $pid + Player $player ) { - $player = Player::find($pid); foreach (['skin', 'cape'] as $type) { $tid = $request->input($type); @@ -231,9 +230,8 @@ class PlayerController extends Controller Request $request, Dispatcher $dispatcher, Filter $filter, - $pid + Player $player ) { - $player = Player::find($pid); $types = $request->input('type', []); foreach (['skin', 'cape'] as $type) { diff --git a/app/Http/Middleware/CheckPlayerExist.php b/app/Http/Middleware/CheckPlayerExist.php deleted file mode 100644 index 8f6d1a9c..00000000 --- a/app/Http/Middleware/CheckPlayerExist.php +++ /dev/null @@ -1,19 +0,0 @@ -route()->parameters, 'pid') ?? $request->input('pid'); - if (!$request->isMethod('get') && !is_null($pid) && is_null(Player::find($pid))) { - return json(trans('general.unexistent-player'), 1); - } - - return $next($request); - } -} diff --git a/resources/assets/src/scripts/urls.ts b/resources/assets/src/scripts/urls.ts index e0098ca7..20963a41 100644 --- a/resources/assets/src/scripts/urls.ts +++ b/resources/assets/src/scripts/urls.ts @@ -41,19 +41,19 @@ export default { page: () => '/user/closet' as const, ids: () => '/user/closet/ids' as const, list: () => '/user/closet/list' as const, - remove: (tid: number) => `/user/closet/${tid}`, rename: (tid: number) => `/user/closet/${tid}`, + remove: (tid: number) => `/user/closet/${tid}`, }, notification: (id: number) => `/user/notifications/${id}`, player: { + add: () => '/user/player' as const, page: () => '/user/player' as const, - add: () => '/user/player/add' as const, bind: () => '/user/player/bind' as const, - delete: (pid: number) => `/user/player/delete/${pid}`, list: () => '/user/player/list' as const, - rename: (pid: number) => `/user/player/rename/${pid}`, - set: (pid: number) => `/user/player/set/${pid}`, - clear: (pid: number) => `/user/player/texture/clear/${pid}`, + delete: (player: number) => `/user/player/${player}`, + rename: (player: number) => `/user/player/${player}/name`, + clear: (player: number) => `/user/player/${player}/textures`, + set: (player: number) => `/user/player/${player}/textures`, }, profile: { avatar: () => '/user/profile/avatar' as const }, score: () => '/user/score-info' as const, diff --git a/routes/api.php b/routes/api.php index d6b68251..807eafc7 100644 --- a/routes/api.php +++ b/routes/api.php @@ -18,10 +18,10 @@ Route::prefix('user')->middleware('auth:jwt,oauth')->group(function () { Route::prefix('players')->middleware('auth:jwt,oauth')->group(function () { Route::get('', 'PlayerController@list'); Route::post('', 'PlayerController@add'); - Route::delete('{pid}', 'PlayerController@delete'); - Route::put('{pid}/name', 'PlayerController@rename'); - Route::put('{pid}/textures', 'PlayerController@setTexture'); - Route::delete('{pid}/textures', 'PlayerController@clearTexture'); + Route::delete('{player}', 'PlayerController@delete'); + Route::put('{player}/name', 'PlayerController@rename'); + Route::put('{player}/textures', 'PlayerController@setTexture'); + Route::delete('{player}/textures', 'PlayerController@clearTexture'); }); Route::prefix('closet')->middleware('auth:jwt,oauth')->group(function () { diff --git a/routes/web.php b/routes/web.php index 39c340e7..7293f16e 100644 --- a/routes/web.php +++ b/routes/web.php @@ -71,11 +71,11 @@ Route::prefix('user') ->group(function () { Route::get('', 'PlayerController@index')->name('page'); Route::get('list', 'PlayerController@list')->name('list'); - Route::post('add', 'PlayerController@add')->name('add'); - Route::post('set/{pid}', 'PlayerController@setTexture')->name('set'); - Route::post('texture/clear/{pid}', 'PlayerController@clearTexture')->name('clear'); - Route::post('rename/{pid}', 'PlayerController@rename')->name('rename'); - Route::post('delete/{pid}', 'PlayerController@delete')->name('delete'); + Route::post('', 'PlayerController@add')->name('add'); + Route::put('{player}/textures', 'PlayerController@setTexture')->name('set'); + Route::delete('{player}/textures', 'PlayerController@clearTexture')->name('clear'); + Route::put('{player}/name', 'PlayerController@rename')->name('rename'); + Route::delete('{player}', 'PlayerController@delete')->name('delete'); Route::view('bind', 'user.bind')->name('bind'); Route::post('bind', 'PlayerController@bind')->name('bind'); }); diff --git a/tests/HttpTest/ControllersTest/PlayerControllerTest.php b/tests/HttpTest/ControllersTest/PlayerControllerTest.php index 24a7e68f..ec5b7186 100644 --- a/tests/HttpTest/ControllersTest/PlayerControllerTest.php +++ b/tests/HttpTest/ControllersTest/PlayerControllerTest.php @@ -43,12 +43,12 @@ class PlayerControllerTest extends TestCase $filter = Fakes\Filter::fake(); // Without player name - $this->postJson('/user/player/add')->assertJsonValidationErrors('name'); + $this->postJson(route('user.player.add'))->assertJsonValidationErrors('name'); // Only A-Za-z0-9_ are allowed option(['player_name_rule' => 'official']); $this->postJson( - '/user/player/add', + route('user.player.add'), ['name' => '角色名'] )->assertJsonValidationErrors('name'); @@ -56,20 +56,20 @@ class PlayerControllerTest extends TestCase option(['player_name_rule' => 'custom']); option(['custom_player_name_regexp' => '/^\d+$/']); $this->postJson( - '/user/player/add', + route('user.player.add'), ['name' => 'yjsnpi'] )->assertJsonValidationErrors('name'); // with an existed player name option(['player_name_rule' => 'official']); $existed = factory(Player::class)->create(); - $this->postJson('/user/player/add', ['name' => $existed->name]) + $this->postJson(route('user.player.add'), ['name' => $existed->name]) ->assertJsonValidationErrors('name'); // Lack of score $user = factory(User::class)->create(['score' => 0]); $this->actingAs($user)->postJson( - '/user/player/add', + route('user.player.add'), ['name' => 'no_score'] )->assertJson([ 'code' => 7, @@ -97,7 +97,7 @@ class PlayerControllerTest extends TestCase return new Rejection('rejected'); }); $this->postJson( - '/user/player/add', + route('user.player.add'), ['name' => 'can'] )->assertJson(['code' => 1, 'message' => 'rejected']); Event::assertDispatched('player.add.attempt'); @@ -110,7 +110,7 @@ class PlayerControllerTest extends TestCase option(['player_name_rule' => 'cjk']); $user = factory(User::class)->create(); $score = $user->score; - $this->actingAs($user)->postJson('/user/player/add', [ + $this->actingAs($user)->postJson(route('user.player.add'), [ 'name' => '角色名', ])->assertJson([ 'code' => 0, @@ -147,7 +147,7 @@ class PlayerControllerTest extends TestCase // Single player option(['single_player' => true]); - $this->postJson('/user/player/add', ['name' => 'abc']) + $this->postJson(route('user.player.add'), ['name' => 'abc']) ->assertJson([ 'code' => 1, 'message' => trans('user.player.add.single'), @@ -170,12 +170,12 @@ class PlayerControllerTest extends TestCase return new Rejection('rejected'); }); $this->actingAs($user) - ->postJson('/user/player/delete/'.$player->pid) + ->deleteJson(route('user.player.delete', ['player' => $player])) ->assertJson(['code' => 1, 'message' => 'rejected']); // success $filter = Fakes\Filter::fake(); - $this->postJson('/user/player/delete/'.$player->pid) + $this->deleteJson(route('user.player.delete', ['player' => $player])) ->assertJson([ 'code' => 0, 'message' => trans('user.player.delete.success', ['name' => $player->name]), @@ -211,7 +211,7 @@ class PlayerControllerTest extends TestCase $player = factory(Player::class)->create(); $user = $player->user; $this->actingAs($user) - ->postJson('/user/player/delete/'.$player->pid) + ->deleteJson(route('user.player.delete', ['player' => $player])) ->assertJson([ 'code' => 0, 'message' => trans('user.player.delete.success', ['name' => $player->name]), @@ -225,7 +225,7 @@ class PlayerControllerTest extends TestCase option(['single_player' => true]); $player = factory(Player::class)->create(['uid' => $user->uid]); $this->actingAs($user) - ->postJson('/user/player/delete/'.$player->pid) + ->deleteJson(route('user.player.delete', ['player' => $player])) ->assertJson([ 'code' => 1, 'message' => trans('user.player.delete.single'), @@ -242,23 +242,29 @@ class PlayerControllerTest extends TestCase // Without new player name $this->actingAs($user) - ->postJson('/user/player/rename/'.$player->pid) + ->putJson(route('user.player.rename', ['player' => $player])) ->assertJsonValidationErrors('name'); // Only A-Za-z0-9_ are allowed option(['player_name_rule' => 'official']); - $this->postJson('/user/player/rename/'.$player->pid, ['name' => '角色名']) - ->assertJsonValidationErrors('name'); + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => '角色名'] + )->assertJsonValidationErrors('name'); // Other invalid characters option(['player_name_rule' => 'cjk']); - $this->postJson('/user/player/rename/'.$player->pid, ['name' => '\\']) - ->assertJsonValidationErrors('name'); + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => '\\'] + )->assertJsonValidationErrors('name'); // with an existed player name $existed = factory(Player::class)->create(); - $this->postJson('/user/player/rename/'.$player->pid, ['name' => $existed->name]) - ->assertJsonValidationErrors('name'); + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => $existed->name] + )->assertJsonValidationErrors('name'); // Rejected by filter $filter = Fakes\Filter::fake(); @@ -268,25 +274,29 @@ class PlayerControllerTest extends TestCase return new Rejection('rejected'); }); - $name = factory(Player::class)->create()->name; - $this->postJson('/user/player/rename/'.$player->pid, ['name' => 'new']) - ->assertJson([ - 'code' => 1, - 'message' => 'rejected', - ]); + factory(Player::class)->create()->name; + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => 'new'] + )->assertJson([ + 'code' => 1, + 'message' => 'rejected', + ]); $filter->remove('user_can_rename_player'); // Success Event::fake(); $pid = $player->pid; - $this->postJson('/user/player/rename/'.$pid, ['name' => 'new_name']) - ->assertJson([ - 'code' => 0, - 'message' => trans( - 'user.player.rename.success', - ['old' => $player->name, 'new' => 'new_name'] - ), - ]); + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => 'new_name'] + )->assertJson([ + 'code' => 0, + 'message' => trans( + 'user.player.rename.success', + ['old' => $player->name, 'new' => 'new_name'] + ), + ]); Event::assertDispatched(Events\PlayerProfileUpdated::class); Event::assertDispatched('player.renaming', function ($event, $payload) use ($pid) { [$player, $newName] = $payload; @@ -309,8 +319,10 @@ class PlayerControllerTest extends TestCase // Single player option(['single_player' => true]); - $this->postJson('/user/player/rename/'.$player->pid, ['name' => 'abc']) - ->assertJson(['code' => 0]); + $this->putJson( + route('user.player.rename', ['player' => $player]), + ['name' => 'abc'] + )->assertJson(['code' => 0]); $this->assertEquals('abc', $player->user->nickname); } @@ -331,12 +343,12 @@ class PlayerControllerTest extends TestCase return new Rejection('rejected'); }); $this->actingAs($user) - ->postJson('/user/player/set/'.$player->pid, ['skin' => -1]) + ->putJson(route('user.player.set', ['player' => $player]), ['skin' => -1]) ->assertJson(['code' => 1, 'message' => 'rejected']); + $filter->remove('can_set_texture'); // Set a not-existed texture - Fakes\Filter::fake(); - $this->postJson('/user/player/set/'.$player->pid, ['skin' => -1]) + $this->putJson(route('user.player.set', ['player' => $player]), ['skin' => -1]) ->assertJson([ 'code' => 1, 'message' => trans('skinlib.non-existent'), @@ -344,33 +356,43 @@ class PlayerControllerTest extends TestCase // Set for "skin" type Event::fake(); - $this->postJson('/user/player/set/'.$player->pid, ['skin' => $skin->tid]) - ->assertJson([ - 'code' => 0, - 'message' => trans('user.player.set.success', ['name' => $player->name]), - ]); + $this->putJson( + route('user.player.set', ['player' => $player]), + ['skin' => $skin->tid] + )->assertJson([ + 'code' => 0, + 'message' => trans('user.player.set.success', ['name' => $player->name]), + ]); $this->assertEquals($skin->tid, Player::find($player->pid)->tid_skin); - Event::assertDispatched('player.texture.updating', function ($event, $payload) use ($player, $skin) { - $this->assertEquals($player->pid, $payload[0]->pid); - $this->assertEquals($skin->tid, $payload[1]->tid); + Event::assertDispatched( + 'player.texture.updating', + function ($event, $payload) use ($player, $skin) { + $this->assertEquals($player->pid, $payload[0]->pid); + $this->assertEquals($skin->tid, $payload[1]->tid); - return true; - }); - Event::assertDispatched('player.texture.updated', function ($event, $payload) use ($player, $skin) { - $this->assertEquals($player->pid, $payload[0]->pid); - $this->assertEquals($skin->tid, $payload[0]->tid_skin); - $this->assertEquals($skin->tid, $payload[1]->tid); + return true; + } + ); + Event::assertDispatched( + 'player.texture.updated', + function ($event, $payload) use ($player, $skin) { + $this->assertEquals($player->pid, $payload[0]->pid); + $this->assertEquals($skin->tid, $payload[0]->tid_skin); + $this->assertEquals($skin->tid, $payload[1]->tid); - return true; - }); + return true; + } + ); // Set for "cape" type Event::fake(); - $this->postJson('/user/player/set/'.$player->pid, ['cape' => $cape->tid]) - ->assertJson([ - 'code' => 0, - 'message' => trans('user.player.set.success', ['name' => $player->name]), - ]); + $this->putJson( + route('user.player.set', ['player' => $player]), + ['cape' => $cape->tid] + )->assertJson([ + 'code' => 0, + 'message' => trans('user.player.set.success', ['name' => $player->name]), + ]); $this->assertEquals($cape->tid, Player::find($player->pid)->tid_cape); } @@ -394,15 +416,18 @@ class PlayerControllerTest extends TestCase return new Rejection('rejected'); }); $this->actingAs($user) - ->postJson('/user/player/texture/clear/'.$player->pid, ['skin' => true]) + ->deleteJson( + route('user.player.clear', ['player' => $player]), + ['skin' => true] + ) ->assertJson(['code' => 1, 'message' => 'rejected']); + $filter->remove('can_clear_texture'); // success - Fakes\Filter::fake(); - $this->postJson('/user/player/texture/clear/'.$player->pid, [ - 'skin' => true, // "1" stands for "true" + $this->deleteJson(route('user.player.clear', ['player' => $player]), [ + 'skin' => true, 'cape' => true, - 'nope' => true, // invalid texture type is acceptable + 'nope' => true, // invalid texture type is acceptable ])->assertJson([ 'code' => 0, 'message' => trans('user.player.clear.success', ['name' => $player->name]), @@ -412,8 +437,10 @@ class PlayerControllerTest extends TestCase Event::assertDispatched(Events\PlayerProfileUpdated::class); Event::fake(); - $this->postJson('/user/player/texture/clear/'.$player->pid, ['type' => ['skin']]) - ->assertJson(['code' => 0]); + $this->deleteJson( + route('user.player.clear', ['player' => $player]), + ['type' => ['skin']] + )->assertJson(['code' => 0]); Event::assertDispatched('player.texture.resetting', function ($event, $payload) use ($player) { $this->assertEquals($player->pid, $payload[0]->pid); $this->assertEquals('skin', $payload[1]); @@ -428,8 +455,10 @@ class PlayerControllerTest extends TestCase }); Event::fake(); - $this->postJson('/user/player/texture/clear/'.$player->pid, ['type' => ['cape']]) - ->assertJson(['code' => 0]); + $this->deleteJson( + route('user.player.clear', ['player' => $player]), + ['type' => ['cape']] + )->assertJson(['code' => 0]); Event::assertDispatched('player.texture.resetting', function ($event, $payload) use ($player) { $this->assertEquals($player->pid, $payload[0]->pid); $this->assertEquals('cape', $payload[1]); diff --git a/tests/HttpTest/MiddlewareTest/CheckPlayerExistTest.php b/tests/HttpTest/MiddlewareTest/CheckPlayerExistTest.php deleted file mode 100644 index c5d43ca9..00000000 --- a/tests/HttpTest/MiddlewareTest/CheckPlayerExistTest.php +++ /dev/null @@ -1,32 +0,0 @@ -getJson('/nope.json')->assertStatus(404); - - $player = factory(Player::class)->create(); - $this->getJson("/{$player->name}.json") - ->assertJson(['username' => $player->name]); // Default is CSL API - - $player = factory(Player::class)->create(); - $user = $player->user; - $this->actingAs($user) - ->postJson('/user/player/rename/-1', ['name' => 'name']) - ->assertJson([ - 'code' => 1, - 'message' => trans('general.unexistent-player'), - ]); - } -}