From 06e15d316cd794d22623b5ab662679579f1e6817 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 27 Oct 2022 22:04:27 +0900 Subject: [PATCH 01/10] Preload solo score performance --- app/Models/Score/Best/Model.php | 1 + app/Models/Score/Model.php | 7 +++++++ app/Transformers/ScoreTransformer.php | 1 + 3 files changed, 9 insertions(+) diff --git a/app/Models/Score/Best/Model.php b/app/Models/Score/Best/Model.php index 6ca0db20ee0..05454ebe0d0 100644 --- a/app/Models/Score/Best/Model.php +++ b/app/Models/Score/Best/Model.php @@ -85,6 +85,7 @@ public function getAttribute($key) 'best' => $this, 'data' => $this->getData(), 'enabled_mods' => $this->getEnabledModsAttribute($this->getRawAttribute('enabled_mods')), + 'nonexistent' => null, 'pass' => true, 'beatmap', diff --git a/app/Models/Score/Model.php b/app/Models/Score/Model.php index 57e7467a41c..b523b77e278 100644 --- a/app/Models/Score/Model.php +++ b/app/Models/Score/Model.php @@ -10,6 +10,7 @@ use App\Models\Beatmap; use App\Models\Model as BaseModel; use App\Models\Solo\ScoreData; +use App\Models\Solo\ScorePerformance; use App\Models\Traits\Scoreable; use App\Models\User; @@ -114,6 +115,11 @@ public function best() return $this->belongsTo("App\\Models\\Score\\Best\\{$basename}", 'high_score_id', 'score_id'); } + public function performance() + { + return $this->belongsTo(ScorePerformance::class, 'nonexistent'); + } + public function user() { return $this->belongsTo(User::class, 'user_id'); @@ -148,6 +154,7 @@ public function getAttribute($key) 'data' => $this->getData(), 'enabled_mods' => $this->getEnabledModsAttribute($this->getRawAttribute('enabled_mods')), + 'nonexistent' => null, 'beatmap', 'best', diff --git a/app/Transformers/ScoreTransformer.php b/app/Transformers/ScoreTransformer.php index 166570dfa41..de54324b3d1 100644 --- a/app/Transformers/ScoreTransformer.php +++ b/app/Transformers/ScoreTransformer.php @@ -24,6 +24,7 @@ class ScoreTransformer extends TransformerAbstract const USER_PROFILE_INCLUDES_PRELOAD = [ 'beatmap', 'beatmap.beatmapset', + 'performance', // it's for user profile so the user is already available // 'user', ]; From 0d6a7e3431f33da679b7bb2a8c45f08c8b78741c Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Mar 2023 21:36:54 +0900 Subject: [PATCH 02/10] Use new index for score leaderboard --- .env.example | 1 + app/Http/Controllers/BeatmapsController.php | 206 ++++++------- app/Libraries/Search/ScoreSearch.php | 5 +- config/osu.php | 1 + .../BeatmapsControllerSoloScoresTest.php | 138 +++++---- tests/Controllers/BeatmapsControllerTest.php | 270 +----------------- 6 files changed, 157 insertions(+), 464 deletions(-) diff --git a/.env.example b/.env.example index 9a43b525a22..656a1343b01 100644 --- a/.env.example +++ b/.env.example @@ -299,6 +299,7 @@ CLIENT_CHECK_VERSION=false # TWITCH_CLIENT_SECRET= # SCORES_ES_CACHE_DURATION= +# SCORES_ES_ENABLE_LEGACY_FILTER=true # SCORES_EXPERIMENTAL_RANK_AS_DEFAULT=false # SCORES_EXPERIMENTAL_RANK_AS_EXTRA=false # SCORES_RANK_CACHE_LOCAL_SERVER=0 diff --git a/app/Http/Controllers/BeatmapsController.php b/app/Http/Controllers/BeatmapsController.php index aa412412e74..7041f4f9fc2 100644 --- a/app/Http/Controllers/BeatmapsController.php +++ b/app/Http/Controllers/BeatmapsController.php @@ -24,6 +24,32 @@ class BeatmapsController extends Controller const DEFAULT_API_INCLUDES = ['beatmapset.ratings', 'failtimes', 'max_combo']; const DEFAULT_SCORE_INCLUDES = ['user', 'user.country', 'user.cover']; + private static function assertSupporterOnlyOptions(?User $currentUser, string $type, array $mods): void + { + $isSupporter = $currentUser?->isSupporter() ?? false; + if ($type !== 'global' && !$isSupporter) { + throw new InvariantException(osu_trans('errors.supporter_only')); + } + if (!empty($mods) && !is_api_request() && !$isSupporter) { + throw new InvariantException(osu_trans('errors.supporter_only')); + } + } + + private static function baseScoreQuery(Beatmap $beatmap, $mode, $mods, $type = null) + { + $query = BestModel::getClass($mode) + ::default() + ->where('beatmap_id', $beatmap->getKey()) + ->with(['beatmap', 'user.country', 'user.userProfileCustomization']) + ->withMods($mods); + + if ($type !== null) { + $query->withType($type, ['user' => auth()->user()]); + } + + return $query; + } + public function __construct() { parent::__construct(); @@ -247,7 +273,7 @@ public function show($id) } /** - * Get Beatmap scores + * Get Beatmap scores (legacy) * * Returns the top scores for a beatmap * @@ -265,61 +291,14 @@ public function show($id) */ public function scores($id) { - $beatmap = Beatmap::findOrFail($id); - if ($beatmap->approved <= 0) { - return ['scores' => []]; - } - - $params = get_params(request()->all(), null, [ - 'limit:int', - 'mode:string', - 'mods:string[]', - 'type:string', - ], ['null_missing' => true]); - - $mode = presence($params['mode']) ?? $beatmap->mode; - $mods = array_values(array_filter($params['mods'] ?? [])); - $type = presence($params['type']) ?? 'global'; - $currentUser = auth()->user(); - - $this->assertSupporterOnlyOptions($currentUser, $type, $mods); - - $query = static::baseScoreQuery($beatmap, $mode, $mods, $type); - - if ($currentUser !== null) { - // own score shouldn't be filtered by visibleUsers() - $userScore = (clone $query)->where('user_id', $currentUser->user_id)->first(); - } - - $scoreTransformer = new ScoreTransformer(); - - $results = [ - 'scores' => json_collection( - $query->visibleUsers()->forListing($params['limit']), - $scoreTransformer, - static::DEFAULT_SCORE_INCLUDES - ), - ]; - - if (isset($userScore)) { - $results['user_score'] = [ - 'position' => $userScore->userRank(compact('type', 'mods')), - 'score' => json_item($userScore, $scoreTransformer, static::DEFAULT_SCORE_INCLUDES), - ]; - // TODO: remove this old camelCased json field - $results['userScore'] = $results['user_score']; - } - - return $results; + return $this->beatmapScores($id, null, true); } /** - * Get Beatmap scores (temp) + * Get Beatmap scores (non-legacy) * * Returns the top scores for a beatmap from newer client. * - * This is a temporary endpoint. - * * --- * * ### Response Format @@ -334,62 +313,7 @@ public function scores($id) */ public function soloScores($id) { - $beatmap = Beatmap::findOrFail($id); - if ($beatmap->approved <= 0) { - return ['scores' => []]; - } - - $params = get_params(request()->all(), null, [ - 'limit:int', - 'mode', - 'mods:string[]', - 'type:string', - ], ['null_missing' => true]); - - if ($params['mode'] !== null) { - $rulesetId = Beatmap::MODES[$params['mode']] ?? null; - if ($rulesetId === null) { - throw new InvariantException('invalid mode specified'); - } - } - $rulesetId ??= $beatmap->playmode; - $mods = array_values(array_filter($params['mods'] ?? [])); - $type = presence($params['type'], 'global'); - $currentUser = auth()->user(); - - $this->assertSupporterOnlyOptions($currentUser, $type, $mods); - - $esFetch = new BeatmapScores([ - 'beatmap_ids' => [$beatmap->getKey()], - 'is_legacy' => false, - 'limit' => $params['limit'], - 'mods' => $mods, - 'ruleset_id' => $rulesetId, - 'type' => $type, - 'user' => $currentUser, - ]); - $scores = $esFetch->all()->loadMissing(['beatmap', 'performance', 'user.country', 'user.userProfileCustomization']); - $userScore = $esFetch->userBest(); - $scoreTransformer = new ScoreTransformer(ScoreTransformer::TYPE_SOLO); - - $results = [ - 'scores' => json_collection( - $scores, - $scoreTransformer, - static::DEFAULT_SCORE_INCLUDES - ), - ]; - - if (isset($userScore)) { - $results['user_score'] = [ - 'position' => $esFetch->rank($userScore), - 'score' => json_item($userScore, $scoreTransformer, static::DEFAULT_SCORE_INCLUDES), - ]; - // TODO: remove this old camelCased json field - $results['userScore'] = $results['user_score']; - } - - return $results; + return $this->beatmapScores($id, ScoreTransformer::TYPE_SOLO, false); } public function updateOwner($id) @@ -499,29 +423,63 @@ public function userScoreAll($beatmapId, $userId) ]; } - private static function baseScoreQuery(Beatmap $beatmap, $mode, $mods, $type = null) + private function beatmapScores(string $id, ?string $scoreTransformerType, bool $isLegacy): array { - $query = BestModel::getClass($mode) - ::default() - ->where('beatmap_id', $beatmap->getKey()) - ->with(['beatmap', 'user.country', 'user.userProfileCustomization']) - ->withMods($mods); - - if ($type !== null) { - $query->withType($type, ['user' => auth()->user()]); + $beatmap = Beatmap::findOrFail($id); + if ($beatmap->approved <= 0) { + return ['scores' => []]; } - return $query; - } + $params = get_params(request()->all(), null, [ + 'limit:int', + 'mode', + 'mods:string[]', + 'type:string', + ], ['null_missing' => true]); - private function assertSupporterOnlyOptions(?User $currentUser, string $type, array $mods): void - { - $isSupporter = $currentUser?->isSupporter() ?? false; - if ($type !== 'global' && !$isSupporter) { - throw new InvariantException(osu_trans('errors.supporter_only')); + if ($params['mode'] !== null) { + $rulesetId = Beatmap::MODES[$params['mode']] ?? null; + if ($rulesetId === null) { + throw new InvariantException('invalid mode specified'); + } } - if (!empty($mods) && !is_api_request() && !$isSupporter) { - throw new InvariantException(osu_trans('errors.supporter_only')); + $rulesetId ??= $beatmap->playmode; + $mods = array_values(array_filter($params['mods'] ?? [])); + $type = presence($params['type'], 'global'); + $currentUser = auth()->user(); + + static::assertSupporterOnlyOptions($currentUser, $type, $mods); + + $esFetch = new BeatmapScores([ + 'beatmap_ids' => [$beatmap->getKey()], + 'is_legacy' => $isLegacy, + 'limit' => $params['limit'], + 'mods' => $mods, + 'ruleset_id' => $rulesetId, + 'type' => $type, + 'user' => $currentUser, + ]); + $scores = $esFetch->all()->loadMissing(['beatmap', 'performance', 'user.country', 'user.userProfileCustomization']); + $userScore = $esFetch->userBest(); + $scoreTransformer = new ScoreTransformer($scoreTransformerType); + + $results = [ + 'scores' => json_collection( + $scores, + $scoreTransformer, + static::DEFAULT_SCORE_INCLUDES + ), + ]; + + if (isset($userScore)) { + $results['user_score'] = [ + 'position' => $esFetch->rank($userScore), + 'score' => json_item($userScore, $scoreTransformer, static::DEFAULT_SCORE_INCLUDES), + ]; + // TODO: remove this old camelCased json field + $results['userScore'] = $results['user_score']; } + + return $results; } } diff --git a/app/Libraries/Search/ScoreSearch.php b/app/Libraries/Search/ScoreSearch.php index 5e6aa6d490c..50736e686de 100644 --- a/app/Libraries/Search/ScoreSearch.php +++ b/app/Libraries/Search/ScoreSearch.php @@ -36,7 +36,7 @@ public function getQuery(): BoolQuery { $query = new BoolQuery(); - if ($this->params->isLegacy !== null) { + if (config('osu.scores.es_enable_legacy_filter') && $this->params->isLegacy !== null) { $query->filter(['term' => ['is_legacy' => $this->params->isLegacy]]); } if ($this->params->rulesetId !== null) { @@ -141,6 +141,9 @@ private function addModsFilter(BoolQuery $query): void ? $modsHelper->allIds : new Set(array_keys($modsHelper->mods[$this->params->rulesetId])); $allMods->remove('PF', 'SD', 'MR'); + if ($this->params->isLegacy || !config('osu.scores.es_enable_legacy_filter')) { + $allMods->remove('CL'); + } $allSearchMods = []; foreach ($mods as $mod) { diff --git a/config/osu.php b/config/osu.php index ee2100f06a8..eba0defc448 100644 --- a/config/osu.php +++ b/config/osu.php @@ -165,6 +165,7 @@ ], 'scores' => [ 'es_cache_duration' => 60 * (get_float(env('SCORES_ES_CACHE_DURATION')) ?? 0.5), // in minutes, converted to seconds + 'es_enable_legacy_filter' => get_bool(env('SCORES_ES_ENABLE_LEGACY_FILTER')) ?? true, 'experimental_rank_as_default' => get_bool(env('SCORES_EXPERIMENTAL_RANK_AS_DEFAULT')) ?? false, 'experimental_rank_as_extra' => get_bool(env('SCORES_EXPERIMENTAL_RANK_AS_EXTRA')) ?? false, 'rank_cache' => [ diff --git a/tests/Controllers/BeatmapsControllerSoloScoresTest.php b/tests/Controllers/BeatmapsControllerSoloScoresTest.php index 2886fa10f40..979f6dbe6b7 100644 --- a/tests/Controllers/BeatmapsControllerSoloScoresTest.php +++ b/tests/Controllers/BeatmapsControllerSoloScoresTest.php @@ -14,14 +14,16 @@ use App\Models\Genre; use App\Models\Group; use App\Models\Language; -use App\Models\Solo\Score as SoloScore; +use App\Models\Solo\Score; use App\Models\User; use App\Models\UserGroup; use App\Models\UserGroupEvent; use App\Models\UserRelation; -use Artisan; use Tests\TestCase; +/** + * @group EsSoloScores + */ class BeatmapsControllerSoloScoresTest extends TestCase { protected $connectionsToTransact = []; @@ -42,9 +44,16 @@ public static function setUpBeforeClass(): void $countryAcronym = static::$user->country_acronym; static::$scores = []; - $scoreFactory = SoloScore::factory(); + $scoreFactory = Score::factory(); foreach (['solo' => 0, 'legacy' => null] as $type => $buildId) { $defaultData = ['build_id' => $buildId]; + $makeMods = fn (array $modNames): array => array_map( + fn (string $modName): array => [ + 'acronym' => $modName, + 'settings' => [], + ], + [...$modNames, ...($type === 'legacy' ? ['CL'] : [])], + ); static::$scores = array_merge(static::$scores, [ "{$type}:user" => $scoreFactory->withData($defaultData, ['total_score' => 1100])->create([ @@ -54,7 +63,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:userMods" => $scoreFactory->withData($defaultData, [ 'total_score' => 1050, - 'mods' => static::defaultMods(['DT', 'HD']), + 'mods' => $makeMods(['DT', 'HD']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -62,7 +71,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:userModsNC" => $scoreFactory->withData($defaultData, [ 'total_score' => 1050, - 'mods' => static::defaultMods(['NC']), + 'mods' => $makeMods(['NC']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -70,7 +79,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:otherUserModsNCPFHigherScore" => $scoreFactory->withData($defaultData, [ 'total_score' => 1010, - 'mods' => static::defaultMods(['NC', 'PF']), + 'mods' => $makeMods(['NC', 'PF']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -78,7 +87,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:userModsLowerScore" => $scoreFactory->withData($defaultData, [ 'total_score' => 1000, - 'mods' => static::defaultMods(['DT', 'HD']), + 'mods' => $makeMods(['DT', 'HD']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -92,7 +101,7 @@ public static function setUpBeforeClass(): void // With preference mods "{$type}:otherUser" => $scoreFactory->withData($defaultData, [ 'total_score' => 1000, - 'mods' => static::defaultMods(['PF']), + 'mods' => $makeMods(['PF']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -100,7 +109,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:otherUserMods" => $scoreFactory->withData($defaultData, [ 'total_score' => 1000, - 'mods' => static::defaultMods(['HD', 'PF', 'NC']), + 'mods' => $makeMods(['HD', 'PF', 'NC']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -108,7 +117,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:otherUserModsExtraNonPreferences" => $scoreFactory->withData($defaultData, [ 'total_score' => 1000, - 'mods' => static::defaultMods(['DT', 'HD', 'HR']), + 'mods' => $makeMods(['DT', 'HD', 'HR']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -116,7 +125,7 @@ public static function setUpBeforeClass(): void ]), "{$type}:otherUserModsUnrelated" => $scoreFactory->withData($defaultData, [ 'total_score' => 1000, - 'mods' => static::defaultMods(['FL']), + 'mods' => $makeMods(['FL']), ])->create([ 'beatmap_id' => static::$beatmap, 'preserve' => true, @@ -152,11 +161,7 @@ public static function setUpBeforeClass(): void 'zebra_id' => $friend->getKey(), ]); - Artisan::call('es:index-scores:queue', [ - '--all' => true, - '--no-interaction' => true, - ]); - (new ScoreSearch())->indexWait(); + static::reindexScores(); }); } @@ -171,7 +176,7 @@ public static function tearDownAfterClass(): void Genre::truncate(); Group::truncate(); Language::truncate(); - SoloScore::truncate(); + Score::truncate(); User::truncate(); UserGroup::truncate(); UserGroupEvent::truncate(); @@ -180,25 +185,13 @@ public static function tearDownAfterClass(): void }); } - private static function defaultMods(array $modNames): array - { - return array_map( - fn ($modName) => [ - 'acronym' => $modName, - 'settings' => [], - ], - $modNames, - ); - } - /** * @dataProvider dataProviderForTestQuery - * @group EsSoloScores */ - public function testQuery(array $scoreKeys, array $params) + public function testQuery(array $scoreKeys, array $params, string $route) { $resp = $this->actingAs(static::$user) - ->json('GET', route('beatmaps.solo-scores', static::$beatmap), $params) + ->json('GET', route("beatmaps.{$route}", static::$beatmap), $params) ->assertSuccessful(); $json = json_decode($resp->getContent(), true); @@ -210,44 +203,49 @@ public function testQuery(array $scoreKeys, array $params) public function dataProviderForTestQuery(): array { - return [ - 'no parameters' => [[ - 'solo:user', - 'solo:otherUserModsNCPFHigherScore', - 'solo:friend', - 'solo:otherUser2Later', - 'solo:otherUser3SameCountry', - ], []], - 'by country' => [[ - 'solo:user', - 'solo:otherUser3SameCountry', - ], ['type' => 'country']], - 'by friend' => [[ - 'solo:user', - 'solo:friend', - ], ['type' => 'friend']], - 'mods filter' => [[ - 'solo:userMods', - 'solo:otherUserMods', - ], ['mods' => ['DT', 'HD']]], - 'mods with implied filter' => [[ - 'solo:userModsNC', - 'solo:otherUserModsNCPFHigherScore', - ], ['mods' => ['NC']]], - 'mods with nomods' => [[ - 'solo:user', - 'solo:otherUserModsNCPFHigherScore', - 'solo:friend', - 'solo:otherUser2Later', - 'solo:otherUser3SameCountry', - ], ['mods' => ['NC', 'NM']]], - 'nomods filter' => [[ - 'solo:user', - 'solo:friend', - 'solo:otherUser', - 'solo:otherUser2Later', - 'solo:otherUser3SameCountry', - ], ['mods' => ['NM']]], - ]; + $ret = []; + foreach (['solo' => 'solo-scores', 'legacy' => 'scores'] as $type => $route) { + $ret = array_merge($ret, [ + "{$type}: no parameters" => [[ + "{$type}:user", + "{$type}:otherUserModsNCPFHigherScore", + "{$type}:friend", + "{$type}:otherUser2Later", + "{$type}:otherUser3SameCountry", + ], [], $route], + "{$type}: by country" => [[ + "{$type}:user", + "{$type}:otherUser3SameCountry", + ], ['type' => 'country'], $route], + "{$type}: by friend" => [[ + "{$type}:user", + "{$type}:friend", + ], ['type' => 'friend'], $route], + "{$type}: mods filter" => [[ + "{$type}:userMods", + "{$type}:otherUserMods", + ], ['mods' => ['DT', 'HD']], $route], + "{$type}: mods with implied filter" => [[ + "{$type}:userModsNC", + "{$type}:otherUserModsNCPFHigherScore", + ], ['mods' => ['NC']], $route], + "{$type}: mods with nomods" => [[ + "{$type}:user", + "{$type}:otherUserModsNCPFHigherScore", + "{$type}:friend", + "{$type}:otherUser2Later", + "{$type}:otherUser3SameCountry", + ], ['mods' => ['NC', 'NM']], $route], + "{$type}: nomods filter" => [[ + "{$type}:user", + "{$type}:friend", + "{$type}:otherUser", + "{$type}:otherUser2Later", + "{$type}:otherUser3SameCountry", + ], ['mods' => ['NM']], $route], + ]); + } + + return $ret; } } diff --git a/tests/Controllers/BeatmapsControllerTest.php b/tests/Controllers/BeatmapsControllerTest.php index 194aa328f9f..2eb358340ca 100644 --- a/tests/Controllers/BeatmapsControllerTest.php +++ b/tests/Controllers/BeatmapsControllerTest.php @@ -10,12 +10,8 @@ use App\Models\Beatmap; use App\Models\Beatmapset; use App\Models\BeatmapsetEvent; -use App\Models\Country; -use App\Models\Score\Best\Model as ScoreBest; use App\Models\User; -use App\Models\UserRelation; use Illuminate\Testing\Fluent\AssertableJson; -use Illuminate\Testing\TestResponse; use Tests\TestCase; class BeatmapsControllerTest extends TestCase @@ -106,7 +102,7 @@ public function testInvalidMode() { $this->json('GET', route('beatmaps.scores', $this->beatmap), [ 'mode' => 'nope', - ])->assertStatus(404); + ])->assertStatus(422); } /** @@ -177,261 +173,6 @@ public function testScoresNonGeneralSupporter() ])->assertStatus(200); } - public function testScores() - { - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1100, - 'user_id' => $this->user, - ]), - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1000, - ]), - // Same total score but achieved later so it should come up after earlier score - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1000, - ]), - ]; - // Hidden score should be filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'hidden' => true, - 'score' => 800, - ]); - // Another score from scores[0] user (should be filtered out) - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 800, - 'user_id' => $this->user, - ]); - // Unrelated score - ScoreBest::getClass(array_rand(Beatmap::MODES))::factory()->create(); - - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', $this->beatmap)) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresByCountry() - { - $countryAcronym = $this->user->country_acronym; - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'country_acronym' => $countryAcronym, - 'score' => 1100, - 'user_id' => $this->user, - ]), - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1000, - 'country_acronym' => $countryAcronym, - 'user_id' => User::factory()->state(['country_acronym' => $countryAcronym]), - ]), - ]; - $otherCountry = Country::factory()->create(); - $otherCountryAcronym = $otherCountry->acronym; - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'country_acronym' => $otherCountryAcronym, - 'user_id' => User::factory()->state(['country_acronym' => $otherCountryAcronym]), - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'type' => 'country'])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresByFriend() - { - $friend = User::factory()->create(); - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1100, - 'user_id' => $friend, - ]), - // Own score is included - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1000, - 'user_id' => $this->user, - ]), - ]; - UserRelation::create([ - 'friend' => true, - 'user_id' => $this->user->getKey(), - 'zebra_id' => $friend->getKey(), - ]); - // Non-friend score is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'type' => 'friend'])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresModsFilter() - { - $modsHelper = app('mods'); - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'HD']), - 'score' => 1500, - ]), - // Score with preference mods is included - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'HD', 'NC', 'PF']), - 'score' => 1100, - 'user_id' => $this->user, - ]), - ]; - // No mod is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => 0, - ]); - // Unrelated mod is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['FL']), - ]); - // Extra non-preference mod is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'HD', 'HR']), - ]); - // From same user but lower score is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'HD']), - 'score' => 1000, - 'user_id' => $this->user, - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'mods' => ['DT', 'HD']])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresModsWithImpliedFilter() - { - $modsHelper = app('mods'); - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'NC']), - 'score' => 1500, - ]), - // Score with preference mods is included - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'NC', 'PF']), - 'score' => 1100, - 'user_id' => $this->user, - ]), - ]; - // No mod is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => 0, - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'mods' => ['NC']])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresModsWithNomodsFilter() - { - $modsHelper = app('mods'); - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'NC']), - 'score' => 1500, - ]), - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => 0, - 'score' => 1100, - 'user_id' => $this->user, - ]), - ]; - // With unrelated mod - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT', 'NC', 'HD']), - 'score' => 1500, - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'mods' => ['DT', 'NC', 'NM']])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - - public function testScoresNomodsFilter() - { - $modsHelper = app('mods'); - $scoreClass = ScoreBest::getClassByRulesetId($this->beatmap->playmode); - $scores = [ - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1500, - 'enabled_mods' => 0, - ]), - // Preference mod is included - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'score' => 1100, - 'user_id' => $this->user, - 'enabled_mods' => $modsHelper->idsToBitset(['PF']), - ]), - ]; - // Non-preference mod is filtered out - $scoreClass::factory()->create([ - 'beatmap_id' => $this->beatmap, - 'enabled_mods' => $modsHelper->idsToBitset(['DT']), - ]); - - $this->user->update(['osu_subscriber' => true]); - $resp = $this->actingAs($this->user) - ->json('GET', route('beatmaps.scores', ['beatmap' => $this->beatmap, 'mods' => ['NM']])) - ->assertSuccessful(); - - $this->assertSameScoresFromResponse($scores, $resp); - } - public function testShowForApi() { $beatmap = Beatmap::factory()->create(); @@ -621,15 +362,6 @@ protected function setUp(): void $this->beatmap = Beatmap::factory()->qualified()->create(); } - private function assertSameScoresFromResponse(array $scores, TestResponse $response): void - { - $json = json_decode($response->getContent(), true); - $this->assertSame(count($scores), count($json['scores'])); - foreach ($json['scores'] as $i => $jsonScore) { - $this->assertSame($scores[$i]->getKey(), $jsonScore['id']); - } - } - private function createExistingFruitsBeatmap() { return Beatmap::factory()->create([ From 4a6f7525938fd4921d75e268db4b27f5d6c7171e Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 21 Jun 2023 17:13:14 +0900 Subject: [PATCH 03/10] Use solo score index for user beatmap scores --- app/Http/Controllers/BeatmapsController.php | 55 ++++++++++--------- routes/web.php | 4 +- .../BeatmapsControllerSoloScoresTest.php | 37 +++++++++++++ 3 files changed, 67 insertions(+), 29 deletions(-) diff --git a/app/Http/Controllers/BeatmapsController.php b/app/Http/Controllers/BeatmapsController.php index 7041f4f9fc2..4dc812fb522 100644 --- a/app/Http/Controllers/BeatmapsController.php +++ b/app/Http/Controllers/BeatmapsController.php @@ -9,9 +9,11 @@ use App\Jobs\Notifications\BeatmapOwnerChange; use App\Libraries\BeatmapDifficultyAttributes; use App\Libraries\Score\BeatmapScores; +use App\Libraries\Score\UserRank; +use App\Libraries\Search\ScoreSearch; +use App\Libraries\Search\ScoreSearchParams; use App\Models\Beatmap; use App\Models\BeatmapsetEvent; -use App\Models\Score\Best\Model as BestModel; use App\Models\User; use App\Transformers\BeatmapTransformer; use App\Transformers\ScoreTransformer; @@ -35,21 +37,6 @@ private static function assertSupporterOnlyOptions(?User $currentUser, string $t } } - private static function baseScoreQuery(Beatmap $beatmap, $mode, $mods, $type = null) - { - $query = BestModel::getClass($mode) - ::default() - ->where('beatmap_id', $beatmap->getKey()) - ->with(['beatmap', 'user.country', 'user.userProfileCustomization']) - ->withMods($mods); - - if ($type !== null) { - $query->withType($type, ['user' => auth()->user()]); - } - - return $query; - } - public function __construct() { parent::__construct(); @@ -374,13 +361,25 @@ public function userScore($beatmapId, $userId) $mode = presence($params['mode'] ?? null, $beatmap->mode); $mods = array_values(array_filter($params['mods'] ?? [])); - $score = static::baseScoreQuery($beatmap, $mode, $mods) - ->visibleUsers() - ->where('user_id', $userId) - ->firstOrFail(); + $baseParams = ScoreSearchParams::fromArray([ + 'beatmap_ids' => [$beatmap->getKey()], + 'is_legacy' => true, + 'limit' => 1, + 'mods' => $mods, + 'ruleset_id' => Beatmap::MODES[$mode], + 'sort' => 'score_desc', + 'user_id' => (int) $userId, + ]); + $score = (new ScoreSearch($baseParams))->records()->first(); + abort_if($score === null, 404); + + $rankParams = clone $baseParams; + $rankParams->beforeScore = $score; + $rankParams->userId = null; + $rank = UserRank::getRank($rankParams); return [ - 'position' => $score->userRank(compact('mods')), + 'position' => $rank, 'score' => json_item( $score, new ScoreTransformer(), @@ -411,12 +410,14 @@ public function userScoreAll($beatmapId, $userId) { $beatmap = Beatmap::scoreable()->findOrFail($beatmapId); $mode = presence(get_string(request('mode'))) ?? $beatmap->mode; - $scores = BestModel::getClass($mode) - ::default() - ->where([ - 'beatmap_id' => $beatmap->getKey(), - 'user_id' => $userId, - ])->get(); + $params = ScoreSearchParams::fromArray([ + 'beatmap_ids' => [$beatmap->getKey()], + 'is_legacy' => true, + 'ruleset_id' => Beatmap::MODES[$mode], + 'sort' => 'score_desc', + 'user_id' => (int) $userId, + ]); + $scores = (new ScoreSearch($params))->records(); return [ 'scores' => json_collection($scores, new ScoreTransformer()), diff --git a/routes/web.php b/routes/web.php index ac2994702d2..d1234917760 100644 --- a/routes/web.php +++ b/routes/web.php @@ -403,8 +403,8 @@ Route::get('lookup', 'BeatmapsController@lookup')->name('lookup'); Route::group(['prefix' => '{beatmap}'], function () { - Route::get('scores/users/{user}', 'BeatmapsController@userScore'); - Route::get('scores/users/{user}/all', 'BeatmapsController@userScoreAll'); + Route::get('scores/users/{user}', 'BeatmapsController@userScore')->name('user.score'); + Route::get('scores/users/{user}/all', 'BeatmapsController@userScoreAll')->name('user.scores'); Route::get('scores', 'BeatmapsController@scores')->name('scores'); Route::get('solo-scores', 'BeatmapsController@soloScores')->name('solo-scores'); diff --git a/tests/Controllers/BeatmapsControllerSoloScoresTest.php b/tests/Controllers/BeatmapsControllerSoloScoresTest.php index 979f6dbe6b7..c7f7b404157 100644 --- a/tests/Controllers/BeatmapsControllerSoloScoresTest.php +++ b/tests/Controllers/BeatmapsControllerSoloScoresTest.php @@ -14,6 +14,7 @@ use App\Models\Genre; use App\Models\Group; use App\Models\Language; +use App\Models\OAuth; use App\Models\Solo\Score; use App\Models\User; use App\Models\UserGroup; @@ -176,6 +177,8 @@ public static function tearDownAfterClass(): void Genre::truncate(); Group::truncate(); Language::truncate(); + OAuth\Client::truncate(); + OAuth\Token::truncate(); Score::truncate(); User::truncate(); UserGroup::truncate(); @@ -201,6 +204,40 @@ public function testQuery(array $scoreKeys, array $params, string $route) } } + public function testUserScore() + { + $url = route('api.beatmaps.user.score', [ + 'beatmap' => static::$beatmap->getKey(), + 'mods' => ['DT', 'HD'], + 'user' => static::$user->getKey(), + ]); + $this->actAsScopedUser(static::$user); + $this + ->json('GET', $url) + ->assertJsonPath('score.id', static::$scores['legacy:userMods']->getKey()); + } + + public function testUserScoreAll() + { + $url = route('api.beatmaps.user.scores', [ + 'beatmap' => static::$beatmap->getKey(), + 'user' => static::$user->getKey(), + ]); + $this->actAsScopedUser(static::$user); + $this + ->json('GET', $url) + ->assertJsonCount(4, 'scores') + ->assertJsonPath( + 'scores.*.id', + array_map(fn (string $key): int => static::$scores[$key]->getKey(), [ + 'legacy:user', + 'legacy:userMods', + 'legacy:userModsNC', + 'legacy:userModsLowerScore', + ]) + ); + } + public function dataProviderForTestQuery(): array { $ret = []; From e07255239017a57896ce9d76c4ee7117dbc7286f Mon Sep 17 00:00:00 2001 From: nanaya Date: Fri, 28 Oct 2022 21:56:47 +0900 Subject: [PATCH 04/10] Use solo score for profile page (except recent plays) --- app/Http/Controllers/UsersController.php | 15 ++++- app/Models/Solo/Score.php | 5 ++ app/Models/Solo/ScoreLegacyIdMap.php | 34 ++++++++++ app/Models/Traits/UserScoreable.php | 80 +++++++++--------------- 4 files changed, 79 insertions(+), 55 deletions(-) create mode 100644 app/Models/Solo/ScoreLegacyIdMap.php diff --git a/app/Http/Controllers/UsersController.php b/app/Http/Controllers/UsersController.php index 16c993b0d9d..f3f1156d9b7 100644 --- a/app/Http/Controllers/UsersController.php +++ b/app/Http/Controllers/UsersController.php @@ -19,6 +19,8 @@ use App\Models\Country; use App\Models\IpBan; use App\Models\Log; +use App\Models\Solo\Score as SoloScore; +use App\Models\Solo\ScoreLegacyIdMap; use App\Models\User; use App\Models\UserAccountHistory; use App\Models\UserNotFound; @@ -786,9 +788,16 @@ private function getExtra($page, array $options, int $perPage = 10, int $offset case 'scoresFirsts': $transformer = new ScoreTransformer(); $includes = ScoreTransformer::USER_PROFILE_INCLUDES; - $query = $this->user->scoresFirst($this->mode, true) - ->visibleUsers() - ->reorderBy('score_id', 'desc') + $scoreQuery = $this->user->scoresFirst($this->mode, true)->unorder(); + $userFirstsQuery = $scoreQuery->select($scoreQuery->qualifyColumn('score_id')); + $soloMappingQuery = ScoreLegacyIdMap + ::where('ruleset_id', Beatmap::MODES[$this->mode]) + ->whereIn('old_score_id', $userFirstsQuery) + ->select('score_id'); + $query = SoloScore + ::whereIn('id', $soloMappingQuery) + ->default() + ->reorderBy('id', 'desc') ->with(ScoreTransformer::USER_PROFILE_INCLUDES_PRELOAD); $userRelationColumn = 'user'; break; diff --git a/app/Models/Solo/Score.php b/app/Models/Solo/Score.php index 6bca92c036d..a6507f001e1 100644 --- a/app/Models/Solo/Score.php +++ b/app/Models/Solo/Score.php @@ -111,6 +111,11 @@ public function scopeIndexable(Builder $query): Builder ->whereHas('user', fn (Builder $q): Builder => $q->default()); } + public function scopeDefault(Builder $query): Builder + { + return $query->whereHas('beatmap'); + } + public function getAttribute($key) { return match ($key) { diff --git a/app/Models/Solo/ScoreLegacyIdMap.php b/app/Models/Solo/ScoreLegacyIdMap.php new file mode 100644 index 00000000000..156de063d15 --- /dev/null +++ b/app/Models/Solo/ScoreLegacyIdMap.php @@ -0,0 +1,34 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +declare(strict_types=1); + +namespace App\Models\Solo; + +use App\Models\Model; + +/** + * @property int $ruleset_id + * @property int $old_score_id + * @property int $score_id + */ +class ScoreLegacyIdMap extends Model +{ + public $incrementing = false; + public $timestamps = false; + + protected $primaryKey = ':composite'; + protected $primaryKeys = ['ruleset_id', 'old_score_id']; + protected $table = 'solo_scores_legacy_id_map'; + + public function getAttribute($key) + { + return match ($key) { + 'ruleset_id', + 'old_score_id', + 'score_id' => $this->getRawAttribute($key), + }; + } +} diff --git a/app/Models/Traits/UserScoreable.php b/app/Models/Traits/UserScoreable.php index 54761f9e121..54add569b1c 100644 --- a/app/Models/Traits/UserScoreable.php +++ b/app/Models/Traits/UserScoreable.php @@ -5,53 +5,26 @@ namespace App\Models\Traits; -use App\Libraries\Elasticsearch\BoolQuery; -use App\Libraries\Elasticsearch\SearchResponse; -use App\Libraries\Search\BasicSearch; -use App\Models\Score\Best; +use App\Libraries\Score\FetchDedupedScores; +use App\Libraries\Search\ScoreSearchParams; +use App\Models\Beatmap; +use App\Models\Solo\Score; +use Illuminate\Database\Eloquent\Collection; trait UserScoreable { - private $beatmapBestScoreIds = []; + private array $beatmapBestScoreIds = []; + private array $beatmapBestScores = []; - public function aggregatedScoresBest(string $mode, int $size): SearchResponse + public function aggregatedScoresBest(string $mode, int $size): array { - $index = config('osu.elasticsearch.prefix')."high_scores_{$mode}"; - - $search = new BasicSearch($index, "aggregatedScoresBest_{$mode}"); - $search->connectionName = 'scores'; - $search - ->size(0) // don't care about hits - ->query( - (new BoolQuery()) - ->filter(['term' => ['user_id' => $this->getKey()]]) - ) - ->setAggregations([ - 'by_beatmaps' => [ - 'terms' => [ - 'field' => 'beatmap_id', - // sort by sub-aggregation max_pp, with score_id as tie breaker - 'order' => [['max_pp' => 'desc'], ['min_score_id' => 'asc']], - 'size' => $size, - ], - 'aggs' => [ - 'top_scores' => [ - 'top_hits' => [ - 'size' => 1, - 'sort' => [['pp' => ['order' => 'desc']]], - ], - ], - // top_hits aggregation is not useable for sorting, so we need an extra aggregation to sort on. - 'max_pp' => ['max' => ['field' => 'pp']], - 'min_score_id' => ['min' => ['field' => 'score_id']], - ], - ], - ]); - - $response = $search->response(); - $search->assertNoError(); - - return $response; + return (new FetchDedupedScores('beatmap_id', ScoreSearchParams::fromArray([ + 'is_legacy' => true, + 'limit' => $size, + 'ruleset_id' => Beatmap::MODES[$mode], + 'sort' => 'pp_desc', + 'user_id' => $this->getKey(), + ])))->all(); } public function beatmapBestScoreIds(string $mode) @@ -60,16 +33,13 @@ public function beatmapBestScoreIds(string $mode) // aggregations do not support regular pagination. // always fetching 100 to cache; we're not supporting beyond 100, either. $this->beatmapBestScoreIds[$mode] = cache_remember_mutexed( - "search-cache:beatmapBestScores:{$this->getKey()}:{$mode}", + "search-cache:beatmapBestScoresSolo:{$this->getKey()}:{$mode}", config('osu.scores.es_cache_duration'), [], function () use ($mode) { - // FIXME: should return some sort of error on error - $buckets = $this->aggregatedScoresBest($mode, 100)->aggregations('by_beatmaps')['buckets'] ?? []; + $this->beatmapBestScores[$mode] = $this->aggregatedScoresBest($mode, 100); - return array_map(function ($bucket) { - return array_get($bucket, 'top_scores.hits.hits.0._id'); - }, $buckets); + return array_column($this->beatmapBestScores[$mode], 'id'); }, function () { // TODO: propagate a more useful message back to the client @@ -82,12 +52,18 @@ function () { return $this->beatmapBestScoreIds[$mode]; } - public function beatmapBestScores(string $mode, int $limit, int $offset = 0, $with = []) + public function beatmapBestScores(string $mode, int $limit, int $offset = 0, $with = []): Collection { - $ids = array_slice($this->beatmapBestScoreIds($mode), $offset, $limit); - $clazz = Best\Model::getClass($mode); + $ids = $this->beatmapBestScoreIds($mode); + + if (isset($this->beatmapBestScores[$mode])) { + $results = new Collection(array_slice($this->beatmapBestScores[$mode], $offset, $limit)); + } else { + $ids = array_slice($ids, $offset, $limit); + $results = Score::whereKey($ids)->orderByField('id', $ids)->get(); + } - $results = $clazz::whereIn('score_id', $ids)->orderByField('score_id', $ids)->with($with)->get(); + $results->load($with); // fill in positions for weighting // also preload the user relation From c908529d10685d87de665b1806c09efc73f7f9f1 Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 31 Oct 2022 21:36:22 +0900 Subject: [PATCH 05/10] Order alphabetically --- app/Models/Solo/Score.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Models/Solo/Score.php b/app/Models/Solo/Score.php index a6507f001e1..2be4ed1074f 100644 --- a/app/Models/Solo/Score.php +++ b/app/Models/Solo/Score.php @@ -101,6 +101,11 @@ public function user() return $this->belongsTo(User::class, 'user_id'); } + public function scopeDefault(Builder $query): Builder + { + return $query->whereHas('beatmap'); + } + /** * This should match the one used in osu-elastic-indexer. */ @@ -111,11 +116,6 @@ public function scopeIndexable(Builder $query): Builder ->whereHas('user', fn (Builder $q): Builder => $q->default()); } - public function scopeDefault(Builder $query): Builder - { - return $query->whereHas('beatmap'); - } - public function getAttribute($key) { return match ($key) { From 904a6763272d1e50367f2743988d6b422b5c8cab Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 14 Dec 2022 13:39:41 +0900 Subject: [PATCH 06/10] Actually support sorting scores by pp --- app/Libraries/Search/ScoreSearchParams.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/Libraries/Search/ScoreSearchParams.php b/app/Libraries/Search/ScoreSearchParams.php index 9ad2d4bc5ea..946fca9cc64 100644 --- a/app/Libraries/Search/ScoreSearchParams.php +++ b/app/Libraries/Search/ScoreSearchParams.php @@ -88,6 +88,12 @@ public function setSort(?string $sort): void new Sort('id', 'asc'), ]; break; + case 'pp_desc': + $this->sorts = [ + new Sort('pp', 'desc'), + new Sort('id', 'asc'), + ]; + break; case null: $this->sorts = []; break; From e6d76326dafd0882f59aa48ecfad4f3fcf9ed25f Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 20 Dec 2022 17:28:02 +0900 Subject: [PATCH 07/10] Tag scores best search --- app/Libraries/Elasticsearch/Search.php | 4 +--- app/Libraries/Score/FetchDedupedScores.php | 8 ++++++-- app/Models/Traits/UserScoreable.php | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/Libraries/Elasticsearch/Search.php b/app/Libraries/Elasticsearch/Search.php index 3aaca42a783..e2169f6a26b 100644 --- a/app/Libraries/Elasticsearch/Search.php +++ b/app/Libraries/Elasticsearch/Search.php @@ -22,10 +22,8 @@ abstract class Search extends HasSearch implements Queryable /** * A tag to use when logging timing of fetches. * FIXME: context-based tagging would be nicer. - * - * @var string|null */ - public $loggingTag; + public ?string $loggingTag; protected $aggregations; protected $index; diff --git a/app/Libraries/Score/FetchDedupedScores.php b/app/Libraries/Score/FetchDedupedScores.php index cdbcc2c2ffe..f796fda7d9d 100644 --- a/app/Libraries/Score/FetchDedupedScores.php +++ b/app/Libraries/Score/FetchDedupedScores.php @@ -15,8 +15,11 @@ class FetchDedupedScores private int $limit; private array $result; - public function __construct(private string $dedupeColumn, private ScoreSearchParams $params) - { + public function __construct( + private string $dedupeColumn, + private ScoreSearchParams $params, + private ?string $searchLoggingTag = null + ) { $this->limit = $this->params->size; } @@ -24,6 +27,7 @@ public function all(): array { $this->params->size = $this->limit + 50; $search = new ScoreSearch($this->params); + $search->loggingTag = $this->searchLoggingTag; $nextCursor = null; $hasNext = true; diff --git a/app/Models/Traits/UserScoreable.php b/app/Models/Traits/UserScoreable.php index 54add569b1c..5e6efa9cab9 100644 --- a/app/Models/Traits/UserScoreable.php +++ b/app/Models/Traits/UserScoreable.php @@ -24,7 +24,7 @@ public function aggregatedScoresBest(string $mode, int $size): array 'ruleset_id' => Beatmap::MODES[$mode], 'sort' => 'pp_desc', 'user_id' => $this->getKey(), - ])))->all(); + ]), "aggregatedScoresBest_{$mode}"))->all(); } public function beatmapBestScoreIds(string $mode) From 16ece1e07cb045378322944f3414a4dbd82ce0f7 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 3 Jan 2023 18:36:08 +0900 Subject: [PATCH 08/10] Filter out score without beatmap --- app/Models/Traits/UserScoreable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/Traits/UserScoreable.php b/app/Models/Traits/UserScoreable.php index 5e6efa9cab9..20abc6082bb 100644 --- a/app/Models/Traits/UserScoreable.php +++ b/app/Models/Traits/UserScoreable.php @@ -60,7 +60,7 @@ public function beatmapBestScores(string $mode, int $limit, int $offset = 0, $wi $results = new Collection(array_slice($this->beatmapBestScores[$mode], $offset, $limit)); } else { $ids = array_slice($ids, $offset, $limit); - $results = Score::whereKey($ids)->orderByField('id', $ids)->get(); + $results = Score::whereKey($ids)->orderByField('id', $ids)->whereHas('beatmap.beatmapset')->get(); } $results->load($with); From 739822d4054bef037e2846b6d86943acb6198644 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Mar 2023 21:41:53 +0900 Subject: [PATCH 09/10] Use solo scores index for beatmap pack completion check --- app/Libraries/Search/ScoreSearch.php | 3 + app/Libraries/Search/ScoreSearchParams.php | 2 + app/Models/BeatmapPack.php | 108 ++++++++---------- .../Models/BeatmapPackUserCompletionTest.php | 105 ++++++++++++----- 4 files changed, 129 insertions(+), 89 deletions(-) diff --git a/app/Libraries/Search/ScoreSearch.php b/app/Libraries/Search/ScoreSearch.php index 50736e686de..92bc5d5c3d9 100644 --- a/app/Libraries/Search/ScoreSearch.php +++ b/app/Libraries/Search/ScoreSearch.php @@ -36,6 +36,9 @@ public function getQuery(): BoolQuery { $query = new BoolQuery(); + if ($this->params->excludeConverts) { + $query->filter(['term' => ['convert' => false]]); + } if (config('osu.scores.es_enable_legacy_filter') && $this->params->isLegacy !== null) { $query->filter(['term' => ['is_legacy' => $this->params->isLegacy]]); } diff --git a/app/Libraries/Search/ScoreSearchParams.php b/app/Libraries/Search/ScoreSearchParams.php index 9ad2d4bc5ea..69053786f99 100644 --- a/app/Libraries/Search/ScoreSearchParams.php +++ b/app/Libraries/Search/ScoreSearchParams.php @@ -21,6 +21,7 @@ public static function fromArray(array $rawParams): static { $params = new static(); $params->beatmapIds = $rawParams['beatmap_ids'] ?? null; + $params->excludeConverts = $rawParams['exclude_converts'] ?? $params->excludeConverts; $params->excludeMods = $rawParams['exclude_mods'] ?? null; $params->isLegacy = $rawParams['is_legacy'] ?? null; $params->mods = $rawParams['mods'] ?? null; @@ -42,6 +43,7 @@ public static function fromArray(array $rawParams): static public ?array $beatmapIds = null; public ?Score $beforeScore = null; public ?int $beforeTotalScore = null; + public bool $excludeConverts = false; public ?array $excludeMods = null; public ?bool $isLegacy = null; public ?array $mods = null; diff --git a/app/Models/BeatmapPack.php b/app/Models/BeatmapPack.php index b57b9849ebe..45c1c09e1c1 100644 --- a/app/Models/BeatmapPack.php +++ b/app/Models/BeatmapPack.php @@ -5,7 +5,9 @@ namespace App\Models; -use Exception; +use App\Libraries\Search\ScoreSearch; +use App\Libraries\Search\ScoreSearchParams; +use Ds\Set; /** * @property string $author @@ -86,65 +88,55 @@ public function userCompletionData($user) { if ($user !== null) { $userId = $user->getKey(); - $beatmapsetIds = $this->items()->pluck('beatmapset_id')->all(); - $query = Beatmap::select('beatmapset_id')->distinct()->whereIn('beatmapset_id', $beatmapsetIds); - - if ($this->playmode === null) { - static $scoreRelations; - - // generate list of beatmap->score relation names for each modes - // store int mode as well as it'll be used for filtering the scores - if (!isset($scoreRelations)) { - $scoreRelations = []; - foreach (Beatmap::MODES as $modeStr => $modeInt) { - $scoreRelations[] = [ - 'playmode' => $modeInt, - 'relation' => camel_case("scores_best_{$modeStr}"), - ]; - } - } - - // outer where function - // The idea is SELECT ... WHERE ... AND ( OR OR ...). - $query->where(function ($q) use ($scoreRelations, $userId) { - foreach ($scoreRelations as $scoreRelation) { - // The scores> mentioned above is generated here. - // As it's "playmode = AND EXISTS (< score for user>)", - // wrap them so it's not flat "playmode = AND EXISTS ... OR playmode = AND EXISTS ...". - $q->orWhere(function ($qq) use ($scoreRelation, $userId) { - $qq - // this playmode filter ensures the scores are limited to non-convert maps - ->where('playmode', '=', $scoreRelation['playmode']) - ->whereHas($scoreRelation['relation'], function ($scoreQuery) use ($userId) { - $scoreQuery->where('user_id', '=', $userId); - - if ($this->no_diff_reduction) { - $scoreQuery->withoutMods(app('mods')->difficultyReductionIds->toArray()); - } - }); - }); - } - }); - } else { - $modeStr = Beatmap::modeStr($this->playmode); - - if ($modeStr === null) { - throw new Exception("beatmapset pack {$this->getKey()} has invalid playmode: {$this->playmode}"); - } - - $scoreRelation = camel_case("scores_best_{$modeStr}"); - - $query->whereHas($scoreRelation, function ($query) use ($userId) { - $query->where('user_id', '=', $userId); - - if ($this->no_diff_reduction) { - $query->withoutMods(app('mods')->difficultyReductionIds->toArray()); - } - }); + + $beatmaps = Beatmap + ::whereIn('beatmapset_id', $this->items()->select('beatmapset_id')) + ->select(['beatmap_id', 'beatmapset_id', 'playmode']) + ->get(); + $beatmapsetIdsByBeatmapId = []; + foreach ($beatmaps as $beatmap) { + $beatmapsetIdsByBeatmapId[$beatmap->beatmap_id] = $beatmap->beatmapset_id; + } + $params = [ + 'beatmap_ids' => array_keys($beatmapsetIdsByBeatmapId), + 'exclude_converts' => $this->playmode === null, + 'is_legacy' => true, + 'limit' => 0, + 'ruleset_id' => $this->playmode, + 'user_id' => $userId, + ]; + if ($this->no_diff_reduction) { + $params['exclude_mods'] = app('mods')->difficultyReductionIds->toArray(); } - $completedBeatmapsetIds = $query->pluck('beatmapset_id')->all(); - $completed = count($completedBeatmapsetIds) === count($beatmapsetIds); + static $aggName = 'by_beatmap'; + + $search = new ScoreSearch(ScoreSearchParams::fromArray($params)); + $search->size(0); + $search->setAggregations([$aggName => [ + 'terms' => [ + 'field' => 'beatmap_id', + 'size' => max(1, count($params['beatmap_ids'])), + ], + 'aggs' => [ + 'scores' => [ + 'top_hits' => [ + 'size' => 1, + ], + ], + ], + ]]); + $response = $search->response(); + $search->assertNoError(); + $completedBeatmapIds = array_map( + fn (array $hit): int => (int) $hit['key'], + $response->aggregations($aggName)['buckets'], + ); + $completedBeatmapsetIds = (new Set(array_map( + fn (int $beatmapId): int => $beatmapsetIdsByBeatmapId[$beatmapId], + $completedBeatmapIds, + )))->toArray(); + $completed = count($completedBeatmapsetIds) === count(array_unique($beatmapsetIdsByBeatmapId)); } return [ diff --git a/tests/Models/BeatmapPackUserCompletionTest.php b/tests/Models/BeatmapPackUserCompletionTest.php index 12122c1225a..db97b27a888 100644 --- a/tests/Models/BeatmapPackUserCompletionTest.php +++ b/tests/Models/BeatmapPackUserCompletionTest.php @@ -7,53 +7,96 @@ namespace Tests\Models; +use App\Libraries\Search\ScoreSearch; use App\Models\Beatmap; use App\Models\BeatmapPack; -use App\Models\Score\Best as ScoreBest; +use App\Models\BeatmapPackItem; +use App\Models\Beatmapset; +use App\Models\Country; +use App\Models\Genre; +use App\Models\Group; +use App\Models\Language; +use App\Models\Solo\Score; use App\Models\User; +use App\Models\UserGroup; +use App\Models\UserGroupEvent; use Tests\TestCase; +/** + * @group EsSoloScores + */ class BeatmapPackUserCompletionTest extends TestCase { - /** - * @dataProvider dataProviderForTestBasic - */ - public function testBasic(string $userType, ?string $packRuleset, bool $completed): void + private static array $users; + private static BeatmapPack $pack; + + public static function setUpBeforeClass(): void { + parent::setUpBeforeClass(); + + (new static())->refreshApplication(); $beatmap = Beatmap::factory()->ranked()->state([ 'playmode' => Beatmap::MODES['taiko'], ])->create(); - $pack = BeatmapPack::factory()->create(); - $pack->items()->create(['beatmapset_id' => $beatmap->beatmapset_id]); - - $scoreUser = User::factory()->create(); - $scoreClass = ScoreBest\Taiko::class; - switch ($userType) { - case 'convertOsu': - $checkUser = $scoreUser; - $scoreClass = ScoreBest\Osu::class; - break; - case 'default': - $checkUser = $scoreUser; - break; - case 'null': - $checkUser = null; - break; - case 'unrelated': - $checkUser = User::factory()->create(); - break; - } - - $scoreClass::factory()->create([ + static::$pack = BeatmapPack::factory()->create(); + static::$pack->items()->create(['beatmapset_id' => $beatmap->beatmapset_id]); + + static::$users = [ + 'convertOsu' => User::factory()->create(), + 'default' => User::factory()->create(), + 'null' => null, + 'unrelated' => User::factory()->create(), + ]; + + Score::factory()->create([ 'beatmap_id' => $beatmap, - 'user_id' => $scoreUser->getKey(), + 'ruleset_id' => Beatmap::MODES['osu'], + 'preserve' => true, + 'user_id' => static::$users['convertOsu'], ]); + Score::factory()->create([ + 'beatmap_id' => $beatmap, + 'preserve' => true, + 'user_id' => static::$users['default'], + ]); + + static::reindexScores(); + } + + public static function tearDownAfterClass(): void + { + parent::tearDownAfterClass(); + + (new static())->refreshApplication(); + Beatmap::truncate(); + BeatmapPack::truncate(); + BeatmapPackItem::truncate(); + Beatmapset::truncate(); + Country::truncate(); + Genre::truncate(); + Group::truncate(); + Language::truncate(); + Score::truncate(); + User::truncate(); + UserGroup::truncate(); + UserGroupEvent::truncate(); + (new ScoreSearch())->deleteAll(); + } + + protected $connectionsToTransact = []; + + /** + * @dataProvider dataProviderForTestBasic + */ + public function testBasic(string $userType, ?string $packRuleset, bool $completed): void + { + $user = static::$users[$userType]; $rulesetId = $packRuleset === null ? null : Beatmap::MODES[$packRuleset]; - $pack->update(['playmode' => $rulesetId]); - $pack->refresh(); + static::$pack->update(['playmode' => $rulesetId]); + static::$pack->refresh(); - $data = $pack->userCompletionData($checkUser); + $data = static::$pack->userCompletionData($user); $this->assertSame($completed ? 1 : 0, count($data['beatmapset_ids'])); $this->assertSame($completed, $data['completed']); } From 4bbe38ebc2ecd4eb4cf36d2864372df370ae09dc Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 31 Oct 2022 22:02:53 +0900 Subject: [PATCH 10/10] Use solo scores for profile page recent scores --- app/Http/Controllers/UsersController.php | 9 ++++++--- app/Models/Solo/Score.php | 12 ++++++++++++ app/Models/User.php | 18 ++++++++++++++++++ app/Transformers/UserCompactTransformer.php | 2 +- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/UsersController.php b/app/Http/Controllers/UsersController.php index f3f1156d9b7..06ebfb69eab 100644 --- a/app/Http/Controllers/UsersController.php +++ b/app/Http/Controllers/UsersController.php @@ -178,7 +178,7 @@ public function extraPages($_id, $page) 'monthly_playcounts' => json_collection($this->user->monthlyPlaycounts, new UserMonthlyPlaycountTransformer()), 'recent' => $this->getExtraSection( 'scoresRecent', - $this->user->scores($this->mode, true)->includeFails(false)->count() + $this->user->recentScoreCount($this->mode) ), 'replays_watched_counts' => json_collection($this->user->replaysWatchedCounts, new UserReplaysWatchedCountTransformer()), ]; @@ -816,9 +816,12 @@ private function getExtra($page, array $options, int $perPage = 10, int $offset case 'scoresRecent': $transformer = new ScoreTransformer(); $includes = ScoreTransformer::USER_PROFILE_INCLUDES; - $query = $this->user->scores($this->mode, true) + $query = $this->user->soloScores() + ->default() + ->forRuleset($this->mode) ->includeFails($options['includeFails'] ?? false) - ->with([...ScoreTransformer::USER_PROFILE_INCLUDES_PRELOAD, 'best']); + ->reorderBy('id', 'desc') + ->with(ScoreTransformer::USER_PROFILE_INCLUDES_PRELOAD); $userRelationColumn = 'user'; break; } diff --git a/app/Models/Solo/Score.php b/app/Models/Solo/Score.php index 2be4ed1074f..9a688db4c15 100644 --- a/app/Models/Solo/Score.php +++ b/app/Models/Solo/Score.php @@ -106,6 +106,18 @@ public function scopeDefault(Builder $query): Builder return $query->whereHas('beatmap'); } + public function scopeForRuleset(Builder $query, string $ruleset): Builder + { + return $query->where('ruleset_id', Beatmap::MODES[$ruleset]); + } + + public function scopeIncludeFails(Builder $query, bool $includeFails): Builder + { + return $includeFails + ? $query + : $query->where('data->passed', true); + } + /** * This should match the one used in osu-elastic-indexer. */ diff --git a/app/Models/User.php b/app/Models/User.php index 91753b37777..afe9a3185c5 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -916,6 +916,7 @@ public function getAttribute($key) 'scoresMania', 'scoresOsu', 'scoresTaiko', + 'soloScores', 'statisticsFruits', 'statisticsMania', 'statisticsOsu', @@ -1427,6 +1428,11 @@ public function scoresBest(string $mode, bool $returnQuery = false) return $returnQuery ? $this->$relation() : $this->$relation; } + public function soloScores(): HasMany + { + return $this->hasMany(Solo\Score::class); + } + public function topicWatches() { return $this->hasMany(TopicWatch::class); @@ -1776,6 +1782,18 @@ public function authHash(): string return hash('sha256', $this->user_email).':'.hash('sha256', $this->user_password); } + public function recentScoreCount(string $ruleset): int + { + return $this->soloScores() + ->default() + ->forRuleset($ruleset) + ->includeFails(false) + ->select('id') + ->limit(100) + ->get() + ->count(); + } + public function resetSessions(?string $excludedSessionId = null): void { SessionStore::destroy($this->getKey(), $excludedSessionId); diff --git a/app/Transformers/UserCompactTransformer.php b/app/Transformers/UserCompactTransformer.php index f1f3603d0ba..56d91c3cede 100644 --- a/app/Transformers/UserCompactTransformer.php +++ b/app/Transformers/UserCompactTransformer.php @@ -383,7 +383,7 @@ public function includeScoresPinnedCount(User $user) public function includeScoresRecentCount(User $user) { - return $this->primitive($user->scores($this->mode, true)->includeFails(false)->count()); + return $this->primitive($user->recentScoreCount($this->mode)); } public function includeStatistics(User $user)