Self-heal stale storage_used on upload/delete hot path

Make increaseStorageUsed/decrementStorageUsed recalculate from source when
the cached counter is older than STALE_AFTER_HOURS (168h) or never
calculated, so an affected user is corrected the next time they upload or
delete without waiting for the nightly reconciler. Callers save/delete the
media row before calling these, so the from-source recalc already reflects
the change and the incremental delta is skipped on the recalc path.

- Add UserStorageService::STALE_AFTER_HOURS and isStale() helper (no extra
  query: reads the already-loaded model), with defensive Carbon parsing
- Cast users.storage_used_updated_at to datetime so freshness comparisons
  work on a Carbon instance
- Add tests for stale/fresh/never-calculated increase and decrement paths
pull/7175/head
Your Name 2 weeks ago
parent 6496904293
commit 26d3e8bb8e

@ -24,6 +24,7 @@ class User extends Authenticatable implements OAuthenticatable
'email_verified_at' => 'datetime',
'2fa_setup_at' => 'datetime',
'last_active_at' => 'datetime',
'storage_used_updated_at' => 'datetime',
];
}

@ -4,11 +4,42 @@ namespace App\Services;
use App\Models\Media;
use App\Models\User;
use Carbon\Carbon;
class UserStorageService
{
const CACHE_KEY = 'pf:services:user-storage:byId:';
/**
* How long (in hours) a cached storage_used value is trusted for
* incremental add/subtract before the hot path recalculates it from source.
*
* Mirrors the scheduled `user:storage:recalculate --stale=168` reconciler:
* an active user who uploads or deletes will self-heal a stale counter
* without waiting for the nightly job.
*/
const STALE_AFTER_HOURS = 168;
/**
* Whether a user's cached storage_used is too old (or missing) to be
* trusted for an incremental delta and should be recalculated from source.
*/
protected static function isStale(User $user): bool
{
$updatedAt = $user->storage_used_updated_at;
if (! $updatedAt) {
return true;
}
// Be defensive: the value is normally cast to Carbon on the User model,
// but tolerate a raw string if a model is hydrated without casts.
if (! $updatedAt instanceof Carbon) {
$updatedAt = Carbon::parse($updatedAt);
}
return $updatedAt->lt(now()->subHours(self::STALE_AFTER_HOURS));
}
public static function get($id)
{
$user = User::find($id);
@ -51,10 +82,13 @@ class UserStorageService
* stored media, without re-summing the whole media table.
*
* This is the counterpart to decrementStorageUsed and is used on the upload
* path. If storage_used has never been calculated (storage_used_updated_at
* is null) it seeds the counter from source first so the increment is added
* to an accurate base rather than zero. Any drift is corrected by the
* periodic recalculateUpdateStorageUsed reconciler.
* path, AFTER the new media row has been saved.
*
* Self-healing: if the cached counter is missing or stale (see
* STALE_AFTER_HOURS) it recalculates from source instead of trusting the
* incremental value. Because callers save the media row before calling
* this, a from-source recalc already accounts for the new media, so the
* delta is NOT re-applied on the recalc path (that would double count).
*
* @param int $id User id
* @param int $sizeInBytes Size of the added media in bytes
@ -67,13 +101,14 @@ class UserStorageService
return null;
}
// Seed from source if never calculated, so we add to an accurate base.
$base = $user->storage_used_updated_at
? (int) $user->storage_used
: self::calculateStorageUsed($id);
// Stale or uncalculated: recompute from source. The just-saved media is
// already included, so return the source value without adding the delta.
if (self::isStale($user)) {
return self::recalculateUpdateStorageUsed($id);
}
$sizeInKbs = (int) floor(((int) $sizeInBytes) / 1000);
$updatedVal = max(0, $base + $sizeInKbs);
$updatedVal = max(0, (int) $user->storage_used + $sizeInKbs);
$user->storage_used = $updatedVal;
$user->storage_used_updated_at = now();
@ -86,14 +121,14 @@ class UserStorageService
* Decrement a user's cached storage_used by the size (in bytes) of removed
* media, without re-summing the whole media table.
*
* This is the fast path used when media is deleted. Any drift it introduces
* (e.g. double-processed jobs, deletions that bypass this path) is corrected
* by the periodic recalculateUpdateStorageUsed reconciler. The value is
* clamped at zero so it can never go negative.
* This is the fast path used when media is deleted, AFTER the media row has
* been removed. The value is clamped at zero so it can never go negative.
*
* Note: only acts on an already-populated counter. If storage_used has
* never been calculated (storage_used_updated_at is null), it leaves the
* value untouched so the next get()/recalculate computes it from source.
* Self-healing: if the cached counter is missing or stale (see
* STALE_AFTER_HOURS) it recalculates from source instead of trusting the
* incremental value. Because callers delete the media row before calling
* this, a from-source recalc already excludes the removed media, so the
* delta is NOT re-applied on the recalc path (that would over-subtract).
*
* @param int $id User id
* @param int $sizeInBytes Size of the removed media in bytes
@ -106,9 +141,10 @@ class UserStorageService
return null;
}
// Nothing cached yet: let the next full calculation establish the value.
if (! $user->storage_used_updated_at) {
return null;
// Stale or uncalculated: recompute from source. The removed media is
// already excluded, so return the source value without subtracting.
if (self::isStale($user)) {
return self::recalculateUpdateStorageUsed($id);
}
$sizeInKbs = (int) floor(((int) $sizeInBytes) / 1000);

@ -131,23 +131,66 @@ it('increases storage_used by the added media size in KB', function () {
expect((int) $user->storage_used)->toBe(800);
});
it('seeds from actual media when increasing an uncalculated counter', function () {
it('recalculates from source instead of adding when the counter was never calculated', function () {
$user = User::factory()->create();
$user->refresh();
// Fresh user with pre-existing media but no cached counter yet.
// Fresh user with pre-existing media but no cached counter yet. Callers
// save the media row before calling increaseStorageUsed, so an
// uncalculated (stale) counter recomputes from source rather than adding.
expect($user->storage_used_updated_at)->toBeNull();
makeMedia($user, 200000, 1); // 200 KB already on disk
makeMedia($user, 200000, 1); // 200 KB already on disk (the "just-saved" media)
// Add another 500 KB; base should be seeded from the 200 KB of media.
$result = UserStorageService::increaseStorageUsed($user->id, 500000);
$result = UserStorageService::increaseStorageUsed($user->id, 200000);
expect($result)->toBe(700);
// Source already includes the 200 KB row; delta is NOT re-added.
expect($result)->toBe(200);
$user->refresh();
expect((int) $user->storage_used)->toBe(700);
expect((int) $user->storage_used)->toBe(200);
expect($user->storage_used_updated_at)->not->toBeNull();
});
/*
| Self-healing: when the cached counter is older than STALE_AFTER_HOURS, the
| upload path recalculates from source (which already includes the just-saved
| media) instead of trusting a possibly-drifted incremental value (#7169).
*/
it('recalculates from source on increase when the counter is stale', function () {
$user = User::factory()->create();
$user->refresh();
// Wildly inflated counter, last touched well beyond the stale window.
$user->storage_used = 999999;
$user->storage_used_updated_at = now()->subHours(UserStorageService::STALE_AFTER_HOURS + 1);
$user->save();
// Actual media on disk (the just-saved upload) is 300 KB.
makeMedia($user, 300000, 1);
$result = UserStorageService::increaseStorageUsed($user->id, 300000);
expect($result)->toBe(300);
$user->refresh();
expect((int) $user->storage_used)->toBe(300);
});
it('trusts the incremental value on increase when the counter is fresh', function () {
$user = User::factory()->create();
$user->refresh();
// Fresh counter that intentionally disagrees with actual media; the fast
// path must trust it and only add the delta, not recompute.
$user->storage_used = 300;
$user->storage_used_updated_at = now();
$user->save();
makeMedia($user, 999000, 1); // disagrees with the cached 300
$result = UserStorageService::increaseStorageUsed($user->id, 500000);
// 300 + 500 = 800 (incremental), NOT recalculated from the 999 KB media.
expect($result)->toBe(800);
});
it('returns null when increasing a missing user', function () {
expect(UserStorageService::increaseStorageUsed(999999, 500000))->toBeNull();
});
@ -202,19 +245,61 @@ it('clamps decrement at zero and never goes negative', function () {
expect((int) $user->storage_used)->toBe(0);
});
it('skips decrement when the counter was never calculated', function () {
it('recalculates from source on decrement when the counter was never calculated', function () {
$user = User::factory()->create();
$user->refresh();
// Fresh user: storage_used_updated_at is null.
// Fresh user: storage_used_updated_at is null (treated as stale). Callers
// delete the media row before calling decrementStorageUsed, so the remaining
// media is the source of truth.
expect($user->storage_used_updated_at)->toBeNull();
makeMedia($user, 150000, 1); // 150 KB of remaining media
$result = UserStorageService::decrementStorageUsed($user->id, 500000);
// Skipped so a later get()/recalculate establishes the true value.
expect($result)->toBeNull();
// Recomputed from remaining media; delta is NOT subtracted on top.
expect($result)->toBe(150);
$user->refresh();
expect($user->storage_used_updated_at)->toBeNull();
expect((int) $user->storage_used)->toBe(150);
expect($user->storage_used_updated_at)->not->toBeNull();
});
/*
| Self-healing: when the cached counter is older than STALE_AFTER_HOURS, the
| delete path recalculates from source (which already excludes the removed
| media) instead of trusting a possibly-drifted incremental value (#7169).
*/
it('recalculates from source on decrement when the counter is stale', function () {
$user = User::factory()->create();
$user->refresh();
$user->storage_used = 999999;
$user->storage_used_updated_at = now()->subHours(UserStorageService::STALE_AFTER_HOURS + 1);
$user->save();
makeMedia($user, 250000, 1); // 250 KB remaining after the delete
$result = UserStorageService::decrementStorageUsed($user->id, 500000);
expect($result)->toBe(250);
$user->refresh();
expect((int) $user->storage_used)->toBe(250);
});
it('trusts the incremental value on decrement when the counter is fresh', function () {
$user = User::factory()->create();
$user->refresh();
// Fresh counter that disagrees with actual media; fast path must trust it.
$user->storage_used = 800;
$user->storage_used_updated_at = now();
$user->save();
makeMedia($user, 999000, 1); // disagrees with the cached 800
$result = UserStorageService::decrementStorageUsed($user->id, 500000);
// 800 - 500 = 300 (incremental), NOT recalculated from the 999 KB media.
expect($result)->toBe(300);
});
it('returns null when decrementing a missing user', function () {

Loading…
Cancel
Save