Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include user tags in Beatmapset search #11763

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
545c945
render user tags
notbakaneko Dec 10, 2024
4ae47f9
cache tags locally, only cache beatmap tags with ids and counts; retu…
notbakaneko Dec 12, 2024
4a55d98
return tags with counts with beatmaps
notbakaneko Dec 13, 2024
b3fcdfa
add TagJsonWithCount type
notbakaneko Dec 13, 2024
d883075
merge user and mapper tags, place mapper tags at the end
notbakaneko Dec 13, 2024
3c9a6f5
do the sorting in controller
notbakaneko Dec 13, 2024
1503ceb
lint
notbakaneko Dec 17, 2024
f5552af
return id and counts seperately from full tag
notbakaneko Dec 17, 2024
5ad134a
remove route (get from beatmapset instead)
notbakaneko Dec 17, 2024
3c9c2e5
not nullable
notbakaneko Dec 17, 2024
9e01f3d
don't need the extra join/relation anymore; make into query
notbakaneko Dec 18, 2024
715db03
copy and return typed value instead of messing with original object
notbakaneko Dec 18, 2024
dd68826
don't double fetch from redis for transformer
notbakaneko Dec 18, 2024
2b3bb42
should be unique
notbakaneko Dec 19, 2024
05db2c1
removed/unused
notbakaneko Dec 19, 2024
0eefd0b
missing non-incrementing
notbakaneko Dec 19, 2024
c9ad28d
memoize all
notbakaneko Dec 19, 2024
421504c
add composite key test; add missing timestamp properties
notbakaneko Dec 20, 2024
efad0be
tie break with names
notbakaneko Dec 20, 2024
cde18d0
don't dedupe mapper tags since they might be names
notbakaneko Dec 20, 2024
572b046
show user tags per beatmap
notbakaneko Dec 24, 2024
f7550ff
include tags into index
notbakaneko Nov 29, 2024
ae0812d
remove unlikely nulls
notbakaneko Dec 19, 2024
8a9afd1
support unique indexing job
notbakaneko Dec 19, 2024
8c34076
index tags per beatmap instead
notbakaneko Dec 25, 2024
d69738c
search nested tags
notbakaneko Dec 25, 2024
551769b
lint
notbakaneko Dec 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions app/Http/Controllers/BeatmapTagsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions app/Http/Controllers/BeatmapsetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ private function showJson($beatmapset)
'beatmaps.failtimes',
'beatmaps.max_combo',
'beatmaps.owners',
'beatmaps.top_tag_ids',
'converts',
'converts.failtimes',
'converts.owners',
Expand All @@ -415,6 +416,7 @@ private function showJson($beatmapset)
'pack_tags',
'ratings',
'recent_favourites',
'related_tags',
'related_users',
'user',
]);
Expand Down
12 changes: 1 addition & 11 deletions app/Http/Controllers/TagsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

namespace App\Http\Controllers;

use App\Models\Tag;
use App\Transformers\TagTransformer;

class TagsController extends Controller
{
public function __construct()
Expand All @@ -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(),
];
}
}
2 changes: 1 addition & 1 deletion app/Jobs/EsDocument.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class EsDocument implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable;

private array $modelMeta;
protected array $modelMeta;

/**
* Create a new job instance.
Expand Down
25 changes: 25 additions & 0 deletions app/Jobs/EsDocumentUnique.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. 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\Jobs;

use Illuminate\Contracts\Queue\ShouldBeUnique;

class EsDocumentUnique extends EsDocument implements ShouldBeUnique
{
public int $uniqueFor = 600;

public function __construct($model)
{
parent::__construct($model);
}

public function uniqueId(): string
{
return "{$this->modelMeta['class']}-{$this->modelMeta['id']}";
}
}
6 changes: 6 additions & 0 deletions app/Libraries/Search/BeatmapsetSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ public function getQuery()
->should(['term' => ['_id' => ['value' => $this->params->queryString, 'boost' => 100]]])
->should(QueryHelper::queryString($this->params->queryString, $partialMatchFields, 'or', 1 / count($terms)))
->should(QueryHelper::queryString($this->params->queryString, [], 'and'))
->should([
'nested' => [
'path' => 'beatmaps',
'query' => QueryHelper::queryString($this->params->queryString, ['beatmaps.top_tags'], 'or', 0.5 / count($terms)),
],
])
);
}

Expand Down
21 changes: 18 additions & 3 deletions app/Models/Beatmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
namespace App\Models;

use App\Exceptions\InvariantException;
use App\Jobs\EsDocument;
use App\Jobs\EsDocumentUnique;
use App\Libraries\Transactions\AfterCommit;
use App\Traits\Memoizes;
use DB;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
Expand Down Expand Up @@ -53,7 +54,7 @@
*/
class Beatmap extends Model implements AfterCommit
{
use SoftDeletes;
use Memoizes, SoftDeletes;

public $convert = false;

Expand Down Expand Up @@ -222,7 +223,7 @@ public function afterCommit()
$beatmapset = $this->beatmapset;

if ($beatmapset !== null) {
dispatch(new EsDocument($beatmapset));
dispatch(new EsDocumentUnique($beatmapset));
}
}

