Extract duplicated blocked-id and duplicate-shortcode query patterns

Two query patterns were copy-pasted across several call sites:

- The 'users who blocked me, plus myself' list used to filter profile
  search (UserFilter::whereFilterableId($pid)->pluck('user_id')->push($pid))
  appeared in ComposeController (x2) and DirectMessageController. Extracted
  to UserFilterService::searchExcludedProfileIds(). Note this is the
  inverse of blocks() (who I blocked), so it is a distinct method.

- CustomEmoji duplicate detection (groupBy('shortcode')->havingRaw(
  'count(*) > 1')) appeared three times in AdminController. Extracted to a
  CustomEmoji::duplicateShortcodes() query scope.

Adds tests for both. No behaviour change.
pull/7039/head
Your Name 3 weeks ago
parent 7ad550d2f7
commit e4e12fad7c

@ -570,7 +570,7 @@ class AdminController extends Controller
} elseif ($sort == 'remote') {
return $query->latest()->where('domain', '!=', config('pixelfed.domain.app'));
} elseif ($sort == 'duplicates') {
return $query->latest()->groupBy('shortcode')->havingRaw('count(*) > 1');
return $query->latest()->duplicateShortcodes();
} elseif ($sort == 'disabled') {
return $query->latest()->whereDisabled(true);
} elseif ($sort == 'search') {
@ -598,9 +598,9 @@ class AdminController extends Controller
];
if ($pg) {
$res['duplicate'] = CustomEmoji::select('shortcode')->groupBy('shortcode')->havingRaw('count(*) > 1')->count();
$res['duplicate'] = CustomEmoji::select('shortcode')->duplicateShortcodes()->count();
} else {
$res['duplicate'] = CustomEmoji::groupBy('shortcode')->havingRaw('count(*) > 1')->count();
$res['duplicate'] = CustomEmoji::duplicateShortcodes()->count();
}
return $res;

@ -14,7 +14,6 @@ use App\Models\Notification;
use App\Models\Poll;
use App\Models\Profile;
use App\Models\Status;
use App\Models\UserFilter;
use App\Services\AccountService;
use App\Services\CollectionService;
use App\Services\MediaBlocklistService;
@ -23,6 +22,7 @@ use App\Services\MediaStorageService;
use App\Services\MediaTagService;
use App\Services\PlaceService;
use App\Services\SnowflakeService;
use App\Services\UserFilterService;
use App\Services\UserRoleService;
use App\Services\UserStorageService;
use App\Transformer\Api\MediaTransformer;
@ -262,12 +262,7 @@ class ComposeController extends Controller
abort_if($user->has_roles && ! UserRoleService::can('can-post', $user->id), 403, 'Invalid permissions for this action');
$blocked = UserFilter::whereFilterableType(Profile::class)
->whereFilterType('block')
->whereFilterableId($request->user()->profile_id)
->pluck('user_id');
$blocked->push($request->user()->profile_id);
$blocked = UserFilterService::searchExcludedProfileIds($request->user()->profile_id);
$operator = config('database.default') === 'pgsql' ? 'ilike' : 'like';
$results = Profile::select([
@ -454,11 +449,7 @@ class ComposeController extends Controller
return [];
}
$blocked = UserFilter::whereFilterableType(Profile::class)
->whereFilterType('block')
->whereFilterableId($request->user()->profile_id)
->pluck('user_id')
->push($request->user()->profile_id);
$blocked = UserFilterService::searchExcludedProfileIds($request->user()->profile_id);
$currentUserId = $request->user()->profile_id;
$operator = config('database.default') === 'pgsql' ? 'ilike' : 'like';

@ -572,12 +572,7 @@ class DirectMessageController extends Controller
$q = mb_substr($q, 1);
}
$blocked = UserFilter::whereFilterableType(Profile::class)
->whereFilterType('block')
->whereFilterableId($request->user()->profile_id)
->pluck('user_id');
$blocked->push($request->user()->profile_id);
$blocked = UserFilterService::searchExcludedProfileIds($request->user()->profile_id);
$results = Profile::select('id', 'domain', 'username')
->whereNotIn('id', $blocked)

