From 545c945125919800fc4866d992b6d2d6fc954b29 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 10 Dec 2024 21:34:51 +0900 Subject: [PATCH 01/26] render user tags --- .../Controllers/BeatmapsetsController.php | 1 + .../BeatmapsetCompactTransformer.php | 8 ++++++++ resources/js/beatmapsets-show/info.tsx | 19 ++++++++++++------- .../js/interfaces/beatmapset-extended-json.ts | 1 + resources/js/interfaces/beatmapset-json.ts | 2 ++ resources/js/interfaces/tag-json.ts | 8 ++++++++ 6 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 resources/js/interfaces/tag-json.ts diff --git a/app/Http/Controllers/BeatmapsetsController.php b/app/Http/Controllers/BeatmapsetsController.php index d0958f9567a..d8f91d7b5ad 100644 --- a/app/Http/Controllers/BeatmapsetsController.php +++ b/app/Http/Controllers/BeatmapsetsController.php @@ -417,6 +417,7 @@ private function showJson($beatmapset) 'recent_favourites', 'related_users', 'user', + 'user_tags', ]); } } diff --git a/app/Transformers/BeatmapsetCompactTransformer.php b/app/Transformers/BeatmapsetCompactTransformer.php index 546fe4125fe..fd3f7d0694c 100644 --- a/app/Transformers/BeatmapsetCompactTransformer.php +++ b/app/Transformers/BeatmapsetCompactTransformer.php @@ -41,6 +41,7 @@ class BeatmapsetCompactTransformer extends TransformerAbstract 'recent_favourites', 'related_users', 'user', + 'user_tags', ]; // TODO: switch to enum after php 8.1 @@ -299,6 +300,13 @@ public function includeRelatedUsers(Beatmapset $beatmapset) return $this->collection($users, new UserCompactTransformer()); } + public function includeUserTags(Beatmapset $beatmapset) + { + $beatmaps = $this->beatmaps($beatmapset); + + return $this->collection($beatmaps->flatMap->topTags(), new TagTransformer()); + } + private function beatmaps(Beatmapset $beatmapset, ?Fractal\ParamBag $params = null): EloquentCollection { $rel = $beatmapset->trashed() || ($params !== null && $params->get('with_trashed')) ? 'allBeatmaps' : 'beatmaps'; diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index a90cdb9e042..e1974cbda50 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -65,6 +65,15 @@ export default class Info extends React.Component { return ret; } + private get tags() { + return [ + ...this.controller.beatmapset.user_tags.map((tag) => tag.name), + ...this.controller.beatmapset.tags + .split(' ') + .filter(present), + ]; + } + private get withEditDescription() { return this.controller.beatmapset.description.bbcode != null; } @@ -84,10 +93,6 @@ export default class Info extends React.Component { } render() { - const tags = this.controller.beatmapset.tags - .split(' ') - .filter(present); - return (
{this.isEditingDescription && @@ -191,14 +196,14 @@ export default class Info extends React.Component {
- {tags.length > 0 && + {this.tags.length > 0 &&

{trans('beatmapsets.show.info.tags')}

- {tags.map((tag, i) => ( - + {this.tags.map((tag) => ( + >; export type BeatmapsetJsonForShow = diff --git a/resources/js/interfaces/beatmapset-json.ts b/resources/js/interfaces/beatmapset-json.ts index 533ee955779..cb59fc13390 100644 --- a/resources/js/interfaces/beatmapset-json.ts +++ b/resources/js/interfaces/beatmapset-json.ts @@ -9,6 +9,7 @@ import BeatmapsetNominationJson from './beatmapset-nomination-json'; import GenreJson from './genre-json'; import LanguageJson from './language-json'; import Ruleset from './ruleset'; +import TagJson from './tag-json'; import UserJson, { UserJsonDeleted } from './user-json'; export interface Availability { @@ -94,6 +95,7 @@ interface BeatmapsetJsonAvailableIncludes { recent_favourites: UserJson[]; related_users: UserJson[]; user: UserJson | UserJsonDeleted; + user_tags: TagJson[]; } interface HypeData { diff --git a/resources/js/interfaces/tag-json.ts b/resources/js/interfaces/tag-json.ts new file mode 100644 index 00000000000..5d1352a7a4b --- /dev/null +++ b/resources/js/interfaces/tag-json.ts @@ -0,0 +1,8 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +export default interface TagJson { + description: string; + id: number; + name: string; +} From 4ae47f9f743c928a7f6412f395f7eaba35056b12 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 12 Dec 2024 15:57:19 +0900 Subject: [PATCH 02/26] cache tags locally, only cache beatmap tags with ids and counts; return description with response? --- .../Controllers/BeatmapTagsController.php | 9 +---- app/Http/Controllers/TagsController.php | 9 +---- app/Models/Beatmap.php | 26 ++++++++++++++ app/Models/Tag.php | 6 ++-- app/Providers/AppServiceProvider.php | 1 + app/Singletons/Tags.php | 35 +++++++++++++++++++ 6 files changed, 67 insertions(+), 19 deletions(-) create mode 100644 app/Singletons/Tags.php diff --git a/app/Http/Controllers/BeatmapTagsController.php b/app/Http/Controllers/BeatmapTagsController.php index 87c9d9b253b..630010f7564 100644 --- a/app/Http/Controllers/BeatmapTagsController.php +++ b/app/Http/Controllers/BeatmapTagsController.php @@ -29,15 +29,8 @@ public function __construct() public function index($beatmapId) { - $topBeatmapTags = cache_remember_mutexed( - "beatmap_tags:{$beatmapId}", - $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], - [], - fn () => Tag::topTags($beatmapId), - ); - return [ - 'beatmap_tags' => $topBeatmapTags, + 'beatmap_tags' => Beatmap::findOrFail($beatmapId)->topTagsJson(), ]; } diff --git a/app/Http/Controllers/TagsController.php b/app/Http/Controllers/TagsController.php index 80b657d9182..a93ddeff028 100644 --- a/app/Http/Controllers/TagsController.php +++ b/app/Http/Controllers/TagsController.php @@ -21,15 +21,8 @@ public function __construct() public function index() { - $tags = cache_remember_mutexed( - 'tags', - $GLOBALS['cfg']['osu']['tags']['tags_cache_duration'], - [], - fn () => Tag::all(), - ); - return [ - 'tags' => json_collection($tags, new TagTransformer()), + 'tags' => app('tags')->json(), ]; } } diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index b107d638d17..a042eebcf9c 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -348,6 +348,32 @@ public function status() return array_search($this->approved, Beatmapset::STATES, true); } + public function topTagsJson() + { + $tagIds = cache_remember_mutexed( + "beatmap_top_tag_ids:{$this->getKey()}", + $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], + [], + fn () => Tag::topTagIds($this->getKey())->toArray(), + ); + + $cachedTags = app('tags'); + $json = []; + + foreach ($tagIds as $tagId) { + $tag = $cachedTags->get($tagId['id']); + if ($tag !== null) { + $json[] = [ + ...$tagId, + 'description' => $tag->description, + 'name' => $tag->name, + ]; + } + } + + return $json; + } + private function getDifficultyrating() { if ($this->convert) { diff --git a/app/Models/Tag.php b/app/Models/Tag.php index 3513ab3d891..74293c5c00c 100644 --- a/app/Models/Tag.php +++ b/app/Models/Tag.php @@ -23,7 +23,7 @@ public function beatmapTags(): HasMany return $this->hasMany(BeatmapTag::class); } - public static function topTags($beatmapId) + public static function topTagIds(int $beatmapId, int $limit = 50) { return static ::joinRelation( @@ -31,11 +31,11 @@ public static function topTags($beatmapId) fn ($q) => $q->where('beatmap_id', $beatmapId)->whereHas('user', fn ($userQuery) => $userQuery->default()) ) ->groupBy('id') - ->select('id', 'name') + ->select('id') ->selectRaw('COUNT(*) as count') ->orderBy('count', 'desc') ->orderBy('id', 'desc') - ->limit(50) + ->limit($limit) ->get(); } } diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index bd080bdcae9..7d358070636 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -31,6 +31,7 @@ class AppServiceProvider extends ServiceProvider 'layout-cache' => Singletons\LayoutCache::class, 'medals' => Singletons\Medals::class, 'smilies' => Singletons\Smilies::class, + 'tags' => Singletons\Tags::class, 'user-cover-presets' => Singletons\UserCoverPresets::class, ]; diff --git a/app/Singletons/Tags.php b/app/Singletons/Tags.php new file mode 100644 index 00000000000..b8cccbd6e46 --- /dev/null +++ b/app/Singletons/Tags.php @@ -0,0 +1,35 @@ +. 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\Singletons; + +use App\Models\Tag; +use App\Traits\Memoizes; +use App\Transformers\TagTransformer; + +class Tags +{ + use Memoizes; + + public function get(int $id): ?Tag + { + $allById = $this->memoize( + 'allById', + fn () => Tag::all()->keyBy('id'), + ); + + return $allById[$id] ?? null; + } + + public function json(): array + { + return $this->memoize( + __FUNCTION__, + fn () => json_collection(Tag::all(), new TagTransformer()), + ); + } +} From 4a55d98edebb9e5041115a33e339c986d6d73076 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Fri, 13 Dec 2024 15:38:29 +0900 Subject: [PATCH 03/26] return tags with counts with beatmaps --- app/Http/Controllers/BeatmapsetsController.php | 2 +- app/Transformers/BeatmapCompactTransformer.php | 6 ++++++ resources/js/beatmapsets-show/controller.ts | 18 ++++++++++++++++++ resources/js/beatmapsets-show/info.tsx | 9 +++++++-- resources/js/interfaces/beatmap-json.ts | 2 ++ .../js/interfaces/beatmapset-extended-json.ts | 1 - resources/js/interfaces/tag-json.ts | 1 + 7 files changed, 35 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/BeatmapsetsController.php b/app/Http/Controllers/BeatmapsetsController.php index d8f91d7b5ad..692f089fda3 100644 --- a/app/Http/Controllers/BeatmapsetsController.php +++ b/app/Http/Controllers/BeatmapsetsController.php @@ -404,6 +404,7 @@ private function showJson($beatmapset) 'beatmaps.failtimes', 'beatmaps.max_combo', 'beatmaps.owners', + 'beatmaps.tags', 'converts', 'converts.failtimes', 'converts.owners', @@ -417,7 +418,6 @@ private function showJson($beatmapset) 'recent_favourites', 'related_users', 'user', - 'user_tags', ]); } } diff --git a/app/Transformers/BeatmapCompactTransformer.php b/app/Transformers/BeatmapCompactTransformer.php index ef8a96c6542..32cad40ce36 100644 --- a/app/Transformers/BeatmapCompactTransformer.php +++ b/app/Transformers/BeatmapCompactTransformer.php @@ -18,6 +18,7 @@ class BeatmapCompactTransformer extends TransformerAbstract 'failtimes', 'max_combo', 'owners', + 'tags', 'user', ]; @@ -83,6 +84,11 @@ public function includeOwners(Beatmap $beatmap) ]); } + public function includeTags(Beatmap $beatmap) + { + return $this->primitive($beatmap->topTagsJson()); + } + public function includeUser(Beatmap $beatmap) { return $this->item( diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 7447723e6cf..54b2fb2f0ea 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +import BeatmapJson from 'interfaces/beatmap-json'; import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-json'; import UserJson from 'interfaces/user-json'; import { keyBy } from 'lodash'; @@ -70,6 +71,23 @@ export default class Controller { return this.beatmaps.get(this.currentBeatmap.mode) ?? []; } + @computed + get tags() { + const summedTags: Partial> = {}; + for (const beatmap of this.beatmapset.beatmaps) { + if (beatmap.tags == null) continue; + for (const tag of beatmap.tags) { + if (summedTags[tag.id] != null) { + summedTags[tag.id].count += tag.count; + } else { + summedTags[tag.id] = tag; + } + } + } + + return summedTags; + } + @computed get usersById() { return keyBy(this.beatmapset.related_users, 'id') as Partial>; diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index e1974cbda50..97c5b1dafea 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -66,9 +66,14 @@ export default class Info extends React.Component { } private get tags() { + const sortedTags = Object.values(this.controller.tags).sort((a, b) => { + const diff = b.count - a.count; + return diff !== 0 ? diff : a.id - b.id; + }); + return [ - ...this.controller.beatmapset.user_tags.map((tag) => tag.name), - ...this.controller.beatmapset.tags + ...sortedTags.map((tag) => tag.name), + ...this.controller.beatmapset.tags // TODO: something about duplicate mapper tags .split(' ') .filter(present), ]; diff --git a/resources/js/interfaces/beatmap-json.ts b/resources/js/interfaces/beatmap-json.ts index a7d40211981..ee6db218529 100644 --- a/resources/js/interfaces/beatmap-json.ts +++ b/resources/js/interfaces/beatmap-json.ts @@ -4,6 +4,7 @@ import BeatmapOwnerJson from './beatmap-owner-json'; import BeatmapsetJson from './beatmapset-json'; import Ruleset from './ruleset'; +import TagJson from './tag-json'; import UserJson from './user-json'; interface BeatmapFailTimesArray { @@ -17,6 +18,7 @@ interface BeatmapJsonAvailableIncludes { failtimes: BeatmapFailTimesArray; max_combo: number; owners: BeatmapOwnerJson[]; + tags: (TagJson & Required>)[]; user: UserJson; } diff --git a/resources/js/interfaces/beatmapset-extended-json.ts b/resources/js/interfaces/beatmapset-extended-json.ts index 20f2f19a9e6..1a41e2c9d3c 100644 --- a/resources/js/interfaces/beatmapset-extended-json.ts +++ b/resources/js/interfaces/beatmapset-extended-json.ts @@ -57,7 +57,6 @@ type BeatmapsetJsonForShowIncludes = Required>; export type BeatmapsetJsonForShow = diff --git a/resources/js/interfaces/tag-json.ts b/resources/js/interfaces/tag-json.ts index 5d1352a7a4b..557b39d4b8b 100644 --- a/resources/js/interfaces/tag-json.ts +++ b/resources/js/interfaces/tag-json.ts @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. export default interface TagJson { + count?: number; description: string; id: number; name: string; From b3fcdfa9b1b4f2fc3c8761381aa99077ccf4e5d3 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Fri, 13 Dec 2024 16:56:04 +0900 Subject: [PATCH 04/26] add TagJsonWithCount type --- resources/js/beatmapsets-show/controller.ts | 14 ++++++++------ resources/js/interfaces/beatmap-json.ts | 4 ++-- resources/js/interfaces/tag-json.ts | 2 ++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 54b2fb2f0ea..b3d1ab36681 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -1,8 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. -import BeatmapJson from 'interfaces/beatmap-json'; import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-json'; +import { TagJsonWithCount } from 'interfaces/tag-json'; import UserJson from 'interfaces/user-json'; import { keyBy } from 'lodash'; import { action, computed, makeObservable, observable, runInAction } from 'mobx'; @@ -73,15 +73,17 @@ export default class Controller { @computed get tags() { - const summedTags: Partial> = {}; + const summedTags: Partial> = {}; for (const beatmap of this.beatmapset.beatmaps) { if (beatmap.tags == null) continue; + for (const tag of beatmap.tags) { - if (summedTags[tag.id] != null) { - summedTags[tag.id].count += tag.count; - } else { - summedTags[tag.id] = tag; + const summedTag = summedTags[tag.id]; + if (summedTag != null) { + summedTag.count += tag.count; } + + summedTags[tag.id] = summedTag; } } diff --git a/resources/js/interfaces/beatmap-json.ts b/resources/js/interfaces/beatmap-json.ts index ee6db218529..8400706c956 100644 --- a/resources/js/interfaces/beatmap-json.ts +++ b/resources/js/interfaces/beatmap-json.ts @@ -4,7 +4,7 @@ import BeatmapOwnerJson from './beatmap-owner-json'; import BeatmapsetJson from './beatmapset-json'; import Ruleset from './ruleset'; -import TagJson from './tag-json'; +import { TagJsonWithCount } from './tag-json'; import UserJson from './user-json'; interface BeatmapFailTimesArray { @@ -18,7 +18,7 @@ interface BeatmapJsonAvailableIncludes { failtimes: BeatmapFailTimesArray; max_combo: number; owners: BeatmapOwnerJson[]; - tags: (TagJson & Required>)[]; + tags: TagJsonWithCount[]; user: UserJson; } diff --git a/resources/js/interfaces/tag-json.ts b/resources/js/interfaces/tag-json.ts index 557b39d4b8b..da857843edc 100644 --- a/resources/js/interfaces/tag-json.ts +++ b/resources/js/interfaces/tag-json.ts @@ -7,3 +7,5 @@ export default interface TagJson { id: number; name: string; } + +export type TagJsonWithCount = TagJson & Required>; From d88307554c57a4b441deea93fa2c87e65ad8ec62 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Fri, 13 Dec 2024 18:55:35 +0900 Subject: [PATCH 05/26] merge user and mapper tags, place mapper tags at the end --- resources/js/beatmapsets-show/controller.ts | 25 ++++++++++++++++----- resources/js/beatmapsets-show/info.tsx | 11 ++++----- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index b3d1ab36681..542d6c251b8 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -5,12 +5,13 @@ import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-json'; import { TagJsonWithCount } from 'interfaces/tag-json'; import UserJson from 'interfaces/user-json'; import { keyBy } from 'lodash'; -import { action, computed, makeObservable, observable, runInAction } from 'mobx'; +import { action, computed, makeObservable, observable, runInAction, toJS } from 'mobx'; import { deletedUserJson } from 'models/user'; import core from 'osu-core-singleton'; import { find, findDefault, group } from 'utils/beatmap-helper'; import { parse } from 'utils/beatmapset-page-hash'; import { parseJson } from 'utils/json'; +import { present } from 'utils/string'; import { currentUrl } from 'utils/turbolinks'; export type ScoreLoadingState = null | 'error' | 'loading' | 'supporter_only' | 'unranked'; @@ -73,21 +74,33 @@ export default class Controller { @computed get tags() { - const summedTags: Partial> = {}; + const mapperTagSet = new Set(this.beatmapset.tags.split(' ').filter(present)); + + const userTags: Partial> = {}; for (const beatmap of this.beatmapset.beatmaps) { if (beatmap.tags == null) continue; for (const tag of beatmap.tags) { - const summedTag = summedTags[tag.id]; - if (summedTag != null) { + let summedTag = userTags[tag.id]; + if (summedTag == null) { + summedTag = toJS(tag); // don't modify original + userTags[tag.id] = summedTag; + } else { summedTag.count += tag.count; } - summedTags[tag.id] = summedTag; + // TODO: case insensitivity + if (mapperTagSet.has(tag.name)) { + summedTag.count++; + mapperTagSet.delete(tag.name); + } } } - return summedTags; + return { + mapperTags: [...mapperTagSet.values()], + userTags, + }; } @computed diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index 97c5b1dafea..a9c539039ed 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -66,16 +66,17 @@ export default class Info extends React.Component { } private get tags() { - const sortedTags = Object.values(this.controller.tags).sort((a, b) => { + const tags = this.controller.tags; + + const sortedUserTags = Object.values(tags.userTags).sort((a, b) => { + if (a == null || b == null) return 0; // for typing only, doesn't contain nulls. const diff = b.count - a.count; return diff !== 0 ? diff : a.id - b.id; }); return [ - ...sortedTags.map((tag) => tag.name), - ...this.controller.beatmapset.tags // TODO: something about duplicate mapper tags - .split(' ') - .filter(present), + ...sortedUserTags.map((tag) => tag?.name), + ...tags.mapperTags, ]; } From 3c9a6f5fbee35823f67ee1c7e4f4488d4952a89b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Fri, 13 Dec 2024 20:49:04 +0900 Subject: [PATCH 06/26] do the sorting in controller --- resources/js/beatmapsets-show/controller.ts | 12 ++++++++---- resources/js/beatmapsets-show/info.tsx | 8 +------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 542d6c251b8..05e784e6349 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -76,15 +76,15 @@ export default class Controller { get tags() { const mapperTagSet = new Set(this.beatmapset.tags.split(' ').filter(present)); - const userTags: Partial> = {}; + const tags: Partial> = {}; for (const beatmap of this.beatmapset.beatmaps) { if (beatmap.tags == null) continue; for (const tag of beatmap.tags) { - let summedTag = userTags[tag.id]; + let summedTag = tags[tag.id]; if (summedTag == null) { summedTag = toJS(tag); // don't modify original - userTags[tag.id] = summedTag; + tags[tag.id] = summedTag; } else { summedTag.count += tag.count; } @@ -99,7 +99,11 @@ export default class Controller { return { mapperTags: [...mapperTagSet.values()], - userTags, + userTags: Object.values(tags).sort((a, b) => { + if (a == null || b == null) return 0; // for typing only, doesn't contain nulls. + const diff = b.count - a.count; + return diff !== 0 ? diff : a.id - b.id; + }), }; } diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index a9c539039ed..56cbe3641fc 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -68,14 +68,8 @@ export default class Info extends React.Component { private get tags() { const tags = this.controller.tags; - const sortedUserTags = Object.values(tags.userTags).sort((a, b) => { - if (a == null || b == null) return 0; // for typing only, doesn't contain nulls. - const diff = b.count - a.count; - return diff !== 0 ? diff : a.id - b.id; - }); - return [ - ...sortedUserTags.map((tag) => tag?.name), + ...tags.userTags.map((tag) => tag?.name), ...tags.mapperTags, ]; } From 1503ceb9fe3b1080d56b23adc005a63aa93a9e22 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 17 Dec 2024 19:16:39 +0900 Subject: [PATCH 07/26] lint --- app/Http/Controllers/TagsController.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/Http/Controllers/TagsController.php b/app/Http/Controllers/TagsController.php index a93ddeff028..b0ce5c493a9 100644 --- a/app/Http/Controllers/TagsController.php +++ b/app/Http/Controllers/TagsController.php @@ -7,9 +7,6 @@ namespace App\Http\Controllers; -use App\Models\Tag; -use App\Transformers\TagTransformer; - class TagsController extends Controller { public function __construct() From f5552af474b41005cec5c8e60ab54dd0772d9de6 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 17 Dec 2024 20:16:20 +0900 Subject: [PATCH 08/26] return id and counts seperately from full tag --- .../Controllers/BeatmapsetsController.php | 3 +- app/Models/Beatmap.php | 20 +-------- .../BeatmapCompactTransformer.php | 6 +-- .../BeatmapsetCompactTransformer.php | 17 ++++++-- resources/js/beatmapsets-show/controller.ts | 41 ++++++++++++------- resources/js/interfaces/beatmap-json.ts | 3 +- .../js/interfaces/beatmapset-extended-json.ts | 1 + resources/js/interfaces/beatmapset-json.ts | 1 + 8 files changed, 50 insertions(+), 42 deletions(-) diff --git a/app/Http/Controllers/BeatmapsetsController.php b/app/Http/Controllers/BeatmapsetsController.php index 692f089fda3..a2da6fc6b37 100644 --- a/app/Http/Controllers/BeatmapsetsController.php +++ b/app/Http/Controllers/BeatmapsetsController.php @@ -404,7 +404,7 @@ private function showJson($beatmapset) 'beatmaps.failtimes', 'beatmaps.max_combo', 'beatmaps.owners', - 'beatmaps.tags', + 'beatmaps.top_tag_ids', 'converts', 'converts.failtimes', 'converts.owners', @@ -416,6 +416,7 @@ private function showJson($beatmapset) 'pack_tags', 'ratings', 'recent_favourites', + 'related_tags', 'related_users', 'user', ]); diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index a042eebcf9c..57f18ae0aaa 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -348,30 +348,14 @@ public function status() return array_search($this->approved, Beatmapset::STATES, true); } - public function topTagsJson() + public function topTagIds() { - $tagIds = cache_remember_mutexed( + return cache_remember_mutexed( "beatmap_top_tag_ids:{$this->getKey()}", $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], [], fn () => Tag::topTagIds($this->getKey())->toArray(), ); - - $cachedTags = app('tags'); - $json = []; - - foreach ($tagIds as $tagId) { - $tag = $cachedTags->get($tagId['id']); - if ($tag !== null) { - $json[] = [ - ...$tagId, - 'description' => $tag->description, - 'name' => $tag->name, - ]; - } - } - - return $json; } private function getDifficultyrating() diff --git a/app/Transformers/BeatmapCompactTransformer.php b/app/Transformers/BeatmapCompactTransformer.php index 32cad40ce36..9ec4aa198c3 100644 --- a/app/Transformers/BeatmapCompactTransformer.php +++ b/app/Transformers/BeatmapCompactTransformer.php @@ -18,7 +18,7 @@ class BeatmapCompactTransformer extends TransformerAbstract 'failtimes', 'max_combo', 'owners', - 'tags', + 'top_tag_ids', 'user', ]; @@ -84,9 +84,9 @@ public function includeOwners(Beatmap $beatmap) ]); } - public function includeTags(Beatmap $beatmap) + public function includeTopTagIds(Beatmap $beatmap) { - return $this->primitive($beatmap->topTagsJson()); + return $this->primitive($beatmap->topTagIds()); } public function includeUser(Beatmap $beatmap) diff --git a/app/Transformers/BeatmapsetCompactTransformer.php b/app/Transformers/BeatmapsetCompactTransformer.php index fd3f7d0694c..59f47b44d71 100644 --- a/app/Transformers/BeatmapsetCompactTransformer.php +++ b/app/Transformers/BeatmapsetCompactTransformer.php @@ -40,8 +40,8 @@ class BeatmapsetCompactTransformer extends TransformerAbstract 'ratings', 'recent_favourites', 'related_users', + 'related_tags', 'user', - 'user_tags', ]; // TODO: switch to enum after php 8.1 @@ -300,11 +300,22 @@ public function includeRelatedUsers(Beatmapset $beatmapset) return $this->collection($users, new UserCompactTransformer()); } - public function includeUserTags(Beatmapset $beatmapset) + public function includeRelatedTags(Beatmapset $beatmapset) { $beatmaps = $this->beatmaps($beatmapset); + $tagIdSet = new Set($beatmaps->flatMap->topTagIds()->pluck('id')); - return $this->collection($beatmaps->flatMap->topTags(), new TagTransformer()); + $cachedTags = app('tags'); + $json = []; + + foreach ($tagIdSet as $tagId) { + $tag = $cachedTags->get($tagId); + if ($tag !== null) { + $json[] = $tag; + } + } + + return $this->primitive($json); } private function beatmaps(Beatmapset $beatmapset, ?Fractal\ParamBag $params = null): EloquentCollection diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 05e784e6349..e93119133f3 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -5,7 +5,7 @@ import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-json'; import { TagJsonWithCount } from 'interfaces/tag-json'; import UserJson from 'interfaces/user-json'; import { keyBy } from 'lodash'; -import { action, computed, makeObservable, observable, runInAction, toJS } from 'mobx'; +import { action, computed, makeObservable, observable, runInAction } from 'mobx'; import { deletedUserJson } from 'models/user'; import core from 'osu-core-singleton'; import { find, findDefault, group } from 'utils/beatmap-helper'; @@ -72,26 +72,38 @@ export default class Controller { return this.beatmaps.get(this.currentBeatmap.mode) ?? []; } + get relatedTags() { + const map = new Map(); + + for (const tag of this.beatmapset.related_tags) { + map.set(tag.id, tag); + } + + return map; + } + @computed get tags() { const mapperTagSet = new Set(this.beatmapset.tags.split(' ').filter(present)); + const tagMap = new Map(); + + for (const tag of this.beatmapset.related_tags) { + tag.count = 0; // assign 0 and cast + tagMap.set(tag.id, tag as TagJsonWithCount); + } - const tags: Partial> = {}; for (const beatmap of this.beatmapset.beatmaps) { - if (beatmap.tags == null) continue; - - for (const tag of beatmap.tags) { - let summedTag = tags[tag.id]; - if (summedTag == null) { - summedTag = toJS(tag); // don't modify original - tags[tag.id] = summedTag; - } else { - summedTag.count += tag.count; - } + if (beatmap.top_tag_ids == null) continue; + + for (const tagId of beatmap.top_tag_ids) { + const tag = tagMap.get(tagId.id); + if (tag == null) continue; + + tag.count += tagId.count; // TODO: case insensitivity if (mapperTagSet.has(tag.name)) { - summedTag.count++; + tag.count++; mapperTagSet.delete(tag.name); } } @@ -99,8 +111,7 @@ export default class Controller { return { mapperTags: [...mapperTagSet.values()], - userTags: Object.values(tags).sort((a, b) => { - if (a == null || b == null) return 0; // for typing only, doesn't contain nulls. + userTags: [...tagMap.values()].sort((a, b) => { const diff = b.count - a.count; return diff !== 0 ? diff : a.id - b.id; }), diff --git a/resources/js/interfaces/beatmap-json.ts b/resources/js/interfaces/beatmap-json.ts index 8400706c956..9c5837a51ea 100644 --- a/resources/js/interfaces/beatmap-json.ts +++ b/resources/js/interfaces/beatmap-json.ts @@ -4,7 +4,6 @@ import BeatmapOwnerJson from './beatmap-owner-json'; import BeatmapsetJson from './beatmapset-json'; import Ruleset from './ruleset'; -import { TagJsonWithCount } from './tag-json'; import UserJson from './user-json'; interface BeatmapFailTimesArray { @@ -18,7 +17,7 @@ interface BeatmapJsonAvailableIncludes { failtimes: BeatmapFailTimesArray; max_combo: number; owners: BeatmapOwnerJson[]; - tags: TagJsonWithCount[]; + top_tag_ids: { count: number; id: number }[]; user: UserJson; } diff --git a/resources/js/interfaces/beatmapset-extended-json.ts b/resources/js/interfaces/beatmapset-extended-json.ts index 1a41e2c9d3c..c7ff2f4038d 100644 --- a/resources/js/interfaces/beatmapset-extended-json.ts +++ b/resources/js/interfaces/beatmapset-extended-json.ts @@ -55,6 +55,7 @@ type BeatmapsetJsonForShowIncludes = Required>; diff --git a/resources/js/interfaces/beatmapset-json.ts b/resources/js/interfaces/beatmapset-json.ts index cb59fc13390..6519a014d1d 100644 --- a/resources/js/interfaces/beatmapset-json.ts +++ b/resources/js/interfaces/beatmapset-json.ts @@ -93,6 +93,7 @@ interface BeatmapsetJsonAvailableIncludes { nominations: BeatmapsetNominationsInterface; ratings: number[]; recent_favourites: UserJson[]; + related_tags: TagJson[]; related_users: UserJson[]; user: UserJson | UserJsonDeleted; user_tags: TagJson[]; From 5ad134a1904fa3c99e615d26df84a6f23c594d6b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 17 Dec 2024 22:11:43 +0900 Subject: [PATCH 09/26] remove route (get from beatmapset instead) --- app/Http/Controllers/BeatmapTagsController.php | 9 --------- routes/web.php | 2 +- tests/Controllers/BeatmapTagsControllerTest.php | 16 ---------------- tests/api_routes.json | 16 ---------------- 4 files changed, 1 insertion(+), 42 deletions(-) diff --git a/app/Http/Controllers/BeatmapTagsController.php b/app/Http/Controllers/BeatmapTagsController.php index 630010f7564..f6cf56b035a 100644 --- a/app/Http/Controllers/BeatmapTagsController.php +++ b/app/Http/Controllers/BeatmapTagsController.php @@ -23,15 +23,6 @@ public function __construct() 'destroy', ], ]); - - $this->middleware('require-scopes:public', ['only' => 'index']); - } - - public function index($beatmapId) - { - return [ - 'beatmap_tags' => Beatmap::findOrFail($beatmapId)->topTagsJson(), - ]; } public function destroy($beatmapId, $tagId) diff --git a/routes/web.php b/routes/web.php index a94e8853fa3..9b3bfd6151e 100644 --- a/routes/web.php +++ b/routes/web.php @@ -430,7 +430,7 @@ }); }); - Route::apiResource('tags', 'BeatmapTagsController', ['only' => ['index', 'store', 'destroy']]); + Route::apiResource('tags', 'BeatmapTagsController', ['only' => ['store', 'destroy']]); }); }); diff --git a/tests/Controllers/BeatmapTagsControllerTest.php b/tests/Controllers/BeatmapTagsControllerTest.php index 84b17bd570f..f75438c1454 100644 --- a/tests/Controllers/BeatmapTagsControllerTest.php +++ b/tests/Controllers/BeatmapTagsControllerTest.php @@ -12,7 +12,6 @@ use App\Models\Solo\Score; use App\Models\Tag; use App\Models\User; -use Illuminate\Testing\Fluent\AssertableJson; use Tests\TestCase; class BeatmapTagsControllerTest extends TestCase @@ -21,21 +20,6 @@ class BeatmapTagsControllerTest extends TestCase private Beatmap $beatmap; private BeatmapTag $beatmapTag; - public function testIndex(): void - { - $this->actAsScopedUser(User::factory()->create(), ['public']); - - $this - ->get(route('api.beatmaps.tags.index', ['beatmap' => $this->beatmap->getKey()])) - ->assertSuccessful() - ->assertJson(fn (AssertableJson $json) => - $json - ->where('beatmap_tags.0.id', $this->tag->getKey()) - ->where('beatmap_tags.0.name', $this->tag->name) - ->where('beatmap_tags.0.count', 1) - ->etc()); - } - public function testStore(): void { $user = User::factory() diff --git a/tests/api_routes.json b/tests/api_routes.json index 68fe32aac33..007f537198f 100644 --- a/tests/api_routes.json +++ b/tests/api_routes.json @@ -173,22 +173,6 @@ ], "scopes": [] }, - { - "uri": "api/v2/beatmaps/{beatmap}/tags", - "methods": [ - "GET", - "HEAD" - ], - "controller": "App\\Http\\Controllers\\BeatmapTagsController@index", - "middlewares": [ - "App\\Http\\Middleware\\ThrottleRequests:1200,1,api:", - "App\\Http\\Middleware\\RequireScopes", - "App\\Http\\Middleware\\RequireScopes:public" - ], - "scopes": [ - "public" - ] - }, { "uri": "api/v2/beatmaps/{beatmap}/tags", "methods": [ From 3c9c2e51c20ec395525c7160fd8c8b0895f4b651 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 17 Dec 2024 22:19:33 +0900 Subject: [PATCH 10/26] not nullable --- resources/js/beatmapsets-show/info.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index 56cbe3641fc..b5e3eadb3e1 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -69,7 +69,7 @@ export default class Info extends React.Component { const tags = this.controller.tags; return [ - ...tags.userTags.map((tag) => tag?.name), + ...tags.userTags.map((tag) => tag.name), ...tags.mapperTags, ]; } From 9e01f3d8ba725393395677f3c5740a02e92b5362 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Dec 2024 15:27:59 +0900 Subject: [PATCH 11/26] don't need the extra join/relation anymore; make into query --- app/Models/Beatmap.php | 2 +- app/Models/BeatmapTag.php | 12 ++++++++++++ app/Models/Tag.php | 16 ---------------- .../BeatmapsetCompactTransformer.php | 2 +- resources/js/beatmapsets-show/controller.ts | 2 +- resources/js/interfaces/beatmap-json.ts | 2 +- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index 57f18ae0aaa..54205677699 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -354,7 +354,7 @@ public function topTagIds() "beatmap_top_tag_ids:{$this->getKey()}", $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], [], - fn () => Tag::topTagIds($this->getKey())->toArray(), + fn () => BeatmapTag::topTagIdsQuery($this->getKey())->get()->toArray(), ); } diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index a4fca351852..8b1cf46fd17 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -19,6 +19,18 @@ class BeatmapTag extends Model protected $primaryKey = ':composite'; protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id']; + public static function topTagIdsQuery(int $beatmapId, int $limit = 50) + { + return static::where('beatmap_id', $beatmapId) + ->whereHas('user', fn ($userQuery) => $userQuery->default()) + ->groupBy('tag_id') + ->select('tag_id') + ->selectRaw('COUNT(*) as count') + ->orderBy('count', 'desc') + ->orderBy('tag_id', 'asc') + ->limit($limit); + } + public function beatmap() { return $this->belongsTo(Beatmap::class, 'beatmap_id'); diff --git a/app/Models/Tag.php b/app/Models/Tag.php index 74293c5c00c..92134fa99ad 100644 --- a/app/Models/Tag.php +++ b/app/Models/Tag.php @@ -22,20 +22,4 @@ public function beatmapTags(): HasMany { return $this->hasMany(BeatmapTag::class); } - - public static function topTagIds(int $beatmapId, int $limit = 50) - { - return static - ::joinRelation( - 'beatmapTags', - fn ($q) => $q->where('beatmap_id', $beatmapId)->whereHas('user', fn ($userQuery) => $userQuery->default()) - ) - ->groupBy('id') - ->select('id') - ->selectRaw('COUNT(*) as count') - ->orderBy('count', 'desc') - ->orderBy('id', 'desc') - ->limit($limit) - ->get(); - } } diff --git a/app/Transformers/BeatmapsetCompactTransformer.php b/app/Transformers/BeatmapsetCompactTransformer.php index 59f47b44d71..f4f95e18cfd 100644 --- a/app/Transformers/BeatmapsetCompactTransformer.php +++ b/app/Transformers/BeatmapsetCompactTransformer.php @@ -303,7 +303,7 @@ public function includeRelatedUsers(Beatmapset $beatmapset) public function includeRelatedTags(Beatmapset $beatmapset) { $beatmaps = $this->beatmaps($beatmapset); - $tagIdSet = new Set($beatmaps->flatMap->topTagIds()->pluck('id')); + $tagIdSet = new Set($beatmaps->flatMap->topTagIds()->pluck('tag_id')); $cachedTags = app('tags'); $json = []; diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index e93119133f3..04ec53a20a5 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -96,7 +96,7 @@ export default class Controller { if (beatmap.top_tag_ids == null) continue; for (const tagId of beatmap.top_tag_ids) { - const tag = tagMap.get(tagId.id); + const tag = tagMap.get(tagId.tag_id); if (tag == null) continue; tag.count += tagId.count; diff --git a/resources/js/interfaces/beatmap-json.ts b/resources/js/interfaces/beatmap-json.ts index 9c5837a51ea..f00b9b24a14 100644 --- a/resources/js/interfaces/beatmap-json.ts +++ b/resources/js/interfaces/beatmap-json.ts @@ -17,7 +17,7 @@ interface BeatmapJsonAvailableIncludes { failtimes: BeatmapFailTimesArray; max_combo: number; owners: BeatmapOwnerJson[]; - top_tag_ids: { count: number; id: number }[]; + top_tag_ids: { count: number; tag_id: number }[]; user: UserJson; } From 715db0347cc4256d0a453dc23fe5d2d3bf983fc7 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Dec 2024 15:41:00 +0900 Subject: [PATCH 12/26] copy and return typed value instead of messing with original object --- resources/js/beatmapsets-show/controller.ts | 15 ++++++++++++--- resources/js/interfaces/tag-json.ts | 3 --- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 04ec53a20a5..c4fc7f1fd53 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -2,7 +2,7 @@ // See the LICENCE file in the repository root for full licence text. import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-json'; -import { TagJsonWithCount } from 'interfaces/tag-json'; +import TagJson from 'interfaces/tag-json'; import UserJson from 'interfaces/user-json'; import { keyBy } from 'lodash'; import { action, computed, makeObservable, observable, runInAction } from 'mobx'; @@ -25,6 +25,16 @@ interface State { showingNsfwWarning: boolean; } +type TagJsonWithCount = TagJson & { count: number }; + +function asTagJsonWithCount(tag: TagJson) { + return { + count: 0, + ...tag, + }; +} + + export default class Controller { @observable hoveredBeatmap: null | BeatmapJsonForBeatmapsetShow = null; @observable state: State; @@ -88,8 +98,7 @@ export default class Controller { const tagMap = new Map(); for (const tag of this.beatmapset.related_tags) { - tag.count = 0; // assign 0 and cast - tagMap.set(tag.id, tag as TagJsonWithCount); + tagMap.set(tag.id, asTagJsonWithCount(tag)); } for (const beatmap of this.beatmapset.beatmaps) { diff --git a/resources/js/interfaces/tag-json.ts b/resources/js/interfaces/tag-json.ts index da857843edc..5d1352a7a4b 100644 --- a/resources/js/interfaces/tag-json.ts +++ b/resources/js/interfaces/tag-json.ts @@ -2,10 +2,7 @@ // See the LICENCE file in the repository root for full licence text. export default interface TagJson { - count?: number; description: string; id: number; name: string; } - -export type TagJsonWithCount = TagJson & Required>; From dd68826c02a7a4de654abc86457a89993d5dfb3b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 18 Dec 2024 21:02:44 +0900 Subject: [PATCH 13/26] don't double fetch from redis for transformer --- app/Models/Beatmap.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index 54205677699..3cca72263e3 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -8,6 +8,7 @@ use App\Exceptions\InvariantException; use App\Jobs\EsDocument; use App\Libraries\Transactions\AfterCommit; +use App\Traits\Memoizes; use DB; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; @@ -53,7 +54,7 @@ */ class Beatmap extends Model implements AfterCommit { - use SoftDeletes; + use Memoizes, SoftDeletes; public $convert = false; @@ -350,11 +351,15 @@ public function status() public function topTagIds() { - return cache_remember_mutexed( - "beatmap_top_tag_ids:{$this->getKey()}", - $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], - [], - fn () => BeatmapTag::topTagIdsQuery($this->getKey())->get()->toArray(), + // TODO: Add option to multi query when beatmapset requests all tags for beatmaps? + return $this->memoize( + __FUNCTION__, + fn () => cache_remember_mutexed( + "beatmap_top_tag_ids:{$this->getKey()}", + $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], + [], + fn () => BeatmapTag::topTagIdsQuery($this->getKey())->get()->toArray(), + ), ); } From 2b3bb426650d18485bd32964b82a19ef87cf1d8a Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 19 Dec 2024 16:32:55 +0900 Subject: [PATCH 14/26] should be unique --- database/factories/TagFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/factories/TagFactory.php b/database/factories/TagFactory.php index f1bb27b044d..8617d60d431 100644 --- a/database/factories/TagFactory.php +++ b/database/factories/TagFactory.php @@ -16,7 +16,7 @@ class TagFactory extends Factory public function definition(): array { return [ - 'name' => fn () => "Tag {$this->faker->word}", + 'name' => fn () => "Tag {$this->faker->unique()->word}", 'description' => fn () => $this->faker->sentence, ]; } From 05db2c1f21bf2d951f43ab5d2fe779e0efbf7be1 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 19 Dec 2024 19:49:11 +0900 Subject: [PATCH 15/26] removed/unused --- resources/js/beatmapsets-show/controller.ts | 10 ---------- resources/js/interfaces/beatmapset-json.ts | 1 - 2 files changed, 11 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index c4fc7f1fd53..3f2050db321 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -82,16 +82,6 @@ export default class Controller { return this.beatmaps.get(this.currentBeatmap.mode) ?? []; } - get relatedTags() { - const map = new Map(); - - for (const tag of this.beatmapset.related_tags) { - map.set(tag.id, tag); - } - - return map; - } - @computed get tags() { const mapperTagSet = new Set(this.beatmapset.tags.split(' ').filter(present)); diff --git a/resources/js/interfaces/beatmapset-json.ts b/resources/js/interfaces/beatmapset-json.ts index 6519a014d1d..f6203c3c052 100644 --- a/resources/js/interfaces/beatmapset-json.ts +++ b/resources/js/interfaces/beatmapset-json.ts @@ -96,7 +96,6 @@ interface BeatmapsetJsonAvailableIncludes { related_tags: TagJson[]; related_users: UserJson[]; user: UserJson | UserJsonDeleted; - user_tags: TagJson[]; } interface HypeData { From 0eefd0b23cd3da7217c286e2152130330f80a46c Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 19 Dec 2024 19:55:54 +0900 Subject: [PATCH 16/26] missing non-incrementing --- app/Models/BeatmapTag.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index 8b1cf46fd17..96a7be925ce 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -18,6 +18,7 @@ class BeatmapTag extends Model { protected $primaryKey = ':composite'; protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id']; + public $incrementing = false; public static function topTagIdsQuery(int $beatmapId, int $limit = 50) { From c9ad28dad76504b4a94430b4d03ead1fe48d0e6b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 19 Dec 2024 20:01:41 +0900 Subject: [PATCH 17/26] memoize all --- app/Singletons/Tags.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/Singletons/Tags.php b/app/Singletons/Tags.php index b8cccbd6e46..589bad8b557 100644 --- a/app/Singletons/Tags.php +++ b/app/Singletons/Tags.php @@ -10,16 +10,25 @@ use App\Models\Tag; use App\Traits\Memoizes; use App\Transformers\TagTransformer; +use Illuminate\Support\Collection; class Tags { use Memoizes; + /** + * @return Collection + */ + public function all(): Collection + { + return $this->memoize(__FUNCTION__, fn () => Tag::all()); + } + public function get(int $id): ?Tag { $allById = $this->memoize( 'allById', - fn () => Tag::all()->keyBy('id'), + fn () => $this->all()->keyBy('id'), ); return $allById[$id] ?? null; @@ -29,7 +38,7 @@ public function json(): array { return $this->memoize( __FUNCTION__, - fn () => json_collection(Tag::all(), new TagTransformer()), + fn () => json_collection($this->all(), new TagTransformer()), ); } } From 421504c8350250e4c5fcb20dac98e86eff2299b5 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Fri, 20 Dec 2024 23:11:06 +0900 Subject: [PATCH 18/26] add composite key test; add missing timestamp properties --- app/Models/BeatmapTag.php | 2 ++ tests/Models/ModelCompositePrimaryKeysTest.php | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index 96a7be925ce..fc7abf63ce8 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -10,7 +10,9 @@ /** * @property-read Beatmap $beatmap * @property int $beatmap_id + * @property \Carbon\Carbon $created_at * @property int $tag_id + * @property \Carbon\Carbon $updated_at * @property-read User $user * @property int $user_id */ diff --git a/tests/Models/ModelCompositePrimaryKeysTest.php b/tests/Models/ModelCompositePrimaryKeysTest.php index d2e02b3edac..58d8b97575f 100644 --- a/tests/Models/ModelCompositePrimaryKeysTest.php +++ b/tests/Models/ModelCompositePrimaryKeysTest.php @@ -10,6 +10,7 @@ use App\Models\BeatmapDifficulty; use App\Models\BeatmapDifficultyAttrib; use App\Models\BeatmapFailtimes; +use App\Models\BeatmapTag; use App\Models\Chat; use App\Models\FavouriteBeatmapset; use App\Models\Forum; @@ -112,6 +113,16 @@ public static function dataProviderBase() ['type' => 'exit'], ['p1', [0, 10], 11], ], + [ + BeatmapTag::class, + [ + 'beatmap_id' => 0, + 'tag_id' => 0, + 'user_id' => 0, + ], + ['tag_id' => 1], + ['updated_at', [Carbon::now()->subDays(5), Carbon::now()->subDays(1)], Carbon::now()], + ], [ Chat\UserChannel::class, [ From efad0be5359e5d2508d5cb1e1915a3889c03a45a Mon Sep 17 00:00:00 2001 From: bakaneko Date: Sat, 21 Dec 2024 00:20:47 +0900 Subject: [PATCH 19/26] tie break with names --- resources/js/beatmapsets-show/controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 3f2050db321..df472a63140 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -112,7 +112,7 @@ export default class Controller { mapperTags: [...mapperTagSet.values()], userTags: [...tagMap.values()].sort((a, b) => { const diff = b.count - a.count; - return diff !== 0 ? diff : a.id - b.id; + return diff !== 0 ? diff : a.name.localeCompare(b.name); }), }; } From cde18d06af8beaee20e4c780c3f3b43a7e309454 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Sat, 21 Dec 2024 00:22:36 +0900 Subject: [PATCH 20/26] don't dedupe mapper tags since they might be names --- resources/js/beatmapsets-show/controller.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index df472a63140..15f9acc725d 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -84,7 +84,6 @@ export default class Controller { @computed get tags() { - const mapperTagSet = new Set(this.beatmapset.tags.split(' ').filter(present)); const tagMap = new Map(); for (const tag of this.beatmapset.related_tags) { @@ -99,17 +98,11 @@ export default class Controller { if (tag == null) continue; tag.count += tagId.count; - - // TODO: case insensitivity - if (mapperTagSet.has(tag.name)) { - tag.count++; - mapperTagSet.delete(tag.name); - } } } return { - mapperTags: [...mapperTagSet.values()], + mapperTags: this.beatmapset.tags.split(' ').filter(present), userTags: [...tagMap.values()].sort((a, b) => { const diff = b.count - a.count; return diff !== 0 ? diff : a.name.localeCompare(b.name); From 572b0461c9c5906a67def30741526111afb3c837 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 25 Dec 2024 08:02:38 +0900 Subject: [PATCH 21/26] show user tags per beatmap --- resources/js/beatmapsets-show/controller.ts | 27 +++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 15f9acc725d..a48de73ff41 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -83,27 +83,34 @@ export default class Controller { } @computed - get tags() { - const tagMap = new Map(); + get relatedTags() { + const map = new Map(); for (const tag of this.beatmapset.related_tags) { - tagMap.set(tag.id, asTagJsonWithCount(tag)); + map.set(tag.id, tag); } - for (const beatmap of this.beatmapset.beatmaps) { - if (beatmap.top_tag_ids == null) continue; + return map; + } + + @computed + get tags() { + const userTags: TagJsonWithCount[] = []; - for (const tagId of beatmap.top_tag_ids) { - const tag = tagMap.get(tagId.tag_id); - if (tag == null) continue; + if (this.currentBeatmap.top_tag_ids != null) { + for (const tagId of this.currentBeatmap.top_tag_ids) { + const maybeTag = this.relatedTags.get(tagId.tag_id); + if (maybeTag == null) continue; - tag.count += tagId.count; + const tag = asTagJsonWithCount(maybeTag); + tag.count = tagId.count; + userTags.push(asTagJsonWithCount(tag)); } } return { mapperTags: this.beatmapset.tags.split(' ').filter(present), - userTags: [...tagMap.values()].sort((a, b) => { + userTags: userTags.sort((a, b) => { const diff = b.count - a.count; return diff !== 0 ? diff : a.name.localeCompare(b.name); }), From 73162ed460fa34e3bd06f6f628fc2384f6fa8fc2 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 14 Jan 2025 13:09:01 +0900 Subject: [PATCH 22/26] already a copy --- resources/js/beatmapsets-show/controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index a48de73ff41..b7651e70e9a 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -104,7 +104,7 @@ export default class Controller { const tag = asTagJsonWithCount(maybeTag); tag.count = tagId.count; - userTags.push(asTagJsonWithCount(tag)); + userTags.push(tag); } } From 585fa90fbf894ef3c2898f8a1cee4e99e0fd42af Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 14 Jan 2025 14:40:27 +0900 Subject: [PATCH 23/26] tags not being deduped anymore --- resources/js/beatmapsets-show/info.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/js/beatmapsets-show/info.tsx b/resources/js/beatmapsets-show/info.tsx index b5e3eadb3e1..dcbb8e30f3d 100644 --- a/resources/js/beatmapsets-show/info.tsx +++ b/resources/js/beatmapsets-show/info.tsx @@ -202,8 +202,8 @@ export default class Info extends React.Component { {trans('beatmapsets.show.info.tags')}
- {this.tags.map((tag) => ( - + {this.tags.map((tag, i) => ( + Date: Tue, 14 Jan 2025 15:28:34 +0900 Subject: [PATCH 24/26] fix property ordering --- app/Models/BeatmapTag.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index fc7abf63ce8..c83f36a0f8d 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -18,9 +18,10 @@ */ class BeatmapTag extends Model { + public $incrementing = false; + protected $primaryKey = ':composite'; protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id']; - public $incrementing = false; public static function topTagIdsQuery(int $beatmapId, int $limit = 50) { From be4740055755b2ccca8440b7fbeea498ba9eb245 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 14 Jan 2025 15:31:19 +0900 Subject: [PATCH 25/26] only used in one place now... --- resources/js/beatmapsets-show/controller.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index b7651e70e9a..8216dd4c523 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -27,14 +27,6 @@ interface State { type TagJsonWithCount = TagJson & { count: number }; -function asTagJsonWithCount(tag: TagJson) { - return { - count: 0, - ...tag, - }; -} - - export default class Controller { @observable hoveredBeatmap: null | BeatmapJsonForBeatmapsetShow = null; @observable state: State; @@ -102,9 +94,7 @@ export default class Controller { const maybeTag = this.relatedTags.get(tagId.tag_id); if (maybeTag == null) continue; - const tag = asTagJsonWithCount(maybeTag); - tag.count = tagId.count; - userTags.push(tag); + userTags.push({ ...maybeTag, count: tagId.count } ); } } From 3dee3fcb29c85ddc9ab4e77374640785ec6a5ada Mon Sep 17 00:00:00 2001 From: bakaneko Date: Tue, 14 Jan 2025 16:46:40 +0900 Subject: [PATCH 26/26] can be a scope? --- app/Models/Beatmap.php | 2 +- app/Models/BeatmapTag.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/Models/Beatmap.php b/app/Models/Beatmap.php index 3cca72263e3..9bda60e088d 100644 --- a/app/Models/Beatmap.php +++ b/app/Models/Beatmap.php @@ -358,7 +358,7 @@ public function topTagIds() "beatmap_top_tag_ids:{$this->getKey()}", $GLOBALS['cfg']['osu']['tags']['beatmap_tags_cache_duration'], [], - fn () => BeatmapTag::topTagIdsQuery($this->getKey())->get()->toArray(), + fn () => $this->beatmapTags()->topTagIds()->limit(50)->get()->toArray(), ), ); } diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index c83f36a0f8d..a61da0fc571 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -7,6 +7,8 @@ namespace App\Models; +use Illuminate\Contracts\Database\Eloquent\Builder; + /** * @property-read Beatmap $beatmap * @property int $beatmap_id @@ -23,16 +25,14 @@ class BeatmapTag extends Model protected $primaryKey = ':composite'; protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id']; - public static function topTagIdsQuery(int $beatmapId, int $limit = 50) + public function scopeTopTagIds(Builder $query) { - return static::where('beatmap_id', $beatmapId) - ->whereHas('user', fn ($userQuery) => $userQuery->default()) + return $query->whereHas('user', fn ($userQuery) => $userQuery->default()) ->groupBy('tag_id') ->select('tag_id') ->selectRaw('COUNT(*) as count') ->orderBy('count', 'desc') - ->orderBy('tag_id', 'asc') - ->limit($limit); + ->orderBy('tag_id', 'asc'); } public function beatmap()