diff --git a/app/Http/Controllers/BeatmapTagsController.php b/app/Http/Controllers/BeatmapTagsController.php index 87c9d9b253b..f6cf56b035a 100644 --- a/app/Http/Controllers/BeatmapTagsController.php +++ b/app/Http/Controllers/BeatmapTagsController.php @@ -23,22 +23,6 @@ public function __construct() 'destroy', ], ]); - - $this->middleware('require-scopes:public', ['only' => 'index']); - } - - 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, - ]; } public function destroy($beatmapId, $tagId) diff --git a/app/Http/Controllers/BeatmapsetsController.php b/app/Http/Controllers/BeatmapsetsController.php index d0958f9567a..a2da6fc6b37 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.top_tag_ids', 'converts', 'converts.failtimes', 'converts.owners', @@ -415,6 +416,7 @@ private function showJson($beatmapset) 'pack_tags', 'ratings', 'recent_favourites', + 'related_tags', 'related_users', 'user', ]); diff --git a/app/Http/Controllers/TagsController.php b/app/Http/Controllers/TagsController.php index 80b657d9182..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() @@ -21,15 +18,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..9bda60e088d 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; @@ -348,6 +349,20 @@ public function status() return array_search($this->approved, Beatmapset::STATES, true); } + public function topTagIds() + { + // 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 () => $this->beatmapTags()->topTagIds()->limit(50)->get()->toArray(), + ), + ); + } + private function getDifficultyrating() { if ($this->convert) { diff --git a/app/Models/BeatmapTag.php b/app/Models/BeatmapTag.php index a4fca351852..a61da0fc571 100644 --- a/app/Models/BeatmapTag.php +++ b/app/Models/BeatmapTag.php @@ -7,18 +7,34 @@ namespace App\Models; +use Illuminate\Contracts\Database\Eloquent\Builder; + /** * @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 */ class BeatmapTag extends Model { + public $incrementing = false; + protected $primaryKey = ':composite'; protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id']; + public function scopeTopTagIds(Builder $query) + { + 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'); + } + public function beatmap() { return $this->belongsTo(Beatmap::class, 'beatmap_id'); diff --git a/app/Models/Tag.php b/app/Models/Tag.php index 3513ab3d891..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 topTags($beatmapId) - { - return static - ::joinRelation( - 'beatmapTags', - fn ($q) => $q->where('beatmap_id', $beatmapId)->whereHas('user', fn ($userQuery) => $userQuery->default()) - ) - ->groupBy('id') - ->select('id', 'name') - ->selectRaw('COUNT(*) as count') - ->orderBy('count', 'desc') - ->orderBy('id', 'desc') - ->limit(50) - ->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..589bad8b557 --- /dev/null +++ b/app/Singletons/Tags.php @@ -0,0 +1,44 @@ +. 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; +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 () => $this->all()->keyBy('id'), + ); + + return $allById[$id] ?? null; + } + + public function json(): array + { + return $this->memoize( + __FUNCTION__, + fn () => json_collection($this->all(), new TagTransformer()), + ); + } +} diff --git a/app/Transformers/BeatmapCompactTransformer.php b/app/Transformers/BeatmapCompactTransformer.php index ef8a96c6542..9ec4aa198c3 100644 --- a/app/Transformers/BeatmapCompactTransformer.php +++ b/app/Transformers/BeatmapCompactTransformer.php @@ -18,6 +18,7 @@ class BeatmapCompactTransformer extends TransformerAbstract 'failtimes', 'max_combo', 'owners', + 'top_tag_ids', 'user', ]; @@ -83,6 +84,11 @@ public function includeOwners(Beatmap $beatmap) ]); } + public function includeTopTagIds(Beatmap $beatmap) + { + return $this->primitive($beatmap->topTagIds()); + } + public function includeUser(Beatmap $beatmap) { return $this->item( diff --git a/app/Transformers/BeatmapsetCompactTransformer.php b/app/Transformers/BeatmapsetCompactTransformer.php index 546fe4125fe..f4f95e18cfd 100644 --- a/app/Transformers/BeatmapsetCompactTransformer.php +++ b/app/Transformers/BeatmapsetCompactTransformer.php @@ -40,6 +40,7 @@ class BeatmapsetCompactTransformer extends TransformerAbstract 'ratings', 'recent_favourites', 'related_users', + 'related_tags', 'user', ]; @@ -299,6 +300,24 @@ public function includeRelatedUsers(Beatmapset $beatmapset) return $this->collection($users, new UserCompactTransformer()); } + public function includeRelatedTags(Beatmapset $beatmapset) + { + $beatmaps = $this->beatmaps($beatmapset); + $tagIdSet = new Set($beatmaps->flatMap->topTagIds()->pluck('tag_id')); + + $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 { $rel = $beatmapset->trashed() || ($params !== null && $params->get('with_trashed')) ? 'allBeatmaps' : 'beatmaps'; 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, ]; } diff --git a/resources/js/beatmapsets-show/controller.ts b/resources/js/beatmapsets-show/controller.ts index 7447723e6cf..8216dd4c523 100644 --- a/resources/js/beatmapsets-show/controller.ts +++ b/resources/js/beatmapsets-show/controller.ts @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. import { BeatmapsetJsonForShow } from 'interfaces/beatmapset-extended-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'; @@ -10,6 +11,7 @@ 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'; @@ -23,6 +25,8 @@ interface State { showingNsfwWarning: boolean; } +type TagJsonWithCount = TagJson & { count: number }; + export default class Controller { @observable hoveredBeatmap: null | BeatmapJsonForBeatmapsetShow = null; @observable state: State; @@ -70,6 +74,39 @@ export default class Controller { return this.beatmaps.get(this.currentBeatmap.mode) ?? []; } + @computed + 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 userTags: TagJsonWithCount[] = []; + + 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; + + userTags.push({ ...maybeTag, count: tagId.count } ); + } + } + + return { + mapperTags: this.beatmapset.tags.split(' ').filter(present), + userTags: userTags.sort((a, b) => { + const diff = b.count - a.count; + return diff !== 0 ? diff : a.name.localeCompare(b.name); + }), + }; + } + @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 a90cdb9e042..dcbb8e30f3d 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() { + const tags = this.controller.tags; + + return [ + ...tags.userTags.map((tag) => tag.name), + ...tags.mapperTags, + ]; + } + 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,13 +196,13 @@ 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, i) => ( >; diff --git a/resources/js/interfaces/beatmapset-json.ts b/resources/js/interfaces/beatmapset-json.ts index 533ee955779..f6203c3c052 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 { @@ -92,6 +93,7 @@ interface BeatmapsetJsonAvailableIncludes { nominations: BeatmapsetNominationsInterface; ratings: number[]; recent_favourites: UserJson[]; + related_tags: TagJson[]; related_users: UserJson[]; user: UserJson | UserJsonDeleted; } 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; +} diff --git a/routes/web.php b/routes/web.php index 2dc8ccb6689..071aeacdf37 100644 --- a/routes/web.php +++ b/routes/web.php @@ -431,7 +431,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/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, [ 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": [