From c1ae1951413d7835fa2ba7deb57e3f969d255b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Sun, 10 May 2026 22:26:12 +0300 Subject: [PATCH] =?UTF-8?q?fix(jobs):=20RouteSupplierLeadJob=20=E2=80=94?= =?UTF-8?q?=20lockForUpdate=20Project=20+=20recheck=20delivered=5Ftoday=20?= =?UTF-8?q?(Plan=202.5=20#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Закрывает CV.11 audit BLOCKER #2 (Plan 2/5 closure). Проблема: matchEligibleProjects делал SELECT delivered_today < limit БЕЗ lockForUpdate. Между snapshot'ом и createDealCopyForProject (который инкрементит счётчик) — окно для concurrent webhook'а: worker A видит delivered_today=9, limit=10 → OK; createDealCopyForProject → 10. worker B параллельно видит то же 9 → OK; createDealCopyForProject → 11. OVERCOMMIT. Лимит daily_limit_target нарушен, баланс tenant'а списан дважды. Fix: внутри createDealCopyForProject (после lockForUpdate Tenant) — lockForUpdate(Project) + recheck delivered_today >= COALESCE(effective_daily_limit_today, daily_limit_target). Если уже at-limit под блокировкой → return false без charge / counter / deal-row + Log::info('supplier_lead.project_at_limit_skipped') для observability. TDD: новый тест 'rejects deal copy if delivered_today >= limit at lock time' симулирует race через Mockery — мокнутый LeadRouter возвращает project уже at-limit (delivered_today=1, daily_limit_target=1), как будто matchEligibleProjects делал SELECT когда delivered_today=0. Assert: deal НЕ создаётся, counter не растёт, balance не списан. Pest: 549/547 passed (+2 от baseline 547), 1745 assertions, 19s parallel. Larastan + Pint: passed. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/app/Jobs/RouteSupplierLeadJob.php | 24 ++++++ .../Feature/Jobs/RouteSupplierLeadJobTest.php | 75 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/app/app/Jobs/RouteSupplierLeadJob.php b/app/app/Jobs/RouteSupplierLeadJob.php index 8f62e795..c0a96972 100644 --- a/app/app/Jobs/RouteSupplierLeadJob.php +++ b/app/app/Jobs/RouteSupplierLeadJob.php @@ -176,6 +176,30 @@ class RouteSupplierLeadJob implements ShouldQueue ->lockForUpdate() ->firstOrFail(); + // Concurrency recheck: lockForUpdate(Project) + recheck delivered_today + // против лимита под блокировкой. Closes CV.11 audit BLOCKER #2 (Plan 2.5). + // matchEligibleProjects делал SELECT без lock'а — между snapshot'ом и + // этой транзакцией concurrent webhook мог инкрементить счётчик до limit. + // Если лимит уже исчерпан — return false (deal не создаём, баланс не списываем). + /** @var Project $lockedProject */ + $lockedProject = Project::query() + ->whereKey($project->id) + ->lockForUpdate() + ->firstOrFail(); + $effectiveLimit = $lockedProject->effective_daily_limit_today ?? $lockedProject->daily_limit_target; + if ($lockedProject->delivered_today >= $effectiveLimit) { + Log::info('supplier_lead.project_at_limit_skipped', [ + 'supplier_lead_id' => $lead->id, + 'project_id' => $lockedProject->id, + 'tenant_id' => $tenant->id, + 'delivered_today' => $lockedProject->delivered_today, + 'effective_limit' => $effectiveLimit, + ]); + + return false; + } + $project = $lockedProject; + $payload = $lead->raw_payload ?? []; $receivedAt = isset($payload['time']) ? Carbon::createFromTimestamp((int) $payload['time']) diff --git a/app/tests/Feature/Jobs/RouteSupplierLeadJobTest.php b/app/tests/Feature/Jobs/RouteSupplierLeadJobTest.php index 54381d3f..d19b98fc 100644 --- a/app/tests/Feature/Jobs/RouteSupplierLeadJobTest.php +++ b/app/tests/Feature/Jobs/RouteSupplierLeadJobTest.php @@ -13,7 +13,9 @@ use App\Services\LeadRouter; use App\Services\NotificationService; use App\Services\SupplierProjects\SupplierProjectResolver; use Illuminate\Foundation\Testing\DatabaseTransactions; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; +use Mockery as M; uses(DatabaseTransactions::class); @@ -382,3 +384,76 @@ it('handles partial failure: one project throws, others continue routing', funct expect($tenants[0]->fresh()->balance_leads)->toBe(99); expect($tenants[2]->fresh()->balance_leads)->toBe(99); }); + +it('rejects deal copy if delivered_today >= limit at lock time (Plan 2.5 fix #2 race recheck)', function (): void { + // BLOCKER #2 (CV.11 audit): matchEligibleProjects делает SELECT delivered_today < limit + // БЕЗ lockForUpdate. Между snapshot SELECT и createDealCopyForProject (которое + // инкрементит) — окно для concurrent webhook'а: + // worker A видит delivered_today=9, limit=10 → OK; createDealCopyForProject → 10. + // worker B параллельно видит то же 9 → OK; createDealCopyForProject → 11. OVERCOMMIT. + // + // Симуляция: project уже at-limit (delivered_today=1, daily_limit_target=1) к + // моменту createDealCopyForProject — мокнутый LeadRouter возвращает его как eligible + // (так, будто matchEligibleProjects делал SELECT когда delivered_today=0). + // + // Fix #2: внутри createDealCopyForProject под lockForUpdate(Project) — recheck + // delivered_today < COALESCE(effective_daily_limit_today, daily_limit_target). + // Если уже at-limit → return false без charge / counter / deal-row. + $supplier = SupplierProject::factory()->create([ + 'platform' => 'B1', + 'signal_type' => 'site', + 'unique_key' => 'race-recheck.ru', + ]); + $tenant = Tenant::factory()->create(['balance_leads' => 100]); + $project = Project::factory()->create([ + 'tenant_id' => $tenant->id, + 'supplier_b1_project_id' => $supplier->id, + 'signal_type' => 'site', + 'signal_identifier' => 'race-recheck.ru', + 'is_active' => true, + 'daily_limit_target' => 1, + 'effective_daily_limit_today' => null, // COALESCE → daily_limit_target=1 + 'delivered_today' => 1, // ALREADY AT LIMIT (race-window simulation) + 'delivered_in_month' => 5, + ]); + + $vid = 8888; + $lead = SupplierLead::factory()->create([ + 'supplier_project_id' => null, + 'platform' => 'B1', + 'vid' => $vid, + 'phone' => '79991234567', + 'raw_payload' => [ + 'vid' => $vid, + 'project' => 'B1_race-recheck.ru', + 'phone' => '79991234567', + 'time' => now()->getTimestamp(), + ], + ]); + + // Подсунуть LeadRouter mock, который игнорирует filter и возвращает project, + // как будто SELECT'нул его при snapshot delivered_today=0. + $routerMock = M::mock(LeadRouter::class); + $routerMock->shouldReceive('matchEligibleProjects') + ->andReturn(new Collection([$project])); + app()->instance(LeadRouter::class, $routerMock); + + runRouteJob($lead->id); + + $lead->refresh(); + + // После fix #2: deal НЕ создан (recheck под lock увидел limit) → 0 deals. + expect($lead->deals_created_count)->toBe(0); + + // delivered_today остался 1 (НЕ инкрементнулся до 2). + expect($project->fresh()->delivered_today)->toBe(1); + // delivered_in_month НЕ инкрементнулся. + expect($project->fresh()->delivered_in_month)->toBe(5); + + // balance_leads НЕ списан. + expect($tenant->fresh()->balance_leads)->toBe(100); + + // Deal-row не создался. + DB::statement("SET LOCAL app.current_tenant_id = '{$tenant->id}'"); + expect(Deal::query()->where('source_crm_id', $vid)->count())->toBe(0); +});