Merge pull request #6924 from pixelfed/refactor/profile-count-recalc

Refactor/profile count recalc
pull/6923/head
Shlee 4 weeks ago committed by GitHub
commit f54c3f394f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -3,7 +3,6 @@
namespace App\Console\Commands;
use App\Models\Profile;
use App\Models\Status;
use App\Services\Account\AccountStatService;
use App\Services\AccountService;
use Illuminate\Console\Command;
@ -61,21 +60,18 @@ class AccountPostCountStatUpdate extends Command
return;
}
$statusCount = Status::whereProfileId($id)->count();
if ($statusCount != $acct['statuses_count']) {
$profile = Profile::find($id);
if (! $profile) {
AccountStatService::removeFromPostCount($id);
return;
}
$profile->status_count = $statusCount;
$profile->save();
$profile = Profile::find($id);
if (! $profile) {
AccountStatService::removeFromPostCount($id);
AccountService::del($id);
return;
}
// Reconcile only the status_count column (this queue is fed by
// status create/delete events). Shared recompute logic lives in
// AccountStatService so it stays consistent with fix:profilecounts.
AccountStatService::reconcileProfileCounts($profile, ['statuses']);
AccountStatService::removeFromPostCount($id);
}
}

@ -3,13 +3,11 @@
namespace App\Console\Commands;
use App\Jobs\FollowPipeline\FollowServiceWarmCache;
use App\Models\Follower;
use App\Models\Profile;
use App\Services\AccountService;
use App\Services\Account\AccountStatService;
use App\Services\FollowerService;
use Illuminate\Console\Command;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
class FixProfileCounts extends Command
{
@ -22,7 +20,8 @@ class FixProfileCounts extends Command
{id? : Profile id or username to resync (omit with --all)}
{--all : Scan all profiles and resync any with drifted counts}
{--dispatch : Queue FollowServiceWarmCache for follower/following instead of recomputing inline}
{--dry-run : Report drift without changing anything}';
{--dry-run : Report drift without changing anything}
{--force : Skip the confirmation prompt (for scheduled/unattended runs)}';
/**
* The console command description.
@ -31,13 +30,6 @@ class FixProfileCounts extends Command
*/
protected $description = 'Resync a profile\'s cached counts (followers, following, statuses) from source-of-truth tables';
/**
* Visible scopes counted for status_count (matches profile status semantics).
*
* @var array<int, string>
*/
protected const STATUS_SCOPES = ['public', 'private', 'unlisted'];
/**
* Execute the console command.
*
@ -78,7 +70,7 @@ class FixProfileCounts extends Command
// --all: scan every profile, only touch/report the ones that drifted.
$dryRun = $this->option('dry-run');
if (! $dryRun && ! $this->confirm('Resync cached counts for all drifted profiles?', true)) {
if (! $dryRun && ! $this->option('force') && ! $this->confirm('Resync cached counts for all drifted profiles?', true)) {
$this->comment('Aborted.');
return 0;
@ -107,17 +99,24 @@ class FixProfileCounts extends Command
*/
protected function resyncOne(Profile $profile): bool
{
$liveFollowers = (int) Follower::whereFollowingId($profile->id)->count();
$liveFollowing = (int) Follower::whereProfileId($profile->id)->count();
$liveStatuses = (int) $this->liveStatusCount($profile);
$cachedFollowers = (int) $profile->followers_count;
$cachedFollowing = (int) $profile->following_count;
$cachedStatuses = (int) $profile->status_count;
$followersDrift = $liveFollowers !== $cachedFollowers;
$followingDrift = $liveFollowing !== $cachedFollowing;
$statusesDrift = $liveStatuses !== $cachedStatuses;
// Compute drift for all three metrics using the canonical
// source-of-truth logic (shared with the scheduled updater).
$followers = [
'cached' => (int) $profile->followers_count,
'live' => AccountStatService::recalculateFollowerCount($profile->id),
];
$following = [
'cached' => (int) $profile->following_count,
'live' => AccountStatService::recalculateFollowingCount($profile->id),
];
$statuses = [
'cached' => (int) $profile->status_count,
'live' => AccountStatService::recalculateStatusCount($profile->id),
];
$followersDrift = $followers['cached'] !== $followers['live'];
$followingDrift = $following['cached'] !== $following['live'];
$statusesDrift = $statuses['cached'] !== $statuses['live'];
if (! $followersDrift && ! $followingDrift && ! $statusesDrift) {
// No drift: stay silent.
@ -127,67 +126,36 @@ class FixProfileCounts extends Command
// Drift detected: report exactly what drifted.
$this->warn($profile->username.' (id '.$profile->id.') drift detected:');
if ($followersDrift) {
$this->line(' followers: cached='.$cachedFollowers.' live='.$liveFollowers);
$this->line(' followers: cached='.$followers['cached'].' live='.$followers['live']);
}
if ($followingDrift) {
$this->line(' following: cached='.$cachedFollowing.' live='.$liveFollowing);
$this->line(' following: cached='.$following['cached'].' live='.$following['live']);
}
if ($statusesDrift) {
$this->line(' statuses: cached='.$cachedStatuses.' live='.$liveStatuses);
$this->line(' statuses: cached='.$statuses['cached'].' live='.$statuses['live']);
}
if ($this->option('dry-run')) {
return true;
}
// status_count is always recomputed inline (no queue involved).
if ($statusesDrift) {
$profile->status_count = $liveStatuses;
}
if ($this->option('dispatch') && ($followersDrift || $followingDrift)) {
// Persist any status fix first, then let the warm-cache job own
// the follower/following columns and rebuild the Redis sets.
if ($statusesDrift) {
$profile->save();
Cache::forget('profile:status_count:'.$profile->id);
}
// Fix status_count now (via the shared reconciler), then let the
// warm-cache job own the follower/following columns and rebuild
// the Redis sets.
AccountStatService::reconcileProfileCounts($profile, ['statuses']);
Cache::forget(FollowerService::FOLLOWERS_SYNC_KEY.$profile->id);
Cache::forget(FollowerService::FOLLOWING_SYNC_KEY.$profile->id);
FollowServiceWarmCache::dispatch($profile->id)->onQueue('low');
$this->info(' queued FollowServiceWarmCache for profile '.$profile->id.'; statuses='.$profile->status_count.'.');
$this->info(' queued FollowServiceWarmCache for profile '.$profile->id.'; statuses='.$statuses['live'].'.');
return true;
}
// Inline recompute of the drifted columns from source-of-truth tables.
if ($followersDrift) {
$profile->followers_count = (int) DB::table('followers')->whereFollowingId($profile->id)->count();
}
if ($followingDrift) {
$profile->following_count = (int) DB::table('followers')->whereProfileId($profile->id)->count();
}
$profile->save();
// Bust the derived caches so reads reflect the corrected values.
Cache::forget('profile:follower_count:'.$profile->id);
Cache::forget('profile:following_count:'.$profile->id);
Cache::forget('profile:status_count:'.$profile->id);
AccountService::del($profile->id);
// Inline recompute of all drifted columns via the shared reconciler.
AccountStatService::reconcileProfileCounts($profile);
$this->info(' resynced to followers='.$profile->followers_count.', following='.$profile->following_count.', statuses='.$profile->status_count.'.');
return true;
}
/**
* Source-of-truth status count for a profile.
*/
protected function liveStatusCount(Profile $profile): int
{
return $profile->statuses()
->getQuery()
->whereIn('scope', self::STATUS_SCOPES)
->count();
}
}

@ -2,12 +2,126 @@
namespace App\Services\Account;
use App\Models\Follower;
use App\Models\Profile;
use App\Models\Status;
use App\Services\AccountService;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Redis;
class AccountStatService
{
const REFRESH_CACHE_KEY = 'pf:services:accountstats:refresh:daily';
/**
* Status types that count toward a profile's status_count.
*
* Must mirror the increment/decrement logic in StatusEntityLexer and
* StatusDelete, which only adjust status_count for media post types.
*
* @var array<int, string>
*/
const COUNTABLE_STATUS_TYPES = [
'photo',
'photo:album',
'video',
'video:album',
'photo:video:album',
];
/**
* Canonical source-of-truth status_count for a profile.
*/
public static function recalculateStatusCount($pid): int
{
return (int) Status::whereProfileId($pid)
->whereIn('type', self::COUNTABLE_STATUS_TYPES)
->count();
}
/**
* Canonical source-of-truth follower count for a profile.
*/
public static function recalculateFollowerCount($pid): int
{
return (int) Follower::whereFollowingId($pid)->count();
}
/**
* Canonical source-of-truth following count for a profile.
*/
public static function recalculateFollowingCount($pid): int
{
return (int) Follower::whereProfileId($pid)->count();
}
/**
* Reconcile a profile's cached count columns against source-of-truth
* tables. Only writes/clears caches for columns that actually drifted.
*
* @param array<int, string> $only Restrict to a subset of
* ['statuses','followers','following'].
* @return array<string, array{cached:int,live:int,drifted:bool}>
* Per-metric before/after summary.
*/
public static function reconcileProfileCounts($profile, array $only = ['statuses', 'followers', 'following']): array
{
if (! $profile instanceof Profile) {
$profile = Profile::find($profile);
}
if (! $profile) {
return [];
}
$summary = [];
$changed = false;
if (in_array('statuses', $only, true)) {
$cached = (int) $profile->status_count;
$live = self::recalculateStatusCount($profile->id);
$drift = $cached !== $live;
if ($drift) {
$profile->status_count = $live;
$changed = true;
}
$summary['statuses'] = ['cached' => $cached, 'live' => $live, 'drifted' => $drift];
}
if (in_array('followers', $only, true)) {
$cached = (int) $profile->followers_count;
$live = self::recalculateFollowerCount($profile->id);
$drift = $cached !== $live;
if ($drift) {
$profile->followers_count = $live;
$changed = true;
}
$summary['followers'] = ['cached' => $cached, 'live' => $live, 'drifted' => $drift];
}
if (in_array('following', $only, true)) {
$cached = (int) $profile->following_count;
$live = self::recalculateFollowingCount($profile->id);
$drift = $cached !== $live;
if ($drift) {
$profile->following_count = $live;
$changed = true;
}
$summary['following'] = ['cached' => $cached, 'live' => $live, 'drifted' => $drift];
}
if ($changed) {
$profile->save();
Cache::forget('profile:status_count:'.$profile->id);
Cache::forget('profile:follower_count:'.$profile->id);
Cache::forget('profile:following_count:'.$profile->id);
AccountService::del($profile->id);
}
return $summary;
}
public static function incrementPostCount($pid)
{
return Redis::zadd(self::REFRESH_CACHE_KEY, $pid, $pid);

@ -0,0 +1,162 @@
<?php
use App\Models\Follower;
use App\Models\Status;
use App\Models\User;
use App\Services\Account\AccountStatService;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
uses(LazilyRefreshDatabase::class);
/*
|--------------------------------------------------------------------------
| Profile Count Reconciliation
|--------------------------------------------------------------------------
|
| Tests for the shared AccountStatService recompute helpers used by both the
| scheduled app:account-post-count-stat-update command and fix:profilecounts.
| status_count must mirror the increment logic (media post types only).
|
*/
describe('AccountStatService recompute helpers', function () {
it('counts only media post types toward status_count', function () {
$user = User::factory()->create();
$user->refresh();
$pid = $user->profile->id;
// 2 photos + 1 video = 3 countable; text + reply are NOT counted.
Status::factory()->count(2)->photo()->create(['profile_id' => $pid]);
Status::factory()->video()->create(['profile_id' => $pid]);
Status::factory()->create(['profile_id' => $pid, 'type' => 'text']);
Status::factory()->reply()->create(['profile_id' => $pid]);
expect(AccountStatService::recalculateStatusCount($pid))->toBe(3);
});
it('counts followers and following from the followers table', function () {
$a = User::factory()->create();
$a->refresh();
$b = User::factory()->create();
$b->refresh();
$c = User::factory()->create();
$c->refresh();
// b and c follow a; a follows c.
Follower::create(['profile_id' => $b->profile->id, 'following_id' => $a->profile->id]);
Follower::create(['profile_id' => $c->profile->id, 'following_id' => $a->profile->id]);
Follower::create(['profile_id' => $a->profile->id, 'following_id' => $c->profile->id]);
expect(AccountStatService::recalculateFollowerCount($a->profile->id))->toBe(2);
expect(AccountStatService::recalculateFollowingCount($a->profile->id))->toBe(1);
});
});
describe('AccountStatService::reconcileProfileCounts', function () {
it('fixes all drifted columns and reports the summary', function () {
$user = User::factory()->create();
$user->refresh();
$profile = $user->profile;
Status::factory()->count(2)->photo()->create(['profile_id' => $profile->id]);
// Deliberately set wrong cached values.
$profile->status_count = 99;
$profile->followers_count = 42;
$profile->following_count = 7;
$profile->save();
$summary = AccountStatService::reconcileProfileCounts($profile->fresh());
expect($summary['statuses']['drifted'])->toBeTrue();
expect($summary['statuses']['live'])->toBe(2);
expect($summary['followers']['live'])->toBe(0);
expect($summary['following']['live'])->toBe(0);
$profile->refresh();
expect((int) $profile->status_count)->toBe(2);
expect((int) $profile->followers_count)->toBe(0);
expect((int) $profile->following_count)->toBe(0);
});
it('reports no drift and writes nothing when counts are correct', function () {
$user = User::factory()->create();
$user->refresh();
$profile = $user->profile;
Status::factory()->photo()->create(['profile_id' => $profile->id]);
$profile->status_count = 1;
$profile->followers_count = 0;
$profile->following_count = 0;
$profile->save();
$updatedAt = $profile->fresh()->updated_at;
$summary = AccountStatService::reconcileProfileCounts($profile->fresh());
expect($summary['statuses']['drifted'])->toBeFalse();
expect($summary['followers']['drifted'])->toBeFalse();
expect($summary['following']['drifted'])->toBeFalse();
// No write should have occurred (updated_at unchanged).
expect($profile->fresh()->updated_at->eq($updatedAt))->toBeTrue();
});
it('restricts reconciliation to the requested metrics only', function () {
$user = User::factory()->create();
$user->refresh();
$profile = $user->profile;
Status::factory()->count(3)->photo()->create(['profile_id' => $profile->id]);
$profile->status_count = 0; // drifted, should be fixed
$profile->followers_count = 50; // drifted, but must be left alone
$profile->save();
$summary = AccountStatService::reconcileProfileCounts($profile->fresh(), ['statuses']);
expect($summary)->toHaveKey('statuses');
expect($summary)->not->toHaveKey('followers');
$profile->refresh();
expect((int) $profile->status_count)->toBe(3);
expect((int) $profile->followers_count)->toBe(50);
});
it('returns an empty summary for a missing profile id', function () {
expect(AccountStatService::reconcileProfileCounts(999999999999))->toBe([]);
});
});
describe('fix:profilecounts command', function () {
it('is silent for an in-sync profile and reports drift otherwise', function () {
$user = User::factory()->create();
$user->refresh();
$profile = $user->profile;
Status::factory()->count(2)->photo()->create(['profile_id' => $profile->id]);
$profile->status_count = 5; // drift
$profile->save();
$this->artisan('fix:profilecounts', ['id' => (string) $profile->id])
->expectsOutputToContain('drift detected')
->assertExitCode(0);
// Now in sync -> no drift output.
$this->artisan('fix:profilecounts', ['id' => (string) $profile->id])
->doesntExpectOutputToContain('drift detected')
->assertExitCode(0);
});
it('does not modify anything in dry-run mode', function () {
$user = User::factory()->create();
$user->refresh();
$profile = $user->profile;
Status::factory()->photo()->create(['profile_id' => $profile->id]);
$profile->status_count = 88;
$profile->save();
$this->artisan('fix:profilecounts', ['id' => (string) $profile->id, '--dry-run' => true])
->assertExitCode(0);
expect((int) $profile->fresh()->status_count)->toBe(88);
});
});
Loading…
Cancel
Save