Expand Down Expand Up @@ -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 () => BeatmapTag::topTagIdsQuery($this->getKey())->get()->toArray(),
),
);
}

private function getDifficultyrating()
{
if ($this->convert) {
Expand Down
15 changes: 15 additions & 0 deletions app/Models/BeatmapTag.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,29 @@
/**
* @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
{
protected $primaryKey = ':composite';
protected $primaryKeys = ['beatmap_id', 'tag_id', 'user_id'];
public $incrementing = false;

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()
{
Expand Down
4 changes: 2 additions & 2 deletions app/Models/Beatmapset.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use App\Exceptions\ImageProcessorServiceException;
use App\Exceptions\InvariantException;
use App\Jobs\CheckBeatmapsetCovers;
use App\Jobs\EsDocument;
use App\Jobs\EsDocumentUnique;
use App\Jobs\Notifications\BeatmapsetDiscussionLock;
use App\Jobs\Notifications\BeatmapsetDiscussionUnlock;
use App\Jobs\Notifications\BeatmapsetDisqualify;
Expand Down Expand Up @@ -1502,7 +1502,7 @@ public function refreshCache(bool $resetEligibleMainRulesets = false): void

public function afterCommit()
{
dispatch(new EsDocument($this));
dispatch(new EsDocumentUnique($this));
}

public function notificationCover()
Expand Down
16 changes: 0 additions & 16 deletions app/Models/Tag.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
36 changes: 24 additions & 12 deletions app/Models/Traits/Es/BeatmapsetSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,17 @@ private function esBeatmapsValues()
foreach ($this->beatmaps as $beatmap) {
$beatmapValues = [];
foreach ($mappings as $field => $mapping) {
$beatmapValues[$field] = $beatmap->$field;
$value = match ($field) {
'top_tags' => $this->esBeatmapTags($beatmap),
// TODO: remove adding $beatmap->user_id once everything else also populated beatmap_owners by default.
// Duplicate user_id in the array should be fine for now since the field isn't scored for querying.
'user_id' => $beatmap->beatmapOwners->pluck('user_id')->add($beatmap->user_id),
default => $beatmap->$field,
};

$beatmapValues[$field] = $value;
}

// TODO: remove adding $beatmap->user_id once everything else also populated beatmap_owners by default.
// Duplicate user_id in the array should be fine for now since the field isn't scored for querying.
$beatmapValues['user_id'] = $beatmap->beatmapOwners->pluck('user_id')->add($beatmap->user_id);
$values[] = $beatmapValues;

if ($beatmap->playmode === Beatmap::MODES['osu']) {
Expand All @@ -92,15 +97,10 @@ private function esBeatmapsValues()
continue;
}

$convert = clone $beatmap;
$convert->playmode = $modeInt;
$convert->convert = true;
$convertValues = [];
foreach ($mappings as $field => $mapping) {
$convertValues[$field] = $convert->$field;
}
$convertValues = $beatmapValues;
$convertValues['playmode'] = $modeInt;
$convertValues['convert'] = true;

$convertValues['user_id'] = $beatmapValues['user_id']; // just add a copy for converts too.
$values[] = $convertValues;
}
}
Expand All @@ -109,6 +109,18 @@ private function esBeatmapsValues()
return $values;
}

private function esBeatmapTags(Beatmap $beatmap): array
{
$tags = app('tags');

return array_filter(
array_map(
fn ($tagId) => $tags->get($tagId['tag_id'])?->name,
$beatmap->topTagIds()
)
);
}

private function esDifficultiesValues()
{
$mappings = static::esMappings()['difficulties']['properties'];
Expand Down
1 change: 1 addition & 0 deletions app/Providers/AppServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];

Expand Down
44 changes: 44 additions & 0 deletions app/Singletons/Tags.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. 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<Tag>
*/
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()),
);
}
}
6 changes: 6 additions & 0 deletions app/Transformers/BeatmapCompactTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class BeatmapCompactTransformer extends TransformerAbstract
'failtimes',
'max_combo',
'owners',
'top_tag_ids',
'user',
];

Expand Down Expand Up @@ -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(
Expand Down
19 changes: 19 additions & 0 deletions app/Transformers/BeatmapsetCompactTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class BeatmapsetCompactTransformer extends TransformerAbstract
'ratings',
'recent_favourites',
'related_users',
'related_tags',
'user',
];

Expand Down Expand Up @@ -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';
Expand Down
Loading
Loading