Merge pull request #10697 from nanaya/score-link-create

Move score link complete function to static method
ppy · Nov 2, 2023 · bc3cdb0 · bc3cdb0
2 parents 848f059 + cea9342
commit bc3cdb0
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 75 deletions.
diff --git 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
@@ -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
@@ -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,37 @@
  */
 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.');
+            }
+
+            $token->score()->associate($score)->saveOrExplode();
+
+            $ret = (new static())
+                ->playlistItem()->associate($playlistItem)
+                ->score()->associate($score)
+                ->user()->associate($token->user);
+            $ret->saveOrExplode();
+
+            return $ret;
+        });
+    }
+
     public $incrementing = false;
     public $timestamps = false;
 
@@ -64,38 +96,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
@@ -10,32 +10,30 @@
 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;
 
 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' => [