diff --git a/app/Console/Commands/AccountPostCountStatUpdate.php b/app/Console/Commands/AccountPostCountStatUpdate.php index 060664f61..e0d52dd15 100644 --- a/app/Console/Commands/AccountPostCountStatUpdate.php +++ b/app/Console/Commands/AccountPostCountStatUpdate.php @@ -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); } } diff --git a/app/Console/Commands/FixProfileCounts.php b/app/Console/Commands/FixProfileCounts.php index a82e6c04b..9cc583d17 100644 --- a/app/Console/Commands/FixProfileCounts.php +++ b/app/Console/Commands/FixProfileCounts.php @@ -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 - */ - 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(); - } } diff --git a/app/Services/Account/AccountStatService.php b/app/Services/Account/AccountStatService.php index 4cb794659..7d04e613d 100644 --- a/app/Services/Account/AccountStatService.php +++ b/app/Services/Account/AccountStatService.php @@ -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 + */ + 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 $only Restrict to a subset of + * ['statuses','followers','following']. + * @return array + * 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); diff --git a/tests/Feature/Account/ProfileCountReconcileTest.php b/tests/Feature/Account/ProfileCountReconcileTest.php new file mode 100644 index 000000000..598ac7976 --- /dev/null +++ b/tests/Feature/Account/ProfileCountReconcileTest.php @@ -0,0 +1,162 @@ +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); + }); +});