diff --git a/app/Console/Commands/FlushLinkClickCountersCommand.php b/app/Console/Commands/FlushLinkClickCountersCommand.php new file mode 100644 index 0000000..fe4c39c --- /dev/null +++ b/app/Console/Commands/FlushLinkClickCountersCommand.php @@ -0,0 +1,34 @@ +option('recount')) { + $links = $buffer->recount(); + $this->info("Recounted totals for {$links} link(s) from link_clicks."); + + return self::SUCCESS; + } + + $result = $buffer->flush(); + $this->info(sprintf( + 'Flushed %d click(s) across %d link(s) and %d wallet(s).', + $result['clicks'], + $result['links'], + $result['wallets'], + )); + + return self::SUCCESS; + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index c42cb6a..84d318d 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -10,7 +10,18 @@ class AppServiceProvider extends ServiceProvider { public function register(): void { - // + /* + | Click counters are buffered rather than written on the request path, so a + | viral link does not serialise its traffic through one row lock. Redis in + | production; the array store keeps tests and Redis-less local setups working + | (it is process-local, so config must never select it on a real deployment). + */ + $this->app->bind( + \App\Services\Link\ClickCounterStore::class, + fn () => config('link.click_counter_store', 'redis') === 'array' + ? new \App\Services\Link\ArrayClickCounterStore + : new \App\Services\Link\RedisClickCounterStore, + ); } public function boot(): void diff --git a/app/Services/Link/ArrayClickCounterStore.php b/app/Services/Link/ArrayClickCounterStore.php new file mode 100644 index 0000000..549344d --- /dev/null +++ b/app/Services/Link/ArrayClickCounterStore.php @@ -0,0 +1,32 @@ +> */ + private array $hashes = []; + + public function increment(string $hash, string $field, int $by = 1): void + { + $this->hashes[$hash][$field] = (string) (((int) ($this->hashes[$hash][$field] ?? 0)) + $by); + } + + public function put(string $hash, string $field, string $value): void + { + $this->hashes[$hash][$field] = $value; + } + + /** @return array */ + public function drain(string $hash): array + { + $out = $this->hashes[$hash] ?? []; + unset($this->hashes[$hash]); + + return $out; + } +} diff --git a/app/Services/Link/ClickCounterStore.php b/app/Services/Link/ClickCounterStore.php new file mode 100644 index 0000000..5a03ce3 --- /dev/null +++ b/app/Services/Link/ClickCounterStore.php @@ -0,0 +1,26 @@ + + */ + public function drain(string $hash): array; +} diff --git a/app/Services/Link/LinkClickCounterBuffer.php b/app/Services/Link/LinkClickCounterBuffer.php new file mode 100644 index 0000000..2967d83 --- /dev/null +++ b/app/Services/Link/LinkClickCounterBuffer.php @@ -0,0 +1,185 @@ +store->increment(self::CLICKS, (string) $link->id); + if ($isUnique) { + $this->store->increment(self::UNIQUE, (string) $link->id); + } + $this->store->put(self::LAST_CLICKED, (string) $link->id, now()->toDateTimeString()); + $this->store->increment(self::WALLET, (string) $link->user_id); + } catch (Throwable $e) { + // Redis down: fall back to the direct writes rather than lose the count. + // This reintroduces the contention, but only while Redis is unavailable. + Log::warning('link click counter buffer unavailable, writing through', [ + 'short_link_id' => $link->id, + 'error' => $e->getMessage(), + ]); + + $this->writeThrough($link, $isUnique); + } + } + + /** + * Drain the buffer into the database. + * + * Each hash is read and deleted in one transaction so a concurrent click either + * lands in the batch being flushed or in the next one, never in neither. + * + * @return array{links: int, wallets: int, clicks: int} + */ + public function flush(): array + { + [$clicks, $unique, $lastClicked, $wallets] = $this->drain(); + + $totalClicks = 0; + $linkIds = array_unique([...array_keys($clicks), ...array_keys($unique), ...array_keys($lastClicked)]); + + foreach ($linkIds as $linkId) { + $delta = (int) ($clicks[$linkId] ?? 0); + $uniqueDelta = (int) ($unique[$linkId] ?? 0); + $stamp = $lastClicked[$linkId] ?? null; + $totalClicks += $delta; + + // One UPDATE per link per flush, replacing three per click. + $sets = []; + if ($delta !== 0) { + $sets['clicks_total'] = DB::raw('clicks_total + '.$delta); + } + if ($uniqueDelta !== 0) { + $sets['unique_clicks_total'] = DB::raw('unique_clicks_total + '.$uniqueDelta); + } + if ($stamp !== null) { + $sets['last_clicked_at'] = $stamp; + } + if ($sets !== []) { + ShortLink::query()->whereKey($linkId)->update($sets); + } + } + + foreach ($wallets as $userId => $delta) { + if ((int) $delta !== 0) { + LinkWallet::query()->where('user_id', $userId)->increment('clicks_total', (int) $delta); + } + } + + return [ + 'links' => count($linkIds), + 'wallets' => count($wallets), + 'clicks' => $totalClicks, + ]; + } + + /** + * Rebuild the denormalised totals from link_clicks, which is insert-only and + * therefore authoritative. Use after a Redis loss, or to verify drift. + */ + public function recount(): int + { + $rows = DB::table('link_clicks') + ->select('short_link_id') + ->selectRaw('COUNT(*) as total') + ->selectRaw('SUM(CASE WHEN is_unique = 1 THEN 1 ELSE 0 END) as uniques') + ->selectRaw('MAX(clicked_at) as last_clicked') + ->groupBy('short_link_id') + ->get(); + + foreach ($rows as $row) { + ShortLink::query()->whereKey($row->short_link_id)->update([ + 'clicks_total' => (int) $row->total, + 'unique_clicks_total' => (int) $row->uniques, + 'last_clicked_at' => $row->last_clicked, + ]); + } + + return $rows->count(); + } + + /** + * Read-and-clear each hash atomically. + * + * @return array{0: array, 1: array, 2: array, 3: array} + */ + private function drain(): array + { + $out = []; + + foreach ([self::CLICKS, self::UNIQUE, self::LAST_CLICKED, self::WALLET] as $key) { + try { + $out[] = $this->store->drain($key); + } catch (Throwable $e) { + Log::warning('link click counter flush could not drain', [ + 'key' => $key, + 'error' => $e->getMessage(), + ]); + $out[] = []; + } + } + + return $out; + } + + private function writeThrough(ShortLink $link, bool $isUnique): void + { + try { + // Single statement, so the row is locked once rather than three times. + $sets = [ + 'clicks_total' => DB::raw('clicks_total + 1'), + 'last_clicked_at' => now(), + ]; + if ($isUnique) { + $sets['unique_clicks_total'] = DB::raw('unique_clicks_total + 1'); + } + ShortLink::query()->whereKey($link->id)->update($sets); + + LinkWallet::query()->where('user_id', $link->user_id)->increment('clicks_total'); + } catch (Throwable $e) { + // Never let a counter failure break the redirect the visitor came for. + Log::warning('link click counter write-through failed', [ + 'short_link_id' => $link->id, + 'error' => $e->getMessage(), + ]); + } + } +} diff --git a/app/Services/Link/LinkClickRecorder.php b/app/Services/Link/LinkClickRecorder.php index 8765be0..a663650 100644 --- a/app/Services/Link/LinkClickRecorder.php +++ b/app/Services/Link/LinkClickRecorder.php @@ -3,28 +3,35 @@ namespace App\Services\Link; use App\Models\LinkClick; -use App\Models\LinkWallet; use App\Models\ShortLink; use App\Support\Link\LinkGeoResolver; use App\Support\Link\LinkUserAgentParser; use Illuminate\Http\Request; -use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; +use Throwable; class LinkClickRecorder { + public function __construct(private readonly LinkClickCounterBuffer $counters) {} + + /** + * Record one click. + * + * link_clicks is the source of truth and is insert-only, so it carries no lock + * contention no matter how popular a link becomes. The denormalised totals on + * short_links / link_wallets are buffered in Redis and flushed on a schedule — + * see LinkClickCounterBuffer for why they can no longer be written inline. + * + * Nothing here may break the redirect: a visitor who clicked a link must be sent + * on their way even if analytics fails entirely. + */ public function record(ShortLink $link, Request $request): void { - $ipHash = hash('sha256', $request->ip().'|'.config('app.key')); - $windowHours = (int) config('link.click_unique_window_hours', 24); - $isUnique = ! LinkClick::query() - ->where('short_link_id', $link->id) - ->where('ip_hash', $ipHash) - ->where('clicked_at', '>=', now()->subHours($windowHours)) - ->exists(); + try { + $ipHash = hash('sha256', $request->ip().'|'.config('app.key')); + $isUnique = $this->isUnique($link, $ipHash); + $parsed = LinkUserAgentParser::parse((string) $request->userAgent()); - $parsed = LinkUserAgentParser::parse((string) $request->userAgent()); - - DB::transaction(function () use ($link, $request, $ipHash, $isUnique, $parsed) { LinkClick::create([ 'short_link_id' => $link->id, 'ip_hash' => $ipHash, @@ -38,16 +45,31 @@ class LinkClickRecorder 'clicked_at' => now(), ]); - $link->increment('clicks_total'); - if ($isUnique) { - $link->increment('unique_clicks_total'); - } - $link->update(['last_clicked_at' => now()]); + $this->counters->add($link, $isUnique); + } catch (Throwable $e) { + Log::warning('link click recording failed', [ + 'short_link_id' => $link->id, + 'error' => $e->getMessage(), + ]); + } + } - LinkWallet::query() - ->where('user_id', $link->user_id) - ->increment('clicks_total'); - }); + /** + * First click from this IP within the window? + * + * Relies on the (short_link_id, ip_hash, clicked_at) index. Without it this is a + * scan over every click the link has ever received, which degrades as the link + * gets popular — the opposite of what a viral link needs. + */ + private function isUnique(ShortLink $link, string $ipHash): bool + { + $windowHours = (int) config('link.click_unique_window_hours', 24); + + return ! LinkClick::query() + ->where('short_link_id', $link->id) + ->where('ip_hash', $ipHash) + ->where('clicked_at', '>=', now()->subHours($windowHours)) + ->exists(); } private function truncate(?string $value, int $max): ?string diff --git a/app/Services/Link/RedisClickCounterStore.php b/app/Services/Link/RedisClickCounterStore.php new file mode 100644 index 0000000..86b28ab --- /dev/null +++ b/app/Services/Link/RedisClickCounterStore.php @@ -0,0 +1,31 @@ + */ + public function drain(string $hash): array + { + // MULTI/EXEC: the DEL cannot land between another client's HINCRBY and our + // HGETALL, so no increment is read-then-discarded. + $result = Redis::transaction(function ($tx) use ($hash) { + $tx->hgetall($hash); + $tx->del($hash); + }); + + return is_array($result[0] ?? null) ? $result[0] : []; + } +} diff --git a/config/link.php b/config/link.php index 28feb71..b77c239 100644 --- a/config/link.php +++ b/config/link.php @@ -4,6 +4,12 @@ return [ 'price_per_link_ghs' => (float) env('LINK_PRICE_PER_LINK_GHS', 0.05), 'slug_length' => (int) env('LINK_SLUG_LENGTH', 8), 'click_unique_window_hours' => (int) env('LINK_CLICK_UNIQUE_WINDOW_HOURS', 24), + + /* + | Where buffered click counters live between flushes. 'redis' in production; + | 'array' is process-local and only appropriate for tests / Redis-less dev. + */ + 'click_counter_store' => env('LINK_CLICK_COUNTER_STORE', 'redis'), 'public_domain' => env('LINK_PUBLIC_DOMAIN', 'ladl.link'), 'geoip_database' => env('LINK_GEOIP_DATABASE', storage_path('app/geoip/GeoLite2-Country.mmdb')), 'maxmind_account_id' => env('MAXMIND_ACCOUNT_ID'), diff --git a/database/migrations/2026_07_25_140000_add_ip_hash_index_to_link_clicks.php b/database/migrations/2026_07_25_140000_add_ip_hash_index_to_link_clicks.php new file mode 100644 index 0000000..6bfba37 --- /dev/null +++ b/database/migrations/2026_07_25_140000_add_ip_hash_index_to_link_clicks.php @@ -0,0 +1,30 @@ +index(['short_link_id', 'ip_hash', 'clicked_at'], 'link_clicks_dedupe_idx'); + }); + } + + public function down(): void + { + Schema::table('link_clicks', function (Blueprint $table) { + $table->dropIndex('link_clicks_dedupe_idx'); + }); + } +}; diff --git a/routes/console.php b/routes/console.php index 2a7d534..d8a5e44 100644 --- a/routes/console.php +++ b/routes/console.php @@ -16,3 +16,7 @@ Schedule::command('hosting:notify-expiring')->daily()->withoutOverlapping(); Schedule::command('hosting:retry-pending-fulfillment')->everyFifteenMinutes()->withoutOverlapping(); Schedule::command('ssl:renew')->daily()->withoutOverlapping(); Schedule::command('link:geoip-update')->weekly()->withoutOverlapping(); + +// Click counters are buffered in Redis to keep hot links off a shared row lock; +// drain them frequently so analytics stay close to real time. +Schedule::command('link:flush-click-counters')->everyMinute()->withoutOverlapping(); diff --git a/tests/Feature/LinkClickCounterTest.php b/tests/Feature/LinkClickCounterTest.php new file mode 100644 index 0000000..be797bb --- /dev/null +++ b/tests/Feature/LinkClickCounterTest.php @@ -0,0 +1,265 @@ + 'array']); + $this->app->singleton(ClickCounterStore::class, fn () => new ArrayClickCounterStore); + } + + private function user(): User + { + return User::create([ + 'public_id' => (string) Str::uuid(), + 'name' => 'Test', + 'email' => 'test+'.uniqid().'@example.com', + ]); + } + + private function link(User $user, string $slug = 'hot1'): ShortLink + { + LinkWallet::firstOrCreate( + ['user_id' => $user->id], + ['links_total' => 0, 'clicks_total' => 0, 'status' => LinkWallet::STATUS_ACTIVE], + ); + + return ShortLink::create([ + 'user_id' => $user->id, + 'slug' => $slug, + 'source_app' => 'link', + 'source_kind' => 'redirect', + 'is_managed_here' => true, + 'destination_url' => 'https://example.com/target', + 'is_active' => true, + ]); + } + + private function record(ShortLink $link, string $ip = '203.0.113.5'): void + { + $request = Request::create('https://ladl.link/'.$link->slug, 'GET', [], [], [], [ + 'REMOTE_ADDR' => $ip, + 'HTTP_USER_AGENT' => 'Mozilla/5.0 (iPhone; CPU iPhone OS 18_7 like Mac OS X)', + ]); + + app(LinkClickRecorder::class)->record($link, $request); + } + + /* ------------------------------------------------- the deadlock is gone */ + + public function test_recording_a_click_does_not_write_to_the_shared_counter_rows(): void + { + $user = $this->user(); + $link = $this->link($user); + + DB::flushQueryLog(); + DB::enableQueryLog(); + $this->record($link); + $queries = array_column(DB::getQueryLog(), 'query'); + DB::disableQueryLog(); + + // The whole point: no UPDATE against short_links or link_wallets on the + // request path, because that shared row lock is what deadlocked. + $hotWrites = array_values(array_filter( + $queries, + fn (string $q) => (bool) preg_match('/update\s+.?(short_links|link_wallets)/i', $q), + )); + + $this->assertSame([], $hotWrites, "Click path still writes to a shared row:\n".implode("\n", $hotWrites)); + } + + public function test_the_click_itself_is_still_persisted(): void + { + $user = $this->user(); + $link = $this->link($user); + + $this->record($link); + + $this->assertSame(1, LinkClick::where('short_link_id', $link->id)->count()); + $click = LinkClick::where('short_link_id', $link->id)->first(); + $this->assertTrue((bool) $click->is_unique); + $this->assertNotNull($click->ip_hash); + $this->assertSame('mobile', $click->device_type); + } + + public function test_repeat_visits_from_one_ip_count_once_as_unique(): void + { + $user = $this->user(); + $link = $this->link($user); + + $this->record($link, '203.0.113.9'); + $this->record($link, '203.0.113.9'); + $this->record($link, '198.51.100.7'); + + $this->assertSame(3, LinkClick::where('short_link_id', $link->id)->count()); + $this->assertSame(2, LinkClick::where('short_link_id', $link->id)->where('is_unique', true)->count()); + } + + /* ------------------------------------------------------------- flushing */ + + public function test_flush_applies_buffered_counts_to_the_database(): void + { + $user = $this->user(); + $link = $this->link($user); + + $this->record($link, '203.0.113.1'); + $this->record($link, '203.0.113.2'); + $this->record($link, '203.0.113.1'); + + // Buffered, so the denormalised totals have not moved yet. + $this->assertSame(0, (int) $link->fresh()->clicks_total); + + app(LinkClickCounterBuffer::class)->flush(); + + $link->refresh(); + $this->assertSame(3, (int) $link->clicks_total); + $this->assertSame(2, (int) $link->unique_clicks_total); + $this->assertNotNull($link->last_clicked_at); + $this->assertSame(3, (int) LinkWallet::where('user_id', $user->id)->value('clicks_total')); + } + + public function test_flush_is_idempotent_when_the_buffer_is_empty(): void + { + $user = $this->user(); + $link = $this->link($user); + $this->record($link); + + app(LinkClickCounterBuffer::class)->flush(); + $after = (int) $link->fresh()->clicks_total; + + app(LinkClickCounterBuffer::class)->flush(); + + $this->assertSame($after, (int) $link->fresh()->clicks_total, 'A second flush must not double-count.'); + } + + public function test_flush_batches_one_update_per_link_not_one_per_click(): void + { + $user = $this->user(); + $link = $this->link($user); + for ($i = 0; $i < 25; $i++) { + $this->record($link, '203.0.113.'.$i); + } + + DB::flushQueryLog(); + DB::enableQueryLog(); + app(LinkClickCounterBuffer::class)->flush(); + $updates = array_filter( + array_column(DB::getQueryLog(), 'query'), + fn (string $q) => (bool) preg_match('/update\s+.?short_links/i', $q), + ); + DB::disableQueryLog(); + + $this->assertCount(1, $updates, '25 clicks must collapse into a single UPDATE.'); + $this->assertSame(25, (int) $link->fresh()->clicks_total); + } + + public function test_command_flushes_and_reports(): void + { + $user = $this->user(); + $link = $this->link($user); + $this->record($link); + + $this->artisan('link:flush-click-counters') + ->expectsOutputToContain('Flushed 1 click') + ->assertSuccessful(); + + $this->assertSame(1, (int) $link->fresh()->clicks_total); + } + + /* ------------------------------------------------------------ recovery */ + + public function test_recount_rebuilds_totals_from_link_clicks(): void + { + $user = $this->user(); + $link = $this->link($user); + $this->record($link, '203.0.113.1'); + $this->record($link, '203.0.113.2'); + app(LinkClickCounterBuffer::class)->flush(); + + // Simulate drift, e.g. counters lost or double-applied. + $link->update(['clicks_total' => 999, 'unique_clicks_total' => 999]); + + $this->artisan('link:flush-click-counters', ['--recount' => true])->assertSuccessful(); + + $link->refresh(); + $this->assertSame(2, (int) $link->clicks_total); + $this->assertSame(2, (int) $link->unique_clicks_total); + } + + private function breakTheStore(): void + { + $this->app->singleton(ClickCounterStore::class, fn () => new class implements ClickCounterStore + { + public function increment(string $hash, string $field, int $by = 1): void + { + throw new \RuntimeException('redis down'); + } + + public function put(string $hash, string $field, string $value): void + { + throw new \RuntimeException('redis down'); + } + + public function drain(string $hash): array + { + throw new \RuntimeException('redis down'); + } + }); + } + + public function test_a_redis_outage_still_records_the_click(): void + { + $user = $this->user(); + $link = $this->link($user); + $this->breakTheStore(); + + $this->record($link); + + $this->assertSame(1, LinkClick::where('short_link_id', $link->id)->count()); + // Fallback writes straight through, so the total moves immediately. This + // reintroduces the row lock, but only while Redis is unavailable. + $this->assertSame(1, (int) $link->fresh()->clicks_total); + } + + public function test_a_failing_store_never_breaks_the_redirect(): void + { + $user = $this->user(); + $this->link($user, 'direct9'); + $this->breakTheStore(); + + $this->get('https://ladl.link/direct9')->assertRedirect('https://example.com/target'); + } + + public function test_a_failing_flush_does_not_throw(): void + { + $user = $this->user(); + $link = $this->link($user); + $this->record($link); + $this->breakTheStore(); + + $result = app(LinkClickCounterBuffer::class)->flush(); + + $this->assertSame(0, $result['clicks']); + } +}