From 8086a9647b4e99aba0ecda08c3f5ac1aab45321a Mon Sep 17 00:00:00 2001 From: HomeOS Bootstrap Date: Sat, 18 Jul 2026 02:01:45 +0200 Subject: [PATCH] fix(addons): atomic Ring device creation + invalidate ingest gate cache (R15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex R15 on the Ring bridge flagged two issues: - [P1] Concurrent first-sight messages (retained info + motion on bridge startup) could create duplicate devices — no uniqueness on ring_id. Added a partial unique index on (config->>'ring_id'); resolveDevice now creates and, on the unique violation, re-fetches the winner (race-safe). - [P2] The install-gate cache was never invalidated, so ingest was dropped for up to 15s after install and devices kept being created for up to 15s after uninstall. AddonService now forgets the shared cache key on install/uninstall. +3 tests (unique-index guard, repeated-messages-reuse-device, cache invalidation). Suite 43 green. Co-Authored-By: Claude Opus 4.8 --- app/Jobs/IngestRingMessage.php | 17 +++++++++++++--- app/Services/AddonService.php | 13 ++++++++++++ ...01_add_ring_id_unique_index_to_devices.php | 20 +++++++++++++++++++ tests/Feature/AddonTest.php | 17 ++++++++++++++++ tests/Feature/RingIngestTest.php | 20 +++++++++++++++++++ 5 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 database/migrations/2026_07_18_060001_add_ring_id_unique_index_to_devices.php diff --git a/app/Jobs/IngestRingMessage.php b/app/Jobs/IngestRingMessage.php index 4ade85e..20dfadf 100644 --- a/app/Jobs/IngestRingMessage.php +++ b/app/Jobs/IngestRingMessage.php @@ -10,6 +10,7 @@ use App\Services\AddonService; use App\Support\Mqtt\RingNormalizer; use App\Support\Mqtt\RingTopics; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; @@ -87,8 +88,13 @@ class IngestRingMessage implements ShouldQueue private function resolveDevice(string $ringId, string $category): Device { - return Device::where('config->ring_id', $ringId)->first() - ?? Device::create([ + $existing = Device::where('config->ring_id', $ringId)->first(); + if ($existing !== null) { + return $existing; + } + + try { + return Device::create([ 'name' => Str::headline($category).' · '.Str::upper(Str::substr($ringId, -4)), 'vendor' => 'Ring', 'protocol' => 'mqtt', @@ -96,10 +102,15 @@ class IngestRingMessage implements ShouldQueue 'last_seen_at' => now(), 'config' => ['cloud' => true, 'ring_id' => $ringId, 'ring_category' => $category], ]); + } catch (UniqueConstraintViolationException) { + // A concurrent worker created it first (devices_ring_id_unique) — use the winner. + return Device::where('config->ring_id', $ringId)->firstOrFail(); + } } private function ringInstalled(): bool { - return Cache::remember('addon:ring:installed', 15, fn () => Addon::where('key', 'ring')->where('installed', true)->exists()); + return Cache::remember(AddonService::installedCacheKey('ring'), 15, + fn () => Addon::where('key', 'ring')->where('installed', true)->exists()); } } diff --git a/app/Services/AddonService.php b/app/Services/AddonService.php index 8867f70..4ae5436 100644 --- a/app/Services/AddonService.php +++ b/app/Services/AddonService.php @@ -3,6 +3,7 @@ namespace App\Services; use App\Models\Addon; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; /** @@ -12,6 +13,12 @@ use Illuminate\Support\Facades\DB; */ class AddonService { + /** Cache key the ingest gate reads to avoid a DB hit per message. */ + public static function installedCacheKey(string $key): string + { + return "addon:{$key}:installed"; + } + public function row(string $key): Addon { return Addon::firstOrCreate(['key' => $key]); @@ -26,6 +33,10 @@ class AddonService $addon = $this->row($key); $addon->forceFill(['installed' => true])->save(); + // Invalidate the ingest gate immediately — otherwise messages are dropped for up to the + // cache TTL right after install (and, on uninstall below, devices keep being created). + Cache::forget(self::installedCacheKey($key)); + return $addon; } @@ -41,6 +52,8 @@ class AddonService 'config' => null, 'connected_at' => null, ])->save(); + + Cache::forget(self::installedCacheKey($key)); } /** diff --git a/database/migrations/2026_07_18_060001_add_ring_id_unique_index_to_devices.php b/database/migrations/2026_07_18_060001_add_ring_id_unique_index_to_devices.php new file mode 100644 index 0000000..3c62149 --- /dev/null +++ b/database/migrations/2026_07_18_060001_add_ring_id_unique_index_to_devices.php @@ -0,0 +1,20 @@ +>'ring_id')) WHERE config->>'ring_id' IS NOT NULL"); + } + + public function down(): void + { + DB::statement('DROP INDEX IF EXISTS devices_ring_id_unique'); + } +}; diff --git a/tests/Feature/AddonTest.php b/tests/Feature/AddonTest.php index 0fda370..7f4b067 100644 --- a/tests/Feature/AddonTest.php +++ b/tests/Feature/AddonTest.php @@ -6,6 +6,7 @@ use App\Models\Addon; use App\Services\AddonRegistry; use App\Services\AddonService; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; use Tests\TestCase; @@ -78,6 +79,22 @@ class AddonTest extends TestCase $this->assertNull($addon->config); } + public function test_install_and_uninstall_invalidate_the_ingest_gate_cache(): void + { + $key = AddonService::installedCacheKey('ring'); + $service = app(AddonService::class); + + // Prime a stale "not installed" value, then install → cache must be cleared. + Cache::put($key, false, 60); + $service->install('ring'); + $this->assertFalse(Cache::has($key)); + + // Prime a stale "installed" value, then uninstall → cache must be cleared. + Cache::put($key, true, 60); + $service->uninstall('ring'); + $this->assertFalse(Cache::has($key)); + } + public function test_mark_status_only_applies_to_installed_addons(): void { $service = app(AddonService::class); diff --git a/tests/Feature/RingIngestTest.php b/tests/Feature/RingIngestTest.php index af2d0da..8adfcb6 100644 --- a/tests/Feature/RingIngestTest.php +++ b/tests/Feature/RingIngestTest.php @@ -84,6 +84,26 @@ class RingIngestTest extends TestCase $this->assertDatabaseCount('devices', 0); } + public function test_ring_id_is_unique_so_concurrent_creates_cannot_duplicate(): void + { + Device::create(['name' => 'A', 'vendor' => 'Ring', 'protocol' => 'mqtt', 'config' => ['ring_id' => 'dup1']]); + + $this->expectException(\Illuminate\Database\UniqueConstraintViolationException::class); + Device::create(['name' => 'B', 'vendor' => 'Ring', 'protocol' => 'mqtt', 'config' => ['ring_id' => 'dup1']]); + } + + public function test_repeated_messages_reuse_the_same_device(): void + { + $this->installRing(); + + (new IngestRingMessage('ring/loc1/camera/cam9/ding/state', 'ON'))->handle(); + (new IngestRingMessage('ring/loc1/camera/cam9/motion/state', 'ON'))->handle(); + (new IngestRingMessage('ring/loc1/camera/cam9/info/state', json_encode(['batteryLevel' => 50])))->handle(); + + $this->assertSame(1, Device::where('config->ring_id', 'cam9')->count()); + $this->assertSame(3, Device::where('config->ring_id', 'cam9')->first()->entities()->count()); + } + public function test_out_of_order_messages_do_not_overwrite_newer_state(): void { $this->installRing();