diff --git a/app/Http/Controllers/PlayerController.php b/app/Http/Controllers/PlayerController.php index a474c668..832b4a39 100644 --- a/app/Http/Controllers/PlayerController.php +++ b/app/Http/Controllers/PlayerController.php @@ -10,15 +10,13 @@ use App\Http\Middleware\CheckPlayerExist; use App\Http\Middleware\CheckPlayerOwner; use App\Models\Player; use App\Models\Texture; +use App\Models\User; use App\Rules; use Auth; use Blessing\Filter; use Blessing\Rejection; -use Event; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Http\Request; -use Option; -use View; class PlayerController extends Controller { @@ -65,8 +63,9 @@ class PlayerController extends Controller return Auth::user()->players; } - public function add(Request $request, Dispatcher $dispatcher) + public function add(Request $request, Dispatcher $dispatcher, Filter $filter) { + /** @var User */ $user = Auth::user(); if (option('single_player', false)) { @@ -81,14 +80,20 @@ class PlayerController extends Controller 'max:'.option('player_name_length_max'), ], ])['name']; + $name = $filter->apply('new_player_name', $name); $dispatcher->dispatch('player.add.attempt', [$name, $user]); - if (!Player::where('name', $name)->get()->isEmpty()) { + $can = $filter->apply('can_add_player', true, [$name]); + if ($can instanceof Rejection) { + return json($can->getReason(), 1); + } + + if (Player::where('name', $name)->count() > 0) { return json(trans('user.player.add.repeated'), 6); } - if ($user->score < Option::get('score_per_player')) { + if ($user->score < (int) option('score_per_player')) { return json(trans('user.player.add.lack-score'), 7); } @@ -102,7 +107,7 @@ class PlayerController extends Controller $player->tid_cape = 0; $player->save(); - $user->score -= option('score_per_player'); + $user->score -= (int) option('score_per_player'); $user->save(); $dispatcher->dispatch('player.added', [$player, $user]); @@ -111,14 +116,20 @@ class PlayerController extends Controller return json(trans('user.player.add.success', ['name' => $name]), 0, $player->toArray()); } - public function delete(Dispatcher $dispatcher, $pid) + public function delete(Dispatcher $dispatcher, Filter $filter, $pid) { + /** @var User */ $user = auth()->user(); $player = Player::find($pid); $playerName = $player->name; $dispatcher->dispatch('player.delete.attempt', [$player, $user]); + $can = $filter->apply('can_delete_player', true, [$player]); + if ($can instanceof Rejection) { + return json($can->getReason(), 1); + } + if (option('single_player', false)) { return json(trans('user.player.delete.single'), 1); } @@ -129,7 +140,7 @@ class PlayerController extends Controller $player->delete(); if (option('return_score')) { - $user->score += option('score_per_player'); + $user->score += (int) option('score_per_player'); $user->save(); } @@ -145,7 +156,7 @@ class PlayerController extends Controller Filter $filter, $pid ) { - $newName = $request->validate([ + $name = $request->validate([ 'name' => [ 'required', new Rules\PlayerName(), @@ -153,39 +164,55 @@ class PlayerController extends Controller 'max:'.option('player_name_length_max'), ], ])['name']; + $name = $filter->apply('new_player_name', $name); $player = Player::find($pid); - $dispatcher->dispatch('player.renaming', [$player, $newName]); + $dispatcher->dispatch('player.renaming', [$player, $name]); - $can = $filter->apply('user_can_rename_player', true, [$player, $newName]); + $can = $filter->apply('user_can_rename_player', true, [$player, $name]); if ($can instanceof Rejection) { return json($can->getReason(), 1); } - if (!Player::where('name', $newName)->get()->isEmpty()) { + if (Player::where('name', $name)->count() > 0) { return json(trans('user.player.rename.repeated'), 6); } - $oldName = $player->name; - $player->name = $newName; + $old = $player->replicate(); + $player->name = $name; $player->save(); if (option('single_player', false)) { + /** @var User */ $user = auth()->user(); - $user->nickname = $newName; + $user->nickname = $name; $user->save(); } - $dispatcher->dispatch('player.renamed', [$player, $oldName]); + $dispatcher->dispatch('player.renamed', [$player, $old]); - return json(trans('user.player.rename.success', ['old' => $oldName, 'new' => $newName]), 0, $player->toArray()); + return json( + trans('user.player.rename.success', ['old' => $old->name, 'new' => $name]), + 0, + $player->toArray() + ); } - public function setTexture(Request $request, Dispatcher $dispatcher, $pid) - { + public function setTexture( + Request $request, + Dispatcher $dispatcher, + Filter $filter, + $pid + ) { $player = Player::find($pid); foreach (['skin', 'cape'] as $type) { $tid = $request->input($type); + + $can = $filter->apply('can_set_texture', true, [$player, $type, $tid]); + if ($can instanceof Rejection) { + return json($can->getReason(), 1); + } + if ($tid) { $texture = Texture::find($tid); if (!$texture) { @@ -205,12 +232,21 @@ class PlayerController extends Controller return json(trans('user.player.set.success', ['name' => $player->name]), 0, $player->toArray()); } - public function clearTexture(Request $request, Dispatcher $dispatcher, $pid) - { + public function clearTexture( + Request $request, + Dispatcher $dispatcher, + Filter $filter, + $pid + ) { $player = Player::find($pid); $types = $request->input('type', []); foreach (['skin', 'cape'] as $type) { + $can = $filter->apply('can_clear_texture', true, [$player, $type]); + if ($can instanceof Rejection) { + return json($can->getReason(), 1); + } + if ($request->has($type) || in_array($type, $types)) { $dispatcher->dispatch('player.texture.resetting', [$player, $type]); @@ -235,10 +271,11 @@ class PlayerController extends Controller 'max:'.option('player_name_length_max'), ], ])['player']; + /** @var User */ $user = Auth::user(); $player = Player::where('name', $name)->first(); - if (!$player) { + if (empty($player)) { $dispatcher->dispatch('player.adding', [$name, $user]); event(new PlayerWillBeAdded($name)); diff --git a/tests/HttpTest/ControllersTest/PlayerControllerTest.php b/tests/HttpTest/ControllersTest/PlayerControllerTest.php index 8de0b4e3..090d58c9 100644 --- a/tests/HttpTest/ControllersTest/PlayerControllerTest.php +++ b/tests/HttpTest/ControllersTest/PlayerControllerTest.php @@ -6,7 +6,6 @@ use App\Events; use App\Models\Player; use App\Models\Texture; use App\Models\User; -use Blessing\Filter; use Blessing\Rejection; use Event; use Illuminate\Foundation\Testing\DatabaseTransactions; @@ -41,6 +40,7 @@ class PlayerControllerTest extends TestCase public function testAdd() { Event::fake(); + $filter = Fakes\Filter::fake(); // Without player name $this->postJson('/user/player/add')->assertJsonValidationErrors('name'); @@ -54,7 +54,7 @@ class PlayerControllerTest extends TestCase // Custom player name rule (regexp) option(['player_name_rule' => 'custom']); - option(['custom_player_name_regexp' => '/^([0-9]+)$/']); + option(['custom_player_name_regexp' => '/^\d+$/']); $this->postJson( '/user/player/add', ['name' => 'yjsnpi'] @@ -70,6 +70,11 @@ class PlayerControllerTest extends TestCase 'code' => 7, 'message' => trans('user.player.add.lack-score'), ]); + $filter->assertApplied('new_player_name', function ($name) { + $this->assertEquals('no_score', $name); + + return true; + }); Event::assertDispatched('player.add.attempt', function ($event, $payload) use ($user) { $this->assertEquals('no_score', $payload[0]); $this->assertEquals($user->uid, $payload[1]->uid); @@ -79,8 +84,24 @@ class PlayerControllerTest extends TestCase Event::assertNotDispatched('player.adding'); Event::assertNotDispatched('player.added'); + // rejected + Event::fake(); + $filter->add('can_add_player', function ($can, $name) { + $this->assertEquals('can', $name); + + return new Rejection('rejected'); + }); + $this->postJson( + '/user/player/add', + ['name' => 'can'] + )->assertJson(['code' => 1, 'message' => 'rejected']); + Event::assertDispatched('player.add.attempt'); + Event::assertNotDispatched('player.adding'); + Event::assertNotDispatched('player.added'); + // Allowed to use CJK characters Event::fake(); + Fakes\Filter::fake(); option(['player_name_rule' => 'cjk']); $user = factory(User::class)->create(); $score = $user->score; @@ -141,12 +162,25 @@ class PlayerControllerTest extends TestCase public function testDelete() { Event::fake(); + $filter = Fakes\Filter::fake(); $user = factory(User::class)->create(); $player = factory(Player::class)->create(['uid' => $user->uid]); $score = $user->score; + + // rejected + $filter->add('can_delete_player', function ($can, $p) use ($player) { + $this->assertTrue($player->is($p)); + + return new Rejection('rejected'); + }); $this->actingAs($user) ->postJson('/user/player/delete/'.$player->pid) + ->assertJson(['code' => 1, 'message' => 'rejected']); + + // success + $filter = Fakes\Filter::fake(); + $this->postJson('/user/player/delete/'.$player->pid) ->assertJson([ 'code' => 0, 'message' => trans('user.player.delete.success', ['name' => $player->name]), @@ -207,6 +241,7 @@ class PlayerControllerTest extends TestCase public function testRename() { Event::fake(); + $filter = Fakes\Filter::fake(); $player = factory(Player::class)->create(); $user = $player->user; @@ -233,12 +268,17 @@ class PlayerControllerTest extends TestCase 'message' => trans('user.player.rename.repeated'), ]); Event::assertDispatched('player.renaming'); + $filter->assertApplied('new_player_name', function ($newName) use ($name) { + $this->assertEquals($name, $newName); + + return true; + }); // Rejected by filter - $filter = resolve(Filter::class); - $pid = $player->pid; - $filter->add('user_can_rename_player', function ($can, $player, $newName) { - $this->assertEquals('new', $newName); + $filter = Fakes\Filter::fake(); + $filter->add('user_can_rename_player', function ($can, $p, $name) use ($player) { + $this->assertTrue($player->is($p)); + $this->assertEquals('new', $name); return new Rejection('rejected'); }); @@ -252,6 +292,7 @@ class PlayerControllerTest extends TestCase // Success Event::fake(); + $pid = $player->pid; $this->postJson('/user/player/rename/'.$pid, ['name' => 'new_name']) ->assertJson([ 'code' => 0, @@ -268,10 +309,9 @@ class PlayerControllerTest extends TestCase return true; }); - Event::assertDispatched('player.renamed', function ($event, $payload) use ($pid) { - [$player, $oldName] = $payload; - $this->assertEquals($pid, $player->pid); - $this->assertNotEquals('new_name', $oldName); + Event::assertDispatched('player.renamed', function ($event, $payload) use ($player) { + $this->assertTrue($player->fresh()->is($payload[0])); + $this->assertNotEquals('new_name', $payload[1]->name); return true; }); @@ -290,9 +330,22 @@ class PlayerControllerTest extends TestCase $skin = factory(Texture::class)->create(); $cape = factory(Texture::class)->states('cape')->create(); - // Set a not-existed texture + // rejected + $filter = Fakes\Filter::fake(); + $filter->add('can_set_texture', function ($can, $p, $type, $tid) use ($player) { + $this->assertTrue($player->is($p)); + $this->assertEquals('skin', $type); + $this->assertEquals(-1, $tid); + + return new Rejection('rejected'); + }); $this->actingAs($user) ->postJson('/user/player/set/'.$player->pid, ['skin' => -1]) + ->assertJson(['code' => 1, 'message' => 'rejected']); + + // Set a not-existed texture + Fakes\Filter::fake(); + $this->postJson('/user/player/set/'.$player->pid, ['skin' => -1]) ->assertJson([ 'code' => 1, 'message' => trans('skinlib.non-existent'), @@ -341,11 +394,24 @@ class PlayerControllerTest extends TestCase $player->save(); $player->refresh(); + // rejected + $filter = Fakes\Filter::fake(); + $filter->add('can_clear_texture', function ($can, $p, $type) use ($player) { + $this->assertTrue($player->is($p)); + $this->assertEquals('skin', $type); + + return new Rejection('rejected'); + }); $this->actingAs($user) - ->postJson('/user/player/texture/clear/'.$player->pid, [ - 'skin' => 1, // "1" stands for "true" - 'cape' => 1, - 'nope' => 1, // Invalid texture type is acceptable + ->postJson('/user/player/texture/clear/'.$player->pid, ['skin' => true]) + ->assertJson(['code' => 1, 'message' => 'rejected']); + + // success + Fakes\Filter::fake(); + $this->postJson('/user/player/texture/clear/'.$player->pid, [ + 'skin' => true, // "1" stands for "true" + 'cape' => true, + 'nope' => true, // invalid texture type is acceptable ])->assertJson([ 'code' => 0, 'message' => trans('user.player.clear.success', ['name' => $player->name]),