From b3d98fab03fff400d22fa683500e2ea3805d6251 Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 1 Nov 2023 16:16:23 +0900 Subject: [PATCH 1/2] Move score link complete function to static method It makes more sense to have model creation as part of it and prevents "completing" existing one. --- .../Rooms/Playlist/ScoresController.php | 9 +-- app/Models/Multiplayer/Room.php | 8 +-- app/Models/Multiplayer/ScoreLink.php | 66 ++++++++++--------- tests/Models/Multiplayer/ScoreLinkTest.php | 52 ++++++--------- 4 files changed, 60 insertions(+), 75 deletions(-) diff --git a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php index 601d5f20018..23bd4842a72 100644 --- a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php +++ b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php @@ -198,14 +198,7 @@ public function update($roomId, $playlistItemId, $tokenId) $params = Score::extractParams(\Request::all(), $scoreToken); - $scoreLink = $scoreToken - ->playlistItem - ->scoreLinks() - ->make(['user_id' => $scoreToken->user_id]); - $room->completePlay($scoreLink, $params); - $scoreToken->fill(['score_id' => $scoreLink->score_id])->saveOrExplode(); - - return $scoreLink; + return $room->completePlay($scoreToken, $params); }); $score = $scoreLink->score; diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index 6e7624cc3ce..3029ad3cae5 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -400,14 +400,14 @@ public function calculateMissingTopScores() } } - public function completePlay(ScoreLink $scoreLink, array $params) + public function completePlay(ScoreToken $scoreToken, array $params): ScoreLink { - priv_check_user($scoreLink->user, 'MultiplayerScoreSubmit')->ensureCan(); + priv_check_user($scoreToken->user, 'MultiplayerScoreSubmit')->ensureCan(); $this->assertValidCompletePlay(); - return $scoreLink->getConnection()->transaction(function () use ($params, $scoreLink) { - $scoreLink->complete($params); + return $this->getConnection()->transaction(function () use ($params, $scoreToken) { + $scoreLink = ScoreLink::complete($scoreToken, $params); UserScoreAggregate::new($scoreLink->user, $this)->addScoreLink($scoreLink); return $scoreLink; diff --git a/app/Models/Multiplayer/ScoreLink.php b/app/Models/Multiplayer/ScoreLink.php index 3ca353a62ed..532e7f316a2 100644 --- a/app/Models/Multiplayer/ScoreLink.php +++ b/app/Models/Multiplayer/ScoreLink.php @@ -9,6 +9,7 @@ use App\Exceptions\InvariantException; use App\Models\Model; +use App\Models\ScoreToken; use App\Models\Solo\Score; use App\Models\User; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -24,6 +25,39 @@ */ class ScoreLink extends Model { + public static function complete(ScoreToken $token, array $params): static + { + return \DB::transaction(function () use ($params, $token) { + $score = Score::createFromJsonOrExplode($params); + + $playlistItem = $token->playlistItem; + $requiredMods = array_column($playlistItem->required_mods, 'acronym'); + $mods = array_column($score->data->mods, 'acronym'); + if (!empty($requiredMods)) { + if (!empty(array_diff($requiredMods, $mods))) { + throw new InvariantException('This play does not include the mods required.'); + } + } + + $allowedMods = array_column($playlistItem->allowed_mods, 'acronym'); + if (!empty(array_diff($mods, $requiredMods, $allowedMods))) { + throw new InvariantException('This play includes mods that are not allowed.'); + } + + $scoreId = $score->getKey(); + $token->fill(['score_id' => $scoreId])->saveOrExplode(); + + $ret = new static([ + 'playlist_item_id' => $playlistItem->getKey(), + 'score_id' => $scoreId, + 'user_id' => $score->user_id, + ]); + $ret->saveOrExplode(); + + return $ret; + }); + } + public $incrementing = false; public $timestamps = false; @@ -64,38 +98,6 @@ public function getAttribute($key) }; } - public function complete(array $params): void - { - $this->getConnection()->transaction(function () use ($params) { - $score = Score::createFromJsonOrExplode($params); - $mods = $score->data->mods; - - if (!empty($this->playlistItem->required_mods)) { - $missingMods = array_diff( - array_column($this->playlistItem->required_mods, 'acronym'), - array_column($mods, 'acronym') - ); - - if (!empty($missingMods)) { - throw new InvariantException('This play does not include the mods required.'); - } - } - - $disallowedMods = array_diff( - array_column($mods, 'acronym'), - array_column($this->playlistItem->required_mods, 'acronym'), - array_column($this->playlistItem->allowed_mods, 'acronym') - ); - - if (!empty($disallowedMods)) { - throw new InvariantException('This play includes mods that are not allowed.'); - } - - $this->score()->associate($score); - $this->save(); - }); - } - public function position(): ?int { $score = $this->score; diff --git a/tests/Models/Multiplayer/ScoreLinkTest.php b/tests/Models/Multiplayer/ScoreLinkTest.php index 363cfc2462e..8931a0aad31 100644 --- a/tests/Models/Multiplayer/ScoreLinkTest.php +++ b/tests/Models/Multiplayer/ScoreLinkTest.php @@ -10,7 +10,7 @@ use App\Exceptions\InvariantException; use App\Models\Multiplayer\PlaylistItem; use App\Models\Multiplayer\ScoreLink; -use App\Models\User; +use App\Models\ScoreToken; use Carbon\Carbon; use Tests\TestCase; @@ -18,24 +18,22 @@ class ScoreLinkTest extends TestCase { public function testRequiredModsMissing() { - $user = User::factory()->create(); $playlistItem = PlaylistItem::factory()->create([ 'required_mods' => [[ 'acronym' => 'HD', ]], ]); - $scoreLink = ScoreLink::factory()->make([ - 'score_id' => null, - 'user_id' => $user, + $scoreToken = ScoreToken::factory()->create([ + 'beatmap_id' => $playlistItem->beatmap_id, 'playlist_item_id' => $playlistItem, ]); $this->expectException(InvariantException::class); $this->expectExceptionMessage('This play does not include the mods required.'); - $scoreLink->complete([ + ScoreLink::complete($scoreToken, [ 'beatmap_id' => $playlistItem->beatmap_id, 'ruleset_id' => $playlistItem->ruleset_id, - 'user_id' => $user->getKey(), + 'user_id' => $scoreToken->user_id, 'ended_at' => json_date(Carbon::now()), 'mods' => [], 'statistics' => [ @@ -46,23 +44,21 @@ public function testRequiredModsMissing() public function testRequiredModsPresent() { - $user = User::factory()->create(); $playlistItem = PlaylistItem::factory()->create([ 'required_mods' => [[ 'acronym' => 'HD', ]], ]); - $scoreLink = ScoreLink::factory()->make([ - 'score_id' => null, - 'user_id' => $user, + $scoreToken = ScoreToken::factory()->create([ + 'beatmap_id' => $playlistItem->beatmap_id, 'playlist_item_id' => $playlistItem, ]); $this->expectNotToPerformAssertions(); - $scoreLink->complete([ + ScoreLink::complete($scoreToken, [ 'beatmap_id' => $playlistItem->beatmap_id, 'ruleset_id' => $playlistItem->ruleset_id, - 'user_id' => $user->getKey(), + 'user_id' => $scoreToken->user_id, 'ended_at' => json_date(Carbon::now()), 'mods' => [['acronym' => 'HD']], 'statistics' => [ @@ -73,7 +69,6 @@ public function testRequiredModsPresent() public function testExpectedAllowedMod() { - $user = User::factory()->create(); $playlistItem = PlaylistItem::factory()->create([ 'required_mods' => [[ 'acronym' => 'DT', @@ -82,17 +77,16 @@ public function testExpectedAllowedMod() 'acronym' => 'HD', ]], ]); - $scoreLink = ScoreLink::factory()->make([ - 'score_id' => null, - 'user_id' => $user, + $scoreToken = ScoreToken::factory()->create([ + 'beatmap_id' => $playlistItem->beatmap_id, 'playlist_item_id' => $playlistItem, ]); $this->expectNotToPerformAssertions(); - $scoreLink->complete([ + ScoreLink::complete($scoreToken, [ 'beatmap_id' => $playlistItem->beatmap_id, 'ruleset_id' => $playlistItem->ruleset_id, - 'user_id' => $user->getKey(), + 'user_id' => $scoreToken->user_id, 'ended_at' => json_date(Carbon::now()), 'mods' => [ ['acronym' => 'DT'], @@ -106,7 +100,6 @@ public function testExpectedAllowedMod() public function testUnexpectedAllowedMod() { - $user = User::factory()->create(); $playlistItem = PlaylistItem::factory()->create([ 'required_mods' => [[ 'acronym' => 'DT', @@ -115,18 +108,17 @@ public function testUnexpectedAllowedMod() 'acronym' => 'HR', ]], ]); - $scoreLink = ScoreLink::factory()->make([ - 'score_id' => null, - 'user_id' => $user, + $scoreToken = ScoreToken::factory()->create([ + 'beatmap_id' => $playlistItem->beatmap_id, 'playlist_item_id' => $playlistItem, ]); $this->expectException(InvariantException::class); $this->expectExceptionMessage('This play includes mods that are not allowed.'); - $scoreLink->complete([ + ScoreLink::complete($scoreToken, [ 'beatmap_id' => $playlistItem->beatmap_id, 'ruleset_id' => $playlistItem->ruleset_id, - 'user_id' => $user->getKey(), + 'user_id' => $scoreToken->user_id, 'ended_at' => json_date(Carbon::now()), 'mods' => [ ['acronym' => 'DT'], @@ -140,20 +132,18 @@ public function testUnexpectedAllowedMod() public function testUnexpectedModWhenNoModsAreAllowed() { - $user = User::factory()->create(); $playlistItem = PlaylistItem::factory()->create(); // no required or allowed mods. - $scoreLink = ScoreLink::factory()->make([ - 'score_id' => null, - 'user_id' => $user, + $scoreToken = ScoreToken::factory()->create([ + 'beatmap_id' => $playlistItem->beatmap_id, 'playlist_item_id' => $playlistItem, ]); $this->expectException(InvariantException::class); $this->expectExceptionMessage('This play includes mods that are not allowed.'); - $scoreLink->complete([ + ScoreLink::complete($scoreToken, [ 'beatmap_id' => $playlistItem->beatmap_id, 'ruleset_id' => $playlistItem->ruleset_id, - 'user_id' => $user->getKey(), + 'user_id' => $scoreToken->user_id, 'ended_at' => json_date(Carbon::now()), 'mods' => [['acronym' => 'HD']], 'statistics' => [ From 869506e3e407c22426eeb4e8d92b7a5192fb75c5 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 2 Nov 2023 18:39:54 +0900 Subject: [PATCH 2/2] Associate relations right away --- app/Models/Multiplayer/ScoreLink.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/Models/Multiplayer/ScoreLink.php b/app/Models/Multiplayer/ScoreLink.php index 532e7f316a2..2dc0be08c9d 100644 --- a/app/Models/Multiplayer/ScoreLink.php +++ b/app/Models/Multiplayer/ScoreLink.php @@ -44,14 +44,12 @@ public static function complete(ScoreToken $token, array $params): static throw new InvariantException('This play includes mods that are not allowed.'); } - $scoreId = $score->getKey(); - $token->fill(['score_id' => $scoreId])->saveOrExplode(); + $token->score()->associate($score)->saveOrExplode(); - $ret = new static([ - 'playlist_item_id' => $playlistItem->getKey(), - 'score_id' => $scoreId, - 'user_id' => $score->user_id, - ]); + $ret = (new static()) + ->playlistItem()->associate($playlistItem) + ->score()->associate($score) + ->user()->associate($token->user); $ret->saveOrExplode(); return $ret;