From 122477c5c33f03516eb1962de5b18965f4347ba3 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Wed, 24 Apr 2019 13:10:03 +0800 Subject: [PATCH] Add RESTful APIs about players --- app/Http/Controllers/PlayerController.php | 83 ++++++------- app/Http/Middleware/CheckPlayerExist.php | 6 +- app/Http/Middleware/CheckPlayerOwner.php | 17 +-- .../src/components/ApplyToPlayerDialog.vue | 9 +- resources/assets/src/views/user/Players.vue | 13 +-- .../components/ApplyToPlayerDialog.test.ts | 18 +-- .../assets/tests/views/user/Players.test.ts | 10 +- routes/api.php | 7 +- routes/web.php | 8 +- tests/MiddlewareTest.php | 26 +---- tests/PlayerControllerTest.php | 109 +++++++----------- 11 files changed, 119 insertions(+), 187 deletions(-) diff --git a/app/Http/Controllers/PlayerController.php b/app/Http/Controllers/PlayerController.php index 8b527141..6693b055 100644 --- a/app/Http/Controllers/PlayerController.php +++ b/app/Http/Controllers/PlayerController.php @@ -20,23 +20,8 @@ use App\Http\Middleware\CheckPlayerOwner; class PlayerController extends Controller { - /** - * Player Instance. - * - * @var \App\Models\Player - */ - private $player; - public function __construct() { - $this->middleware(function ($request, $next) { - if ($request->has('pid')) { - $this->player = Player::find($request->pid); - } - - return $next($request); - }); - $this->middleware([CheckPlayerExist::class, CheckPlayerOwner::class], [ 'only' => ['delete', 'rename', 'setTexture', 'clearTexture'], ]); @@ -102,20 +87,21 @@ class PlayerController extends Controller $user->setScore(option('score_per_player'), 'minus'); - return json(trans('user.player.add.success', ['name' => $name]), 0); + return json(trans('user.player.add.success', ['name' => $name]), 0, $player->toArray()); } - public function delete() + public function delete($pid) { - $playerName = $this->player->name; + $player = Player::find($pid); + $playerName = $player->name; if (option('single_player', false)) { return json(trans('user.player.delete.single'), 1); } - event(new PlayerWillBeDeleted($this->player)); + event(new PlayerWillBeDeleted($player)); - $this->player->delete(); + $player->delete(); if (option('return_score')) { Auth::user()->setScore(Option::get('score_per_player'), 'plus'); @@ -126,21 +112,20 @@ class PlayerController extends Controller return json(trans('user.player.delete.success', ['name' => $playerName]), 0); } - public function rename(Request $request) + public function rename(Request $request, $pid) { - $this->validate($request, [ - 'new_player_name' => 'required|player_name|min:'.option('player_name_length_min').'|max:'.option('player_name_length_max'), - ]); - - $newName = $request->input('new_player_name'); + $newName = $this->validate($request, [ + 'name' => 'required|player_name|min:'.option('player_name_length_min').'|max:'.option('player_name_length_max'), + ])['name']; + $player = Player::find($pid); if (! Player::where('name', $newName)->get()->isEmpty()) { return json(trans('user.player.rename.repeated'), 6); } - $oldName = $this->player->name; - $this->player->name = $newName; - $this->player->save(); + $oldName = $player->name; + $player->name = $newName; + $player->save(); if (option('single_player', false)) { $user = auth()->user(); @@ -148,38 +133,40 @@ class PlayerController extends Controller $user->save(); } - return json(trans('user.player.rename.success', ['old' => $oldName, 'new' => $newName]), 0); + return json(trans('user.player.rename.success', ['old' => $oldName, 'new' => $newName]), 0, $player->toArray()); } - public function setTexture(Request $request) + public function setTexture(Request $request, $pid) { - foreach ($request->input('tid') as $key => $value) { - $texture = Texture::find($value); + $player = Player::find($pid); + foreach (['skin', 'cape'] as $type) { + if ($tid = $request->input($type)) { + $texture = Texture::find($tid); + if (! $texture) { + return json(trans('skinlib.non-existent'), 1); + } - if (! $texture) { - return json(trans('skinlib.un-existent'), 6); + $field = "tid_$type"; + $player->$field = $tid; + $player->save(); } - - $field = $texture->type == 'cape' ? 'tid_cape' : 'tid_skin'; - - $this->player->$field = $value; - $this->player->save(); } - return json(trans('user.player.set.success', ['name' => $this->player->name]), 0); + return json(trans('user.player.set.success', ['name' => $player->name]), 0, $player->toArray()); } - public function clearTexture(Request $request) + public function clearTexture(Request $request, $pid) { - array_map(function ($type) use ($request) { - if ($request->input($type)) { + $player = Player::find($pid); + array_map(function ($type) use ($request, $player) { + if ($request->has($type)) { $field = "tid_$type"; - $this->player->$field = 0; + $player->$field = 0; } - }, ['skin', 'cape']); - $this->player->save(); + }, $request->input('type') ?? ['skin', 'cape']); + $player->save(); - return json(trans('user.player.clear.success', ['name' => $this->player->name]), 0); + return json(trans('user.player.clear.success', ['name' => $player->name]), 0, $player->toArray()); } public function bind(Request $request) diff --git a/app/Http/Middleware/CheckPlayerExist.php b/app/Http/Middleware/CheckPlayerExist.php index 8c5583a5..d6351fd4 100644 --- a/app/Http/Middleware/CheckPlayerExist.php +++ b/app/Http/Middleware/CheckPlayerExist.php @@ -4,14 +4,16 @@ namespace App\Http\Middleware; use Event; use App\Models\Player; +use Illuminate\Support\Arr; use App\Events\CheckPlayerExists; class CheckPlayerExist { public function handle($request, \Closure $next) { - if ($request->has('pid') && $request->isMethod('post')) { - if (is_null(Player::find($request->input('pid')))) { + $pid = Arr::get($request->route()->parameters, 'pid') ?? $request->input('pid'); + if (! $request->isMethod('get') && ! is_null($pid)) { + if (is_null(Player::find($pid))) { return json(trans('general.unexistent-player'), 1); } else { return $next($request); diff --git a/app/Http/Middleware/CheckPlayerOwner.php b/app/Http/Middleware/CheckPlayerOwner.php index 6b8a05a5..5ab7a330 100644 --- a/app/Http/Middleware/CheckPlayerOwner.php +++ b/app/Http/Middleware/CheckPlayerOwner.php @@ -4,24 +4,15 @@ namespace App\Http\Middleware; use Closure; use App\Models\Player; +use Illuminate\Support\Arr; class CheckPlayerOwner { - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed - */ public function handle($request, Closure $next) { - if ($pid = $request->input('pid')) { - $player = Player::find($pid); - - if ($player->uid != auth()->id()) { - return json(trans('admin.players.no-permission'), 1); - } + $pid = Arr::get($request->route()->parameters, 'pid') ?? $request->input('pid'); + if ($pid && ($player = Player::find($pid)) && $player->uid != auth()->id()) { + return json(trans('admin.players.no-permission'), 1); } return $next($request); diff --git a/resources/assets/src/components/ApplyToPlayerDialog.vue b/resources/assets/src/components/ApplyToPlayerDialog.vue index 01fde109..b9fb3f6b 100644 --- a/resources/assets/src/components/ApplyToPlayerDialog.vue +++ b/resources/assets/src/components/ApplyToPlayerDialog.vue @@ -83,13 +83,10 @@ export default { } const { code, message } = await this.$http.post( - '/user/player/set', + `/user/player/set/${this.selected}`, { - pid: this.selected, - tid: { - skin: this.skin || undefined, - cape: this.cape || undefined, - }, + skin: this.skin || undefined, + cape: this.cape || undefined, } ) if (code === 0) { diff --git a/resources/assets/src/views/user/Players.vue b/resources/assets/src/views/user/Players.vue index 129288f2..aeec8d5d 100644 --- a/resources/assets/src/views/user/Players.vue +++ b/resources/assets/src/views/user/Players.vue @@ -231,8 +231,8 @@ export default { } const { code, message } = await this.$http.post( - '/user/player/rename', - { pid: player.pid, new_player_name: value } + `/user/player/rename/${player.pid}`, + { name: value } ) if (code === 0) { this.$message.success(message) @@ -247,8 +247,8 @@ export default { } const { code, message } = await this.$http.post( - '/user/player/texture/clear', - Object.assign({ pid: this.selected }, this.clear) + `/user/player/texture/clear/${this.selected}`, + this.clear ) if (code === 0) { $('.modal').modal('hide') @@ -271,10 +271,7 @@ export default { return } - const { code, message } = await this.$http.post( - '/user/player/delete', - { pid: player.pid } - ) + const { code, message } = await this.$http.post(`/user/player/delete/${player.pid}`) if (code === 0) { this.$delete(this.players, index) this.$message.success(message) diff --git a/resources/assets/tests/components/ApplyToPlayerDialog.test.ts b/resources/assets/tests/components/ApplyToPlayerDialog.test.ts index e8ff5b2c..698d58ef 100644 --- a/resources/assets/tests/components/ApplyToPlayerDialog.test.ts +++ b/resources/assets/tests/components/ApplyToPlayerDialog.test.ts @@ -20,25 +20,19 @@ test('submit applying texture', async () => { wrapper.setProps({ skin: 1 }) button.trigger('click') expect(Vue.prototype.$http.post).toBeCalledWith( - '/user/player/set', + '/user/player/set/1', { - pid: 1, - tid: { - skin: 1, - cape: undefined, - }, + skin: 1, + cape: undefined, } ) wrapper.setProps({ skin: 0, cape: 1 }) button.trigger('click') expect(Vue.prototype.$http.post).toBeCalledWith( - '/user/player/set', + '/user/player/set/1', { - pid: 1, - tid: { - skin: undefined, - cape: 1, - }, + skin: undefined, + cape: 1, } ) await wrapper.vm.$nextTick() diff --git a/resources/assets/tests/views/user/Players.test.ts b/resources/assets/tests/views/user/Players.test.ts index 7cbb2958..8c3793fc 100644 --- a/resources/assets/tests/views/user/Players.test.ts +++ b/resources/assets/tests/views/user/Players.test.ts @@ -98,8 +98,8 @@ test('change player name', async () => { button.trigger('click') await flushPromises() expect(Vue.prototype.$http.post).toBeCalledWith( - '/user/player/rename', - { pid: 1, new_player_name: 'new-name' } + '/user/player/rename/1', + { name: 'new-name' } ) button.trigger('click') await flushPromises() @@ -127,6 +127,8 @@ test('delete player', async () => { expect(Vue.prototype.$http.post).not.toBeCalled() button.trigger('click') + await flushPromises() + expect(Vue.prototype.$http.post).toBeCalledWith('/user/player/delete/1') expect(wrapper.text()).toContain('to-be-deleted') button.trigger('click') @@ -178,9 +180,9 @@ test('clear texture', async () => { .setChecked() button.trigger('click') expect(Vue.prototype.$http.post).toBeCalledWith( - '/user/player/texture/clear', + '/user/player/texture/clear/1', { - pid: 1, skin: true, cape: false, + skin: true, cape: false, } ) button.trigger('click') diff --git a/routes/api.php b/routes/api.php index e4e4d081..e827b78f 100644 --- a/routes/api.php +++ b/routes/api.php @@ -9,6 +9,11 @@ Route::prefix('auth')->group(function ($route) { Route::prefix('user')->middleware('auth.jwt')->group(function ($route) { $route->put('sign', 'UserController@sign'); - $route->post('player', 'PlayerController@add'); + $route->get('players', 'PlayerController@listAll'); + $route->post('players', 'PlayerController@add'); + $route->delete('players/{pid}', 'PlayerController@delete'); + $route->put('players/{pid}/name', 'PlayerController@rename'); + $route->put('players/{pid}/textures', 'PlayerController@setTexture'); + $route->delete('players/{pid}/textures', 'PlayerController@clearTexture'); }); diff --git a/routes/web.php b/routes/web.php index 2a5756d2..7d530e6a 100644 --- a/routes/web.php +++ b/routes/web.php @@ -65,10 +65,10 @@ Route::group([ Route::any('', 'PlayerController@index'); Route::get('/list', 'PlayerController@listAll'); Route::post('/add', 'PlayerController@add'); - Route::post('/set', 'PlayerController@setTexture'); - Route::post('/texture/clear', 'PlayerController@clearTexture'); - Route::post('/rename', 'PlayerController@rename'); - Route::post('/delete', 'PlayerController@delete'); + Route::post('/set/{pid}', 'PlayerController@setTexture'); + Route::post('/texture/clear/{pid}', 'PlayerController@clearTexture'); + Route::post('/rename/{pid}', 'PlayerController@rename'); + Route::post('/delete/{pid}', 'PlayerController@delete'); Route::view('/bind', 'user.bind'); Route::post('/bind', 'PlayerController@bind'); }); diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index b00df88f..5f0c6c18 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -143,20 +143,11 @@ class MiddlewareTest extends TestCase $player = factory(\App\Models\Player::class)->create(); $user = $player->user; $this->actingAs($user) - ->postJson('/user/player/rename', [ - 'pid' => -1, - 'new_player_name' => 'name', - ])->assertJson([ + ->postJson('/user/player/rename/-1', ['name' => 'name']) + ->assertJson([ 'code' => 1, 'message' => trans('general.unexistent-player'), ]); - $this->actingAs($user) - ->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => 'name', - ])->assertJson([ - 'code' => 0, - ]); } public function testCheckPlayerOwner() @@ -170,20 +161,11 @@ class MiddlewareTest extends TestCase ->assertSuccessful(); $this->actingAs($other_user) - ->postJson('/user/player/rename', [ - 'pid' => $player->pid, - ])->assertJson([ + ->postJson('/user/player/rename/'.$player->pid) + ->assertJson([ 'code' => 1, 'message' => trans('admin.players.no-permission'), ]); - - $this->actingAs($owner) - ->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => 'name', - ])->assertJson([ - 'code' => 0, - ]); } public function testRedirectIfAuthenticated() diff --git a/tests/PlayerControllerTest.php b/tests/PlayerControllerTest.php index a7842b40..2286abf6 100644 --- a/tests/PlayerControllerTest.php +++ b/tests/PlayerControllerTest.php @@ -114,7 +114,7 @@ class PlayerControllerTest extends TestCase $score = $user->score; $this->expectsEvents(Events\PlayerWillBeDeleted::class); $this->actingAs($user) - ->postJson('/user/player/delete', ['pid' => $player->pid]) + ->postJson('/user/player/delete/'.$player->pid) ->assertJson([ 'code' => 0, 'message' => trans('user.player.delete.success', ['name' => $player->name]), @@ -131,7 +131,7 @@ class PlayerControllerTest extends TestCase $player = factory(Player::class)->create(); $user = $player->user; $this->actingAs($user) - ->postJson('/user/player/delete', ['pid' => $player->pid]) + ->postJson('/user/player/delete/'.$player->pid) ->assertJson([ 'code' => 0, 'message' => trans('user.player.delete.success', ['name' => $player->name]), @@ -145,7 +145,7 @@ class PlayerControllerTest extends TestCase option(['single_player' => true]); $player = factory(Player::class)->create(['uid' => $user->uid]); $this->actingAs($user) - ->postJson('/user/player/delete', ['pid' => $player->pid]) + ->postJson('/user/player/delete/'.$player->pid) ->assertJson([ 'code' => 1, 'message' => trans('user.player.delete.single'), @@ -161,54 +161,42 @@ class PlayerControllerTest extends TestCase // Without new player name $this->actingAs($user) - ->postJson('/user/player/rename', [ - 'pid' => $player->pid, - ]) - ->assertJsonValidationErrors('new_player_name'); + ->postJson('/user/player/rename/'.$player->pid) + ->assertJsonValidationErrors('name'); // Only A-Za-z0-9_ are allowed option(['player_name_rule' => 'official']); - $this->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => '角色名', - ])->assertJsonValidationErrors('new_player_name'); + $this->postJson('/user/player/rename/'.$player->pid, ['name' => '角色名']) + ->assertJsonValidationErrors('name'); // Other invalid characters option(['player_name_rule' => 'cjk']); - $this->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => '\\', - ])->assertJsonValidationErrors('new_player_name'); + $this->postJson('/user/player/rename/'.$player->pid, ['name' => '\\']) + ->assertJsonValidationErrors('name'); // Use a duplicated player name $name = factory(Player::class)->create()->name; - $this->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => $name, - ])->assertJson([ - 'code' => 6, - 'message' => trans('user.player.rename.repeated'), - ]); + $this->postJson('/user/player/rename/'.$player->pid, ['name' => $name]) + ->assertJson([ + 'code' => 6, + 'message' => trans('user.player.rename.repeated'), + ]); // Success - $this->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => 'new_name', - ])->assertJson([ - 'code' => 0, - 'message' => trans( - 'user.player.rename.success', - ['old' => $player->name, 'new' => 'new_name'] - ), - ]); + $this->postJson('/user/player/rename/'.$player->pid, ['name' => 'new_name']) + ->assertJson([ + 'code' => 0, + 'message' => trans( + 'user.player.rename.success', + ['old' => $player->name, 'new' => 'new_name'] + ), + ]); Event::assertDispatched(Events\PlayerProfileUpdated::class); // Single player option(['single_player' => true]); - $this->postJson('/user/player/rename', [ - 'pid' => $player->pid, - 'new_player_name' => 'abc', - ])->assertJson(['code' => 0]); + $this->postJson('/user/player/rename/'.$player->pid, ['name' => 'abc']) + ->assertJson(['code' => 0]); $this->assertEquals('abc', $player->user->nickname); } @@ -221,42 +209,27 @@ class PlayerControllerTest extends TestCase // Set a not-existed texture $this->actingAs($user) - ->postJson('/user/player/set', [ - 'pid' => $player->pid, - 'tid' => ['skin' => -1], - ])->assertJson([ - 'code' => 6, - 'message' => trans('skinlib.un-existent'), + ->postJson('/user/player/set/'.$player->pid, ['skin' => -1]) + ->assertJson([ + 'code' => 1, + 'message' => trans('skinlib.non-existent'), ]); // Set for "skin" type - $this->postJson('/user/player/set', [ - 'pid' => $player->pid, - 'tid' => ['skin' => $skin->tid], - ])->assertJson([ - 'code' => 0, - 'message' => trans('user.player.set.success', ['name' => $player->name]), - ]); + $this->postJson('/user/player/set/'.$player->pid, ['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); // Set for "cape" type - $this->postJson('/user/player/set', [ - 'pid' => $player->pid, - 'tid' => ['cape' => $cape->tid], - ])->assertJson([ - 'code' => 0, - 'message' => trans('user.player.set.success', ['name' => $player->name]), - ]); + $this->postJson('/user/player/set/'.$player->pid, ['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); - - // Invalid texture type is acceptable - $this->postJson('/user/player/set', [ - 'pid' => $player->pid, - 'tid' => ['nope' => $skin->tid], // TID must be valid - ])->assertJson([ - 'code' => 0, - 'message' => trans('user.player.set.success', ['name' => $player->name]), - ]); } public function testClearTexture() @@ -271,8 +244,7 @@ class PlayerControllerTest extends TestCase $player->refresh(); $this->actingAs($user) - ->postJson('/user/player/texture/clear', [ - 'pid' => $player->pid, + ->postJson('/user/player/texture/clear/'.$player->pid, [ 'skin' => 1, // "1" stands for "true" 'cape' => 1, 'nope' => 1, // Invalid texture type is acceptable @@ -283,6 +255,9 @@ class PlayerControllerTest extends TestCase $this->assertEquals(0, Player::find($player->pid)->tid_skin); $this->assertEquals(0, Player::find($player->pid)->tid_cape); Event::assertDispatched(Events\PlayerProfileUpdated::class); + + $this->postJson('/user/player/texture/clear/'.$player->pid, ['type' => ['skin']]) + ->assertJson(['code' => 0]); } public function testBind()