From 2daa48940c030eff0745170149ed513a160431dc Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 21 Oct 2024 17:36:52 +0900 Subject: [PATCH 01/27] Fix percentile related key names --- app/Models/DailyChallengeUserStats.php | 2 +- app/Models/Multiplayer/PlaylistItem.php | 10 +++++----- resources/lang/en/rankings.php | 4 ++-- resources/views/rankings/daily_challenge.blade.php | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/Models/DailyChallengeUserStats.php b/app/Models/DailyChallengeUserStats.php index 564f0986a20..d91d0394ca2 100644 --- a/app/Models/DailyChallengeUserStats.php +++ b/app/Models/DailyChallengeUserStats.php @@ -178,7 +178,7 @@ private function updatePercentile( foreach ($playlistPercentile as $p => $totalScore) { if ($highScore->total_score >= $totalScore) { - $this->{"top_{$p}_placements"}++; + $this->{"{$p}_placements"}++; } } $this->last_percentile_calculation = $startTime; diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 117961b5cb6..92ed3817d8f 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -119,7 +119,7 @@ public function save(array $options = []) public function scorePercentile(): array { - $key = "playlist_item_score_percentile:{$this->getKey()}"; + $key = "playlist_item_score_percentile:v2:{$this->getKey()}"; if (!$this->expired && !$this->room->hasEnded()) { $key .= ':ongoing'; @@ -134,11 +134,11 @@ public function scorePercentile(): array return $count === 0 ? [ - '10p' => 0, - '50p' => 0, + 'top_10p' => 0, + 'top_50p' => 0, ] : [ - '10p' => $scores[max(0, (int) ($count * 0.1) - 1)], - '50p' => $scores[max(0, (int) ($count * 0.5) - 1)], + 'top_10p' => $scores[max(0, (int) ($count * 0.1) - 1)], + 'top_50p' => $scores[max(0, (int) ($count * 0.5) - 1)], ]; }); } diff --git a/resources/lang/en/rankings.php b/resources/lang/en/rankings.php index d5b813c8645..e1dc187c861 100644 --- a/resources/lang/en/rankings.php +++ b/resources/lang/en/rankings.php @@ -11,8 +11,8 @@ 'daily_challenge' => [ 'beatmap' => 'Difficulty', - 'percentile_10' => 'Top 10% Score', - 'percentile_50' => 'Top 50% Score', + 'top_10p' => 'Top 10% Score', + 'top_50p' => 'Top 50% Score', ], 'filter' => [ diff --git a/resources/views/rankings/daily_challenge.blade.php b/resources/views/rankings/daily_challenge.blade.php index 8636cd291dc..96228e5166d 100644 --- a/resources/views/rankings/daily_challenge.blade.php +++ b/resources/views/rankings/daily_challenge.blade.php @@ -52,18 +52,18 @@
- {{ osu_trans('rankings.daily_challenge.percentile_10') }} + {{ osu_trans('rankings.daily_challenge.top_10p') }}
- {{ i18n_number_format($percentile['10p']) }} + {{ i18n_number_format($percentile['top_10p']) }}
- {{ osu_trans('rankings.daily_challenge.percentile_50') }} + {{ osu_trans('rankings.daily_challenge.top_50p') }}
- {{ i18n_number_format($percentile['50p']) }} + {{ i18n_number_format($percentile['top_50p']) }}
From 4679c080874294b38e8e140c4a9c61e32b6edbce Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 21 Oct 2024 20:48:16 +0900 Subject: [PATCH 02/27] Pass timestamp to elasticsearch for beatmap search Elasticsearch json time is limited to four digit years. --- .../Search/BeatmapsetQueryParser.php | 61 +++++++++---------- .../Search/BeatmapsetQueryParserTest.php | 40 ++++++------ 2 files changed, 51 insertions(+), 50 deletions(-) diff --git a/app/Libraries/Search/BeatmapsetQueryParser.php b/app/Libraries/Search/BeatmapsetQueryParser.php index b47b67c46b2..faa4f51b8cd 100644 --- a/app/Libraries/Search/BeatmapsetQueryParser.php +++ b/app/Libraries/Search/BeatmapsetQueryParser.php @@ -6,7 +6,7 @@ namespace App\Libraries\Search; use App\Models\Beatmapset; -use Carbon\Carbon; +use Carbon\CarbonImmutable; class BeatmapsetQueryParser { @@ -114,43 +114,38 @@ private static function makeDateRangeOption(string $operator, string $value): ?a $value = presence(trim($value, '"')); if (preg_match('#^\d{4}$#', $value) === 1) { - $startTime = Carbon::create($value, 1, 1, 0, 0, 0, 'UTC'); - $endTimeFunction = 'addYears'; + $startTime = CarbonImmutable::create($value, 1, 1, 0, 0, 0, 'UTC'); + $endTime = $startTime->addYears(1); } elseif (preg_match('#^(?\d{4})[-./]?(?\d{1,2})$#', $value, $m) === 1) { - $startTime = Carbon::create($m['year'], $m['month'], 1, 0, 0, 0, 'UTC'); - $endTimeFunction = 'addMonths'; + $startTime = CarbonImmutable::create($m['year'], $m['month'], 1, 0, 0, 0, 'UTC'); + $endTime = $startTime->addMonths(1); } elseif (preg_match('#^(?\d{4})[-./]?(?\d{1,2})[-./]?(?\d{1,2})$#', $value, $m) === 1) { - $startTime = Carbon::create($m['year'], $m['month'], $m['day'], 0, 0, 0, 'UTC'); - $endTimeFunction = 'addDays'; + $startTime = CarbonImmutable::create($m['year'], $m['month'], $m['day'], 0, 0, 0, 'UTC'); + $endTime = $startTime->addDays(1); } else { - $startTime = parse_time_to_carbon($value)?->utc(); - $endTimeFunction = 'addSeconds'; + $startTime = parse_time_to_carbon($value)?->toImmutable()->utc(); + $endTime = $startTime?->addSeconds(1); } - if (isset($startTime) && isset($endTimeFunction)) { - switch ($operator) { - case '=': - return [ - 'gte' => json_time($startTime), - 'lt' => json_time($startTime->$endTimeFunction()), - ]; - case '<': - return [ - 'lt' => json_time($startTime), - ]; - case '<=': - return [ - 'lt' => json_time($startTime->$endTimeFunction()), - ]; - case '>': - return [ - 'gte' => json_time($startTime->$endTimeFunction()), - ]; - case '>=': - return [ - 'gte' => json_time($startTime), - ]; - } + if (isset($startTime) && isset($endTime)) { + return match ($operator) { + '=' => [ + 'gte' => $startTime->getTimestampMs(), + 'lt' => $endTime->getTimestampMs(), + ], + '<' => [ + 'lt' => $startTime->getTimestampMs(), + ], + '<=' => [ + 'lt' => $endTime->getTimestampMs(), + ], + '>' => [ + 'gte' => $endTime->getTimestampMs(), + ], + '>=' => [ + 'gte' => $startTime->getTimestampMs(), + ], + }; } return null; diff --git a/tests/Libraries/Search/BeatmapsetQueryParserTest.php b/tests/Libraries/Search/BeatmapsetQueryParserTest.php index 7d1ea0587d0..273f6a07a54 100644 --- a/tests/Libraries/Search/BeatmapsetQueryParserTest.php +++ b/tests/Libraries/Search/BeatmapsetQueryParserTest.php @@ -7,18 +7,11 @@ use App\Libraries\Search\BeatmapsetQueryParser; use App\Models\Beatmapset; +use Carbon\CarbonImmutable; use Tests\TestCase; class BeatmapsetQueryParserTest extends TestCase { - /** - * @dataProvider queryDataProvider - */ - public function testParse(?string $query, ?array $expected) - { - $this->assertSame(json_encode($expected), json_encode(BeatmapsetQueryParser::parse($query))); - } - public static function queryDataProvider() { return [ @@ -43,15 +36,15 @@ public static function queryDataProvider() ['creator=hello', ['keywords' => null, 'options' => ['creator' => 'hello']]], ['artist=hello', ['keywords' => null, 'options' => ['artist' => 'hello']]], ['artist="hello world"', ['keywords' => null, 'options' => ['artist' => 'hello world']]], - ['created=2017', ['keywords' => null, 'options' => ['created' => ['gte' => '2017-01-01T00:00:00+00:00', 'lt' => '2018-01-01T00:00:00+00:00']]]], - ['ranked>2018', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2019-01-01T00:00:00+00:00']]]], - ['ranked<2018-05', ['keywords' => null, 'options' => ['ranked' => ['lt' => '2018-05-01T00:00:00+00:00']]]], - ['ranked<=2018.05', ['keywords' => null, 'options' => ['ranked' => ['lt' => '2018-06-01T00:00:00+00:00']]]], - ['ranked=2018/05', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2018-05-01T00:00:00+00:00', 'lt' => '2018-06-01T00:00:00+00:00']]]], - ['ranked=2018.05.01', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2018-05-01T00:00:00+00:00', 'lt' => '2018-05-02T00:00:00+00:00']]]], - ['ranked>2018/05/01', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2018-05-02T00:00:00+00:00']]]], - ['ranked>="2020-07-21 12:30:30 +09:00"', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2020-07-21T03:30:30+00:00']]]], - ['ranked="2020-07-21 12:30:30 +09:00"', ['keywords' => null, 'options' => ['ranked' => ['gte' => '2020-07-21T03:30:30+00:00', 'lt' => '2020-07-21T03:30:31+00:00']]]], + ['created=2017', ['keywords' => null, 'options' => ['created' => ['gte' => static::parseTime('2017-01-01'), 'lt' => static::parseTime('2018-01-01')]]]], + ['ranked>2018', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2019-01-01')]]]], + ['ranked<2018-05', ['keywords' => null, 'options' => ['ranked' => ['lt' => static::parseTime('2018-05-01')]]]], + ['ranked<=2018.05', ['keywords' => null, 'options' => ['ranked' => ['lt' => static::parseTime('2018-06-01')]]]], + ['ranked=2018/05', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2018-05-01'), 'lt' => static::parseTime('2018-06-01')]]]], + ['ranked=2018.05.01', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2018-05-01'), 'lt' => static::parseTime('2018-05-02')]]]], + ['ranked>2018/05/01', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2018-05-02')]]]], + ['ranked>="2020-07-21 12:30:30 +09:00"', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2020-07-21 03:30:30')]]]], + ['ranked="2020-07-21 12:30:30 +09:00"', ['keywords' => null, 'options' => ['ranked' => ['gte' => static::parseTime('2020-07-21 03:30:30'), 'lt' => static::parseTime('2020-07-21 03:30:31')]]]], ['ranked="invalid date format"', ['keywords' => 'ranked="invalid date format"', 'options' => []]], // multiple options @@ -103,4 +96,17 @@ public static function queryDataProvider() ['status=lo', ['keywords' => null, 'options' => ['status' => ['gte' => Beatmapset::STATES['loved'], 'lte' => Beatmapset::STATES['loved']]]]], ]; } + + private static function parseTime(string $timeString): int + { + return CarbonImmutable::parse($timeString)->getTimestampMs(); + } + + /** + * @dataProvider queryDataProvider + */ + public function testParse(?string $query, ?array $expected) + { + $this->assertSame(json_encode($expected), json_encode(BeatmapsetQueryParser::parse($query))); + } } From 4451e60084f881ec713791eac44906d00074ec89 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 22 Oct 2024 20:23:44 +0900 Subject: [PATCH 03/27] Fix user listing preload scope --- app/Http/Controllers/FriendsController.php | 3 +-- app/Http/Controllers/GroupsController.php | 3 +-- app/Models/User.php | 8 ++++---- app/Transformers/UserCompactTransformer.php | 1 + 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/FriendsController.php b/app/Http/Controllers/FriendsController.php index d86bcdb59e2..5e55b639f28 100644 --- a/app/Http/Controllers/FriendsController.php +++ b/app/Http/Controllers/FriendsController.php @@ -37,8 +37,7 @@ public function index() $friends = $currentUser ->friends() - ->with('statistics'.studly_case($currentMode)) - ->eagerloadForListing() + ->eagerloadForListing($currentMode) ->orderBy('username', 'asc') ->get(); diff --git a/app/Http/Controllers/GroupsController.php b/app/Http/Controllers/GroupsController.php index d02003e05df..1929faafcff 100644 --- a/app/Http/Controllers/GroupsController.php +++ b/app/Http/Controllers/GroupsController.php @@ -16,8 +16,7 @@ public function show($id) $currentMode = default_mode(); $users = $group->users() - ->with('statistics'.studly_case($currentMode)) - ->eagerloadForListing() + ->eagerloadForListing($currentMode) ->default() ->orderBy('username', 'asc') ->get(); diff --git a/app/Models/User.php b/app/Models/User.php index 7823d021f6c..262134cd4af 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -25,6 +25,7 @@ use App\Models\OAuth\Client; use App\Traits\Memoizes; use App\Traits\Validatable; +use App\Transformers\UserCompactTransformer; use Cache; use Carbon\Carbon; use DB; @@ -2012,13 +2013,12 @@ public function scopeOnline($query) ->where('user_lastvisit', '>', time() - $GLOBALS['cfg']['osu']['user']['online_window']); } - public function scopeEagerloadForListing($query) + public function scopeEagerloadForListing($query, string $rulesetName) { return $query->with([ - 'country', + ...UserCompactTransformer::CARD_INCLUDES_PRELOAD, + static::statisticsRelationName($rulesetName), 'supporterTagPurchases', - 'userGroups', - 'userProfileCustomization', ]); } diff --git a/app/Transformers/UserCompactTransformer.php b/app/Transformers/UserCompactTransformer.php index 1179d9a9e2c..075084b78b6 100644 --- a/app/Transformers/UserCompactTransformer.php +++ b/app/Transformers/UserCompactTransformer.php @@ -25,6 +25,7 @@ class UserCompactTransformer extends TransformerAbstract 'userGroups', ]; + // Use `eagerloadForListing` scope for preloading. const LIST_INCLUDES = [ ...self::CARD_INCLUDES, 'statistics', From cea89217b99432c7eb192f8a82eb92c0f2938633 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 22 Oct 2024 20:52:17 +0900 Subject: [PATCH 04/27] Move to the transformer itself --- app/Http/Controllers/FriendsController.php | 2 +- app/Http/Controllers/GroupsController.php | 2 +- app/Models/User.php | 10 ---------- app/Transformers/UserCompactTransformer.php | 11 ++++++++++- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/FriendsController.php b/app/Http/Controllers/FriendsController.php index 5e55b639f28..40383979c97 100644 --- a/app/Http/Controllers/FriendsController.php +++ b/app/Http/Controllers/FriendsController.php @@ -37,7 +37,7 @@ public function index() $friends = $currentUser ->friends() - ->eagerloadForListing($currentMode) + ->with(UserCompactTransformer::listIncludesPreload($currentMode)) ->orderBy('username', 'asc') ->get(); diff --git a/app/Http/Controllers/GroupsController.php b/app/Http/Controllers/GroupsController.php index 1929faafcff..a5761fa02f6 100644 --- a/app/Http/Controllers/GroupsController.php +++ b/app/Http/Controllers/GroupsController.php @@ -16,7 +16,7 @@ public function show($id) $currentMode = default_mode(); $users = $group->users() - ->eagerloadForListing($currentMode) + ->with(UserCompactTransformer::listIncludesPreload($currentMode)) ->default() ->orderBy('username', 'asc') ->get(); diff --git a/app/Models/User.php b/app/Models/User.php index 262134cd4af..da10756d17b 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -25,7 +25,6 @@ use App\Models\OAuth\Client; use App\Traits\Memoizes; use App\Traits\Validatable; -use App\Transformers\UserCompactTransformer; use Cache; use Carbon\Carbon; use DB; @@ -2013,15 +2012,6 @@ public function scopeOnline($query) ->where('user_lastvisit', '>', time() - $GLOBALS['cfg']['osu']['user']['online_window']); } - public function scopeEagerloadForListing($query, string $rulesetName) - { - return $query->with([ - ...UserCompactTransformer::CARD_INCLUDES_PRELOAD, - static::statisticsRelationName($rulesetName), - 'supporterTagPurchases', - ]); - } - public function checkPassword($password) { return Hash::check($password, $this->getAuthPassword()); diff --git a/app/Transformers/UserCompactTransformer.php b/app/Transformers/UserCompactTransformer.php index 075084b78b6..461eba7f7af 100644 --- a/app/Transformers/UserCompactTransformer.php +++ b/app/Transformers/UserCompactTransformer.php @@ -25,7 +25,7 @@ class UserCompactTransformer extends TransformerAbstract 'userGroups', ]; - // Use `eagerloadForListing` scope for preloading. + // Paired with static::listIncludesPreload const LIST_INCLUDES = [ ...self::CARD_INCLUDES, 'statistics', @@ -113,6 +113,15 @@ class UserCompactTransformer extends TransformerAbstract 'is_silenced' => 'IsNotOAuth', ]; + public static function listIncludesPreload(string $rulesetName): array + { + return [ + ...static::CARD_INCLUDES_PRELOAD, + User::statisticsRelationName($rulesetName), + 'supporterTagPurchases', + ]; + } + public function transform(User $user) { return [ From fcf96a115f8c761a682314f8e692b24359a60fb3 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 22 Oct 2024 21:20:40 +0900 Subject: [PATCH 05/27] Return user relation json for friends listing endpoint Requires specifying api version 20241022 or higher. --- app/Http/Controllers/FriendsController.php | 32 ++++++++++++++------ app/Models/User.php | 6 ++++ app/Transformers/UserCompactTransformer.php | 2 +- app/Transformers/UserRelationTransformer.php | 12 ++++++-- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/FriendsController.php b/app/Http/Controllers/FriendsController.php index 40383979c97..d8046a3fb2d 100644 --- a/app/Http/Controllers/FriendsController.php +++ b/app/Http/Controllers/FriendsController.php @@ -9,6 +9,7 @@ use App\Models\User; use App\Models\UserRelation; use App\Transformers\UserCompactTransformer; +use App\Transformers\UserRelationTransformer; use Auth; use Exception; @@ -35,23 +36,34 @@ public function index() $currentUser = auth()->user(); $currentMode = default_mode(); - $friends = $currentUser - ->friends() - ->with(UserCompactTransformer::listIncludesPreload($currentMode)) - ->orderBy('username', 'asc') - ->get(); + $relationFriends = $currentUser->relationFriends->sortBy('username'); + $relationFriends->load(array_map( + fn ($userPreload) => "target.{$userPreload}", + UserCompactTransformer::listIncludesPreload($currentMode), + )); + + $isApi = is_api_request(); + + if ($isApi && api_version() >= 20241022) { + return json_collection($relationFriends, new UserRelationTransformer(), [ + "target:ruleset({$currentMode})", + ...array_map( + fn ($userInclude) => "target.{$userInclude}", + UserCompactTransformer::LIST_INCLUDES, + ), + ]); + } + $friends = $relationFriends->pluck('target'); $usersJson = json_collection( $friends, (new UserCompactTransformer())->setMode($currentMode), UserCompactTransformer::LIST_INCLUDES ); - if (is_api_request()) { - return $usersJson; - } - - return ext_view('friends.index', compact('usersJson')); + return $isApi + ? $usersJson + : ext_view('friends.index', compact('usersJson')); } public function store() diff --git a/app/Models/User.php b/app/Models/User.php index da10756d17b..5a03d87eb1d 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -923,6 +923,7 @@ public function getAttribute($key) 'rankHighests', 'rankHistories', 'receivedKudosu', + 'relationFriends', 'relations', 'replaysWatchedCounts', 'reportedIn', @@ -1518,6 +1519,11 @@ public function usernameChangeHistoryPublic() ->orderBy('timestamp', 'ASC'); } + public function relationFriends(): HasMany + { + return $this->relations()->friends()->withMutual(); + } + public function relations() { return $this->hasMany(UserRelation::class); diff --git a/app/Transformers/UserCompactTransformer.php b/app/Transformers/UserCompactTransformer.php index 461eba7f7af..c0f628e601d 100644 --- a/app/Transformers/UserCompactTransformer.php +++ b/app/Transformers/UserCompactTransformer.php @@ -255,7 +255,7 @@ public function includeFollowerCount(User $user) public function includeFriends(User $user) { return $this->collection( - $user->relations()->friends()->withMutual()->get(), + $user->relationFriends, new UserRelationTransformer() ); } diff --git a/app/Transformers/UserRelationTransformer.php b/app/Transformers/UserRelationTransformer.php index 7e0819e17eb..291bec3cffa 100644 --- a/app/Transformers/UserRelationTransformer.php +++ b/app/Transformers/UserRelationTransformer.php @@ -6,6 +6,7 @@ namespace App\Transformers; use App\Models\UserRelation; +use League\Fractal\ParamBag; class UserRelationTransformer extends TransformerAbstract { @@ -23,8 +24,15 @@ public function transform(UserRelation $userRelation) ]; } - public function includeTarget(UserRelation $userRelation) + public function includeTarget(UserRelation $userRelation, ?ParamBag $params = null) { - return $this->item($userRelation->target, new UserCompactTransformer()); + $transformer = new UserCompactTransformer(); + + $rulesetName = $params?->get('ruleset')[0]; + if ($rulesetName !== null) { + $transformer->setMode($rulesetName); + } + + return $this->item($userRelation->target, $transformer); } } From 923fd709d12048a8b1ad6a7444408bfb37bf2a3d Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 22 Oct 2024 21:52:46 +0900 Subject: [PATCH 06/27] Fix incorrect mutual state when blocking user --- app/Models/UserRelation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/UserRelation.php b/app/Models/UserRelation.php index 7fd6d2560b9..a211f8c6368 100644 --- a/app/Models/UserRelation.php +++ b/app/Models/UserRelation.php @@ -64,7 +64,7 @@ public function scopeWithMutual($query) { $selfJoin = 'COALESCE(( - SELECT 1 + SELECT phpbb_zebra.friend FROM phpbb_zebra z WHERE phpbb_zebra.zebra_id = z.user_id AND z.zebra_id = phpbb_zebra.user_id From 2a1c04f07df7e21d4823b8b2c16c0ee89ebb8038 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 24 Oct 2024 18:35:43 +0900 Subject: [PATCH 07/27] Use native uuid generator function --- resources/js/models/chat/create-announcement.ts | 3 +-- resources/js/models/chat/message.ts | 3 +-- resources/js/turbolinks-overrides.coffee | 2 +- resources/js/turbolinks.d.ts | 1 - resources/js/utils/seq.ts | 4 ---- 5 files changed, 3 insertions(+), 10 deletions(-) diff --git a/resources/js/models/chat/create-announcement.ts b/resources/js/models/chat/create-announcement.ts index 51742489230..f2028d673fc 100644 --- a/resources/js/models/chat/create-announcement.ts +++ b/resources/js/models/chat/create-announcement.ts @@ -3,7 +3,6 @@ import UserJson from 'interfaces/user-json'; import { action, autorun, computed, makeObservable, observable } from 'mobx'; -import { uuid } from 'utils/seq'; import { present } from 'utils/string'; import { maxMessageLength } from './channel'; @@ -34,7 +33,7 @@ export default class CreateAnnouncement { @observable validUsers = new Map(); private initialized = false; - private readonly uuid = uuid(); + private readonly uuid = crypto.randomUUID(); @computed get errors() { diff --git a/resources/js/models/chat/message.ts b/resources/js/models/chat/message.ts index bedc568e47a..f266aabd43b 100644 --- a/resources/js/models/chat/message.ts +++ b/resources/js/models/chat/message.ts @@ -6,14 +6,13 @@ import { action, computed, makeObservable, observable } from 'mobx'; import User from 'models/user'; import * as moment from 'moment'; import core from 'osu-core-singleton'; -import { uuid } from 'utils/seq'; export default class Message { @observable channelId = -1; @observable content = ''; @observable errored = false; @observable isAction = false; - @observable messageId: number | string = uuid(); + @observable messageId: number | string = crypto.randomUUID(); @observable persisted = false; @observable senderId = -1; @observable timestamp: string = moment().toISOString(); diff --git a/resources/js/turbolinks-overrides.coffee b/resources/js/turbolinks-overrides.coffee index 929fd314b3d..258e0cc7e08 100644 --- a/resources/js/turbolinks-overrides.coffee +++ b/resources/js/turbolinks-overrides.coffee @@ -45,7 +45,7 @@ Turbolinks.Controller::advanceHistory = (url) -> location = @lastRenderedLocation @cache.put location, snapshot.clone() @lastRenderedLocation = Turbolinks.Location.wrap(url) - @pushHistoryWithLocationAndRestorationIdentifier url, Turbolinks.uuid() + @pushHistoryWithLocationAndRestorationIdentifier url, crypto.randomUUID() # @lastRenderedLocation must be updated so the most recent url will be used for @cache diff --git a/resources/js/turbolinks.d.ts b/resources/js/turbolinks.d.ts index 0b754d0d2dd..5dbc1b86052 100644 --- a/resources/js/turbolinks.d.ts +++ b/resources/js/turbolinks.d.ts @@ -31,7 +31,6 @@ declare module 'turbolinks' { controller: Controller; setProgressBarDelay(delayInMilliseconds: number): void; supported: boolean; - uuid(): string; visit(location: string, options?: TurbolinksAction): void; } } diff --git a/resources/js/utils/seq.ts b/resources/js/utils/seq.ts index 53ee52b5f00..62c32c1448a 100644 --- a/resources/js/utils/seq.ts +++ b/resources/js/utils/seq.ts @@ -6,7 +6,3 @@ let currVal = 0; export function nextVal() { return ++currVal; } - -export function uuid() { - return Turbolinks.uuid(); // no point rolling our own -} From 877c73c1b78f05df21f76851e04e5bf945fc5c10 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 24 Oct 2024 20:07:44 +0900 Subject: [PATCH 08/27] Fix show more button alignment on comments Block layout now has justify-items/self. --- resources/css/bem/show-more-link.less | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/css/bem/show-more-link.less b/resources/css/bem/show-more-link.less index e11add7b3c4..56b4be15574 100644 --- a/resources/css/bem/show-more-link.less +++ b/resources/css/bem/show-more-link.less @@ -47,6 +47,7 @@ padding-left: 10px; padding-right: $padding-left; font-weight: normal; + justify-self: start; } &--comments { From f558d40aefb427bbbb2dc5b6ae57309fc2ba662c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 09:45:16 +0200 Subject: [PATCH 09/27] Add test coverage for existing playlist item conversion check Just for insurance purposes, before I factor it out. --- tests/Models/Multiplayer/PlaylistItemTest.php | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 tests/Models/Multiplayer/PlaylistItemTest.php diff --git a/tests/Models/Multiplayer/PlaylistItemTest.php b/tests/Models/Multiplayer/PlaylistItemTest.php new file mode 100644 index 00000000000..ba483c78ca4 --- /dev/null +++ b/tests/Models/Multiplayer/PlaylistItemTest.php @@ -0,0 +1,90 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +namespace Tests\Models\Multiplayer; + +use App\Exceptions\InvariantException; +use App\Models\Beatmap; +use App\Models\Multiplayer\PlaylistItem; +use App\Models\Multiplayer\Room; +use App\Models\User; +use Tests\TestCase; + +class PlaylistItemTest extends TestCase +{ + /** + * @dataProvider modesProvider + */ + public function testOsuBeatmapPlayableInAnyRuleset(int $rulesetId) + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 0, + ]); + $room = Room::factory()->create(); + $user = User::factory()->create(); + $playlistItem = new PlaylistItem([ + 'beatmap_id' => $beatmap->getKey(), + 'ruleset_id' => $rulesetId, + 'room_id' => $room->getKey(), + 'owner_id' => $user->getKey(), + 'required_mods' => [], + 'allowed_mods' => [], + ]); + + $this->expectNotToPerformAssertions(); + $playlistItem->save(); + } + + public function testCatchBeatmapPlayableInCatchRuleset() + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 2, + ]); + $room = Room::factory()->create(); + $user = User::factory()->create(); + $playlistItem = new PlaylistItem([ + 'beatmap_id' => $beatmap->getKey(), + 'ruleset_id' => 2, + 'room_id' => $room->getKey(), + 'owner_id' => $user->getKey(), + 'required_mods' => [], + 'allowed_mods' => [], + ]); + + $this->expectNotToPerformAssertions(); + $playlistItem->save(); + } + + public function testManiaBeatmapNotPlayableInOtherRulesets() + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 3, + ]); + $room = Room::factory()->create(); + $user = User::factory()->create(); + $playlistItem = new PlaylistItem([ + 'beatmap_id' => $beatmap->getKey(), + 'ruleset_id' => 2, + 'room_id' => $room->getKey(), + 'owner_id' => $user->getKey(), + 'required_mods' => [], + 'allowed_mods' => [], + ]); + + $this->expectException(InvariantException::class); + $this->expectExceptionMessageMatches('/^invalid ruleset_id for beatmap \d+$/'); + $playlistItem->save(); + } + + public static function modesProvider() + { + return [ + 'osu' => [0], + 'taiko' => [1], + 'fruits' => [2], + 'mania' => [3], + ]; + } +} From 4807fbd8fbd46ac672c5be0c4676588c464ff37b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 09:48:37 +0200 Subject: [PATCH 10/27] Split helper for checking valid conversions --- app/Http/Controllers/BeatmapsController.php | 2 +- app/Models/Beatmap.php | 4 ++-- app/Models/Multiplayer/PlaylistItem.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/BeatmapsController.php b/app/Http/Controllers/BeatmapsController.php index eedc00b3dae..33b76791953 100644 --- a/app/Http/Controllers/BeatmapsController.php +++ b/app/Http/Controllers/BeatmapsController.php @@ -160,7 +160,7 @@ public function attributes($id) $rulesetId = $beatmap->playmode; } else { abort_if( - $rulesetId !== $beatmap->playmode && !$beatmap->canBeConverted(), + !$beatmap->canBeConvertedTo($rulesetId), 422, "specified beatmap can't be converted to the specified ruleset" ); diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index 3657909f5dc..2f308d03648 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -217,9 +217,9 @@ public function isScoreable() return $this->approved > 0; } - public function canBeConverted() + public function canBeConvertedTo(int $rulesetId) { - return $this->playmode === static::MODES['osu']; + return $this->playmode === static::MODES['osu'] || $this->playmode === $rulesetId; } public function getAttribute($key) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 92ed3817d8f..00846b34988 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -158,7 +158,7 @@ private function assertValidMaxAttempts() private function validateRuleset() { // osu beatmaps can be played in any mode, but non-osu maps can only be played in their specific modes - if ($this->beatmap->playmode !== Ruleset::osu->value && $this->beatmap->playmode !== $this->ruleset_id) { + if (!$this->beatmap->canBeConvertedTo($this->ruleset_id)) { throw new InvariantException("invalid ruleset_id for beatmap {$this->beatmap->beatmap_id}"); } } From 83b3637409092413ab1d44c74e677c5ecfaa7170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 09:51:15 +0200 Subject: [PATCH 11/27] Add test coverage for attempting to create room with playlist item in incorrect ruleset Already works as it should, but adding as insurance. --- tests/Models/Multiplayer/RoomTest.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Models/Multiplayer/RoomTest.php b/tests/Models/Multiplayer/RoomTest.php index b1ad6c2708f..3a1f8307417 100644 --- a/tests/Models/Multiplayer/RoomTest.php +++ b/tests/Models/Multiplayer/RoomTest.php @@ -92,6 +92,29 @@ public function testStartGameWithDeletedBeatmap() (new Room())->startGame($user, $params); } + public function testStartGameWithInvalidRuleset() + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 2, + ]); + $user = User::factory()->create(); + + $params = [ + 'duration' => 60, + 'name' => 'test', + 'playlist' => [ + [ + 'beatmap_id' => $beatmap->getKey(), + 'ruleset_id' => 0, + ], + ], + ]; + + $this->expectException(InvariantException::class); + $this->expectExceptionMessageMatches('/^invalid ruleset_id for beatmap \d+$/'); + (new Room())->startGame($user, $params); + } + public function testRoomHasEnded() { $user = User::factory()->create(); From f1d4897d05415d0c4759952a82a7bb4a4abfc8f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 10:05:11 +0200 Subject: [PATCH 12/27] Add failing test for multiplayer score token request not checking conversion eligibility --- .../Rooms/Playlist/ScoresControllerTest.php | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index b4eee44ba4c..4fb986b8843 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -5,12 +5,14 @@ namespace Tests\Controllers\Multiplayer\Rooms\Playlist; +use App\Models\Beatmap; use App\Models\Build; use App\Models\Multiplayer\PlaylistItem; use App\Models\Multiplayer\ScoreLink; use App\Models\Multiplayer\UserScoreAggregate; use App\Models\ScoreToken; use App\Models\User; +use Illuminate\Support\Facades\DB; use Tests\TestCase; class ScoresControllerTest extends TestCase @@ -129,6 +131,46 @@ public function testStore($allowRanking, $hashParam, $status) config_set('osu.client.check_version', $origClientCheckVersion); } + /** + * This scenario is theoretically impossible to occur via osu-web, + * but realtime multiplayer rooms have their playlist items modified directly in the database + * by osu-server-spectator. + * This test case is insurance that invalid updates performed by it do not cause further breakage. + */ + public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() + { + $origClientCheckVersion = $GLOBALS['cfg']['osu']['client']['check_version']; + config_set('osu.client.check_version', true); + $user = User::factory()->create(); + $beatmap = Beatmap::factory()->create([ + 'playmode' => 2, + ]); + $playlistItem = PlaylistItem::factory()->create([ + 'beatmap_id' => $beatmap->getKey(), + ]); + + // this type of modification is not achievable via the `PlaylistItem` class, because `save()` will rightly prevent it. + DB::statement('UPDATE `multiplayer_playlist_items` SET `ruleset_id` = ? WHERE `id` = ?', [3, $playlistItem->getKey()]); + + $build = Build::factory()->create(['allow_ranking' => true]); + + $this->actAsScopedUser($user, ['*']); + + $this->withHeaders([ + 'x-token' => static::createClientToken($build), + ]); + + $this->expectCountChange(fn () => ScoreToken::count(), 0); + + $this->json('POST', route('api.rooms.playlist.scores.store', [ + 'beatmap_hash' => $playlistItem->beatmap->checksum, + 'playlist' => $playlistItem->getKey(), + 'room' => $playlistItem->room_id, + ]))->assertStatus(422); + + config_set('osu.client.check_version', $origClientCheckVersion); + } + /** * @dataProvider dataProviderForTestUpdate */ From 666b344e9f184a8a317474ae8f6d1a20c77c9c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 10:10:45 +0200 Subject: [PATCH 13/27] Prevent starting play in multiplayer if playlist item is in incorrect state First layer of protection against https://github.com/ppy/osu/issues/30415. --- app/Models/Multiplayer/PlaylistItem.php | 6 +++--- app/Models/Multiplayer/Room.php | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 00846b34988..69793fcda0e 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -111,7 +111,7 @@ public function scoreTokens(): HasMany public function save(array $options = []) { $this->assertValidMaxAttempts(); - $this->validateRuleset(); + $this->assertValidRuleset(); $this->assertValidMods(); return parent::save($options); @@ -155,7 +155,7 @@ private function assertValidMaxAttempts() } } - private function validateRuleset() + public function assertValidRuleset() { // osu beatmaps can be played in any mode, but non-osu maps can only be played in their specific modes if (!$this->beatmap->canBeConvertedTo($this->ruleset_id)) { @@ -163,7 +163,7 @@ private function validateRuleset() } } - private function assertValidMods() + public function assertValidMods() { $allowedModIds = array_column($this->allowed_mods, 'acronym'); $requiredModIds = array_column($this->required_mods, 'acronym'); diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index acb548420ce..6eef867e54e 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -734,5 +734,8 @@ private function assertValidStartPlay(User $user, PlaylistItem $playlistItem) if ($playlistItem->played_at !== null) { throw new InvariantException('Cannot play a playlist item that has already been played.'); } + + $playlistItem->assertValidRuleset(); + $playlistItem->assertValidMods(); } } From 08813cd81e57f7267d3841a53698c4eed129d0db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 10:55:22 +0200 Subject: [PATCH 14/27] Add failing test for solo score token request not checking conversion eligibility --- .../Controllers/ScoreTokensControllerTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Controllers/ScoreTokensControllerTest.php b/tests/Controllers/ScoreTokensControllerTest.php index f52a960e938..382bf0c507d 100644 --- a/tests/Controllers/ScoreTokensControllerTest.php +++ b/tests/Controllers/ScoreTokensControllerTest.php @@ -98,6 +98,32 @@ public function testStoreInvalidParameter(string $paramKey, ?string $paramValue, config_set('osu.client.check_version', $origClientCheckVersion); } + public function testStoreInvalidRulesetConversion(): void + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 2, + ]); + + $routeParams = [ + 'beatmap' => $beatmap->getKey(), + 'ruleset_id' => 1, + ]; + $bodyParams = ['beatmap_hash' => $beatmap->checksum]; + $this->withHeaders(['x-token' => static::createClientToken($this->build)]); + + $this->expectCountChange(fn () => ScoreToken::count(), 0); + + $this->actAsScopedUser($this->user, ['*']); + $this->json( + 'POST', + route('api.beatmaps.solo.score-tokens.store', $routeParams), + $bodyParams + )->assertStatus(422) + ->assertJson([ + 'error' => 'invalid ruleset_id', + ]); + } + public static function dataProviderForTestStore(): array { return [ From 0a5479c7a8173df2342744e14a80d22d83835dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 10:57:43 +0200 Subject: [PATCH 15/27] Prevent solo score token issuance if provided beatmap cannot be converted to provided ruleset --- app/Http/Controllers/ScoreTokensController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Controllers/ScoreTokensController.php b/app/Http/Controllers/ScoreTokensController.php index 44ca3eae66c..06a392e7358 100644 --- a/app/Http/Controllers/ScoreTokensController.php +++ b/app/Http/Controllers/ScoreTokensController.php @@ -36,7 +36,7 @@ public function store($beatmapId) $checks = [ 'beatmap_hash' => fn (string $value): bool => $value === $beatmap->checksum, - 'ruleset_id' => fn (int $value): bool => Beatmap::modeStr($value) !== null, + 'ruleset_id' => fn (int $value): bool => Beatmap::modeStr($value) !== null && $beatmap->canBeConvertedTo($value), ]; foreach ($checks as $key => $testFn) { if (!isset($params[$key])) { From a61cf7f3c83b9139b32c9d290efe53fe9369ae8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 11:29:11 +0200 Subject: [PATCH 16/27] Add positive test for valid ruleset conversion when issuing solo score tokens --- .../Controllers/ScoreTokensControllerTest.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Controllers/ScoreTokensControllerTest.php b/tests/Controllers/ScoreTokensControllerTest.php index 382bf0c507d..e75d971fee0 100644 --- a/tests/Controllers/ScoreTokensControllerTest.php +++ b/tests/Controllers/ScoreTokensControllerTest.php @@ -98,6 +98,29 @@ public function testStoreInvalidParameter(string $paramKey, ?string $paramValue, config_set('osu.client.check_version', $origClientCheckVersion); } + public function testStoreValidRulesetConversion(): void + { + $beatmap = Beatmap::factory()->create([ + 'playmode' => 0, + ]); + + $routeParams = [ + 'beatmap' => $beatmap->getKey(), + 'ruleset_id' => 1, + ]; + $bodyParams = ['beatmap_hash' => $beatmap->checksum]; + $this->withHeaders(['x-token' => static::createClientToken($this->build)]); + + $this->expectCountChange(fn () => ScoreToken::count(), 1); + + $this->actAsScopedUser($this->user, ['*']); + $this->json( + 'POST', + route('api.beatmaps.solo.score-tokens.store', $routeParams), + $bodyParams + )->assertStatus(200); + } + public function testStoreInvalidRulesetConversion(): void { $beatmap = Beatmap::factory()->create([ From 7caaafe3343016256c4701bd43c1b410c48c47bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 11:36:41 +0200 Subject: [PATCH 17/27] Remove unused import --- app/Models/Multiplayer/PlaylistItem.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 69793fcda0e..92f84487d47 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -5,7 +5,6 @@ namespace App\Models\Multiplayer; -use App\Enums\Ruleset; use App\Exceptions\InvariantException; use App\Models\Beatmap; use App\Models\Model; From e4a315738910bf345ababe601a33bdbdaa5a4961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 12:55:10 +0200 Subject: [PATCH 18/27] Use another method of simulating external update in test --- .../Multiplayer/Rooms/Playlist/ScoresControllerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index 4fb986b8843..edb3fb97d04 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -149,8 +149,8 @@ public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() 'beatmap_id' => $beatmap->getKey(), ]); - // this type of modification is not achievable via the `PlaylistItem` class, because `save()` will rightly prevent it. - DB::statement('UPDATE `multiplayer_playlist_items` SET `ruleset_id` = ? WHERE `id` = ?', [3, $playlistItem->getKey()]); + // simulate invalid external modification from osu-server-spectator + PlaylistItem::whereKey($playlistItem->getKey())->update(['ruleset_id' => 3]); $build = Build::factory()->create(['allow_ranking' => true]); From 5ea47aa091f90b9d6aaa0ce57ef227dfcc11752e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 12:56:19 +0200 Subject: [PATCH 19/27] Fix code style in playlist item test --- tests/Models/Multiplayer/PlaylistItemTest.php | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Models/Multiplayer/PlaylistItemTest.php b/tests/Models/Multiplayer/PlaylistItemTest.php index ba483c78ca4..5d5ea994abf 100644 --- a/tests/Models/Multiplayer/PlaylistItemTest.php +++ b/tests/Models/Multiplayer/PlaylistItemTest.php @@ -14,8 +14,18 @@ class PlaylistItemTest extends TestCase { + public static function rulesetsProvider() + { + return [ + 'osu' => [0], + 'taiko' => [1], + 'fruits' => [2], + 'mania' => [3], + ]; + } + /** - * @dataProvider modesProvider + * @dataProvider rulesetsProvider */ public function testOsuBeatmapPlayableInAnyRuleset(int $rulesetId) { @@ -77,14 +87,4 @@ public function testManiaBeatmapNotPlayableInOtherRulesets() $this->expectExceptionMessageMatches('/^invalid ruleset_id for beatmap \d+$/'); $playlistItem->save(); } - - public static function modesProvider() - { - return [ - 'osu' => [0], - 'taiko' => [1], - 'fruits' => [2], - 'mania' => [3], - ]; - } } From e2fec1078e74d56b603be39f43a624cb84b97922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 13:00:15 +0200 Subject: [PATCH 20/27] Extract common validity checking helper on `PlaylistItem` --- app/Models/Multiplayer/PlaylistItem.php | 15 ++++++++++----- app/Models/Multiplayer/Room.php | 6 ++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 92f84487d47..4c8e6175505 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -109,9 +109,7 @@ public function scoreTokens(): HasMany public function save(array $options = []) { - $this->assertValidMaxAttempts(); - $this->assertValidRuleset(); - $this->assertValidMods(); + $this->assertValid(); return parent::save($options); } @@ -142,6 +140,13 @@ public function scorePercentile(): array }); } + public function assertValid() + { + $this->assertValidMaxAttempts(); + $this->assertValidRuleset(); + $this->assertValidMods(); + } + private function assertValidMaxAttempts() { if ($this->max_attempts === null) { @@ -154,7 +159,7 @@ private function assertValidMaxAttempts() } } - public function assertValidRuleset() + private function assertValidRuleset() { // osu beatmaps can be played in any mode, but non-osu maps can only be played in their specific modes if (!$this->beatmap->canBeConvertedTo($this->ruleset_id)) { @@ -162,7 +167,7 @@ public function assertValidRuleset() } } - public function assertValidMods() + private function assertValidMods() { $allowedModIds = array_column($this->allowed_mods, 'acronym'); $requiredModIds = array_column($this->required_mods, 'acronym'); diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index 6eef867e54e..7a2fbe0714d 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -735,7 +735,9 @@ private function assertValidStartPlay(User $user, PlaylistItem $playlistItem) throw new InvariantException('Cannot play a playlist item that has already been played.'); } - $playlistItem->assertValidRuleset(); - $playlistItem->assertValidMods(); + // ensure the playlist item itself is in a valid state. + // this is a defensive measure to prevent further breakage if the item's state is inconsistent + // due to an external modification from osu-server-spectator. + $playlistItem->assertValid(); } } From b43b6966b060b72de599f0efef51dc3ede48c538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 13:01:02 +0200 Subject: [PATCH 21/27] Remove not needed store & restore of config variable in test --- .../Multiplayer/Rooms/Playlist/ScoresControllerTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index edb3fb97d04..42b9fd1cc2d 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -139,7 +139,6 @@ public function testStore($allowRanking, $hashParam, $status) */ public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() { - $origClientCheckVersion = $GLOBALS['cfg']['osu']['client']['check_version']; config_set('osu.client.check_version', true); $user = User::factory()->create(); $beatmap = Beatmap::factory()->create([ @@ -167,8 +166,6 @@ public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() 'playlist' => $playlistItem->getKey(), 'room' => $playlistItem->room_id, ]))->assertStatus(422); - - config_set('osu.client.check_version', $origClientCheckVersion); } /** From e7bfb614d3d0d97f7618285278a27ec0c0afd833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 13:07:23 +0200 Subject: [PATCH 22/27] Remove unused import again --- .../Multiplayer/Rooms/Playlist/ScoresControllerTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index 42b9fd1cc2d..297789642c1 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -12,7 +12,6 @@ use App\Models\Multiplayer\UserScoreAggregate; use App\Models\ScoreToken; use App\Models\User; -use Illuminate\Support\Facades\DB; use Tests\TestCase; class ScoresControllerTest extends TestCase From 30fab62ded99296e5309b57d958dfa2f2589aaed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Oct 2024 13:21:20 +0200 Subject: [PATCH 23/27] Remove client check related code from test Does not need to be exercised here anyway. --- .../Multiplayer/Rooms/Playlist/ScoresControllerTest.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index 297789642c1..2477e8d1518 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -138,7 +138,6 @@ public function testStore($allowRanking, $hashParam, $status) */ public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() { - config_set('osu.client.check_version', true); $user = User::factory()->create(); $beatmap = Beatmap::factory()->create([ 'playmode' => 2, @@ -150,14 +149,8 @@ public function testAttemptToStartPlayOnInconsistentPlaylistItemFails() // simulate invalid external modification from osu-server-spectator PlaylistItem::whereKey($playlistItem->getKey())->update(['ruleset_id' => 3]); - $build = Build::factory()->create(['allow_ranking' => true]); - $this->actAsScopedUser($user, ['*']); - $this->withHeaders([ - 'x-token' => static::createClientToken($build), - ]); - $this->expectCountChange(fn () => ScoreToken::count(), 0); $this->json('POST', route('api.rooms.playlist.scores.store', [ From e357c798fb56ce8b326554c9ae81065c2504f89c Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Sat, 26 Oct 2024 17:29:50 -0400 Subject: [PATCH 24/27] Move user creation step after database seeding --- SETUP.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/SETUP.md b/SETUP.md index 099ba65db56..ea2f4d538d0 100644 --- a/SETUP.md +++ b/SETUP.md @@ -208,15 +208,6 @@ p artisan tinker # Development -## Creating your initial user - -In the repository directory: - -```php -php artisan tinker ->>> (new App\Libraries\UserRegistration(["username" => "yourusername", "user_email" => "your@email.com", "password" => "yourpassword"]))->save(); -``` - ## Generating assets ```bash @@ -234,6 +225,17 @@ php artisan migrate:fresh --seed Run the above command to rebuild the database and populate it with sample data. In order for the seeder to seed beatmaps, you must enter a valid osu! API key as the value of the `OSU_API_KEY` property in the `.env` configuration file, as the seeder obtains beatmap data from the osu! API. The key can be obtained from [the "Legacy API" section of your account settings page](https://osu.ppy.sh/home/account/edit#legacy-api). +## Creating your initial user + +In the repository directory: + +```php +php artisan tinker +>>> (new App\Libraries\UserRegistration(["username" => "yourusername", "user_email" => "your@email.com", "password" => "yourpassword"]))->save(); +``` + +Note that seeding sample data (the step above this) is required for user registration to work, otherwise the command above will fail due to missing user groups or otherwise. + ## Continuous asset generation while developing To continuously generate assets as you make changes to files (less, coffeescript) you can run `webpack` in `watch` mode. From 05649208103cad8b92486465bf6999b82a3708c1 Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 28 Oct 2024 22:14:57 +0900 Subject: [PATCH 25/27] Fix dashboard moving around on initial load --- resources/css/bem/menu-images.less | 3 ++- resources/views/home/user.blade.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/resources/css/bem/menu-images.less b/resources/css/bem/menu-images.less index 36a9d66b30e..651d02c032c 100644 --- a/resources/css/bem/menu-images.less +++ b/resources/css/bem/menu-images.less @@ -73,7 +73,6 @@ } &__container { - .center-content(); display: flex; transform: translateX(calc(-100% * var(--index, 0))); width: 100%; @@ -85,6 +84,8 @@ } &__images { + .center-content(); + display: flex; height: 120px; overflow: hidden; position: relative; diff --git a/resources/views/home/user.blade.php b/resources/views/home/user.blade.php index 8600ceb0207..085253a5a24 100644 --- a/resources/views/home/user.blade.php +++ b/resources/views/home/user.blade.php @@ -16,7 +16,7 @@ @if (count($menuImages) > 0)
-
+
{!! spinner() !!}
@if (count($menuImages) > 1) From 7eb9fbf9d6fb6040f694d3cedc955f73680fe081 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Tue, 29 Oct 2024 01:10:21 -0400 Subject: [PATCH 26/27] Place asset generation step more appropriately --- SETUP.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/SETUP.md b/SETUP.md index ea2f4d538d0..b26016e50bd 100644 --- a/SETUP.md +++ b/SETUP.md @@ -208,15 +208,6 @@ p artisan tinker # Development -## Generating assets - -```bash -# build assets (should be done automatically if using docker) -yarn run development -``` - -Note that if you use the bundled docker compose setup, yarn/webpack will be already run in watch mode. - ## Reset the database + seeding sample data ``` @@ -236,6 +227,15 @@ php artisan tinker Note that seeding sample data (the step above this) is required for user registration to work, otherwise the command above will fail due to missing user groups or otherwise. +## Generating assets + +```bash +# build assets (should be done automatically if using docker) +yarn run development +``` + +Note that if you use the bundled docker compose setup, yarn/webpack will be already run in watch mode. + ## Continuous asset generation while developing To continuously generate assets as you make changes to files (less, coffeescript) you can run `webpack` in `watch` mode. From 7b58e9416505047075ad38370910a660de916953 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Tue, 29 Oct 2024 01:10:32 -0400 Subject: [PATCH 27/27] `Mysql` -> `MySQL` --- SETUP.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SETUP.md b/SETUP.md index b26016e50bd..bd38a599cbd 100644 --- a/SETUP.md +++ b/SETUP.md @@ -96,9 +96,9 @@ Note that older versions of Elasticsearch do not work on ARM-based CPUs. `osu-elastic-indexer` currently cannot update indices using Elasticsearch 7; existing records can still be queried normally. -### Mysql +### MySQL -The Mysql images provided by Docker and Mysql have different uids for the `mysql` user, if you are getting permission errors when starting the `db` container like +The MySQL images provided by Docker and MySQL have different uids for the `mysql` user, if you are getting permission errors when starting the `db` container like mysqld: File './binlog.index' not found (OS errno 13 - Permission denied)