@ -17,6 +17,14 @@ class CustomEmoji extends Model
protected $guarded = [];
/**
* Restrict the query to shortcodes that appear on more than one row.
*/
public function scopeDuplicateShortcodes($query)
{
return $query->groupBy('shortcode')->havingRaw('count(*) > 1');
}
public static function scan($text, $activitypub = false)
{
if ((bool) config_cache('federation.custom_emoji.enabled') == false) {

@ -2,8 +2,10 @@
namespace App\Services;
use App\Models\Profile;
use App\Models\UserDomainBlock;
use App\Models\UserFilter;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Redis;
@ -83,6 +85,22 @@ class UserFilterService
return array_unique(array_merge(self::mutes($profile_id), self::blocks($profile_id)));
}
/**
* Profile ids that should be excluded from profile search results for the
* given profile: everyone who has blocked this profile, plus the profile
* itself. Returned as a collection so callers can use whereNotIn().
*
* @return Collection<int, int>
*/
public static function searchExcludedProfileIds(int $profile_id)
{
return UserFilter::whereFilterableType(Profile::class)
->whereFilterType('block')
->whereFilterableId($profile_id)
->pluck('user_id')
->push($profile_id);
}
public static function mute(int $profile_id, int $muted_id)
{
if ($profile_id == $muted_id) {

@ -0,0 +1,69 @@
<?php
use App\Models\CustomEmoji;
use App\Models\Profile;
use App\Models\User;
use App\Models\UserFilter;
use App\Services\UserFilterService;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
uses(LazilyRefreshDatabase::class);
/*
|--------------------------------------------------------------------------
| Extracted query patterns
|--------------------------------------------------------------------------
|
| Covers UserFilterService::searchExcludedProfileIds (blocked-by ids + self,
| previously inlined in three search endpoints) and the
| CustomEmoji::duplicateShortcodes scope (previously three copies of
| groupBy('shortcode')->havingRaw('count(*) > 1')).
|
*/
describe('UserFilterService::searchExcludedProfileIds', function () {
it('returns ids of profiles that blocked the user plus the users own id', function () {
$user = User::factory()->create();
$user->refresh();
$blocker = User::factory()->create();
$blocker->refresh();
// $blocker has blocked $user: user_id = blocker, filterable_id = user.
$filter = new UserFilter;
$filter->user_id = $blocker->profile_id;
$filter->filterable_id = $user->profile_id;
$filter->filterable_type = Profile::class;
$filter->filter_type = 'block';
$filter->save();
$excluded = UserFilterService::searchExcludedProfileIds($user->profile_id);
expect($excluded->all())
->toContain($blocker->profile_id)
->toContain($user->profile_id);
});
it('returns only the users own id when nobody has blocked them', function () {
$user = User::factory()->create();
$user->refresh();
$excluded = UserFilterService::searchExcludedProfileIds($user->profile_id);
expect($excluded->all())->toBe([$user->profile_id]);
});
});
describe('CustomEmoji::duplicateShortcodes', function () {
it('only matches shortcodes that appear on more than one row', function () {
// Two rows share :dupe: (different domains satisfy the unique index).
CustomEmoji::create(['shortcode' => ':dupe:', 'domain' => 'a.example']);
CustomEmoji::create(['shortcode' => ':dupe:', 'domain' => 'b.example']);
// Unique shortcode, should be excluded.
CustomEmoji::create(['shortcode' => ':unique:', 'domain' => 'a.example']);
$shortcodes = CustomEmoji::duplicateShortcodes()->pluck('shortcode')->all();
expect($shortcodes)->toContain(':dupe:')->not->toContain(':unique:');
expect(CustomEmoji::duplicateShortcodes()->count())->toBe(1);
});
});
Loading…
Cancel
Save