From 03127a670bc8c987abb7a93efbd50feff65d9f69 Mon Sep 17 00:00:00 2001 From: ahmido Date: Sun, 15 Feb 2026 20:49:38 +0000 Subject: [PATCH] Spec 096: Ops polish (assignment summaries + dedupe + reconcile tracking + seed DX) (#115) Implements Spec 096 ops polish bundle: - Persist durable OperationRun.summary_counts for assignment fetch/restore (final attempt wins) - Server-side dedupe for assignment jobs (15-minute cooldown + non-canonical skip) - Track ReconcileAdapterRunsJob via workspace-scoped OperationRun + stable failure codes + overlap prevention - Seed DX: ensure seeded tenants use UUID v4 external_id and seed satisfies workspace_id NOT NULL constraints Verification (local / evidence-based): - `vendor/bin/sail artisan test --compact tests/Feature/Operations/AssignmentRunSummaryCountsTest.php tests/Feature/Operations/AssignmentJobDedupeTest.php tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php tests/Feature/Seed/PoliciesSeederExternalIdTest.php` - `vendor/bin/sail bin pint --dirty` Spec artifacts included under `specs/096-ops-polish-assignment-dedupe-system-tracking/` (spec/plan/tasks/checklists). Co-authored-by: Ahmed Darrazi Reviewed-on: https://git.cloudarix.de/ahmido/TenantAtlas/pulls/115 --- app/Jobs/FetchAssignmentsJob.php | 332 ++++++++++++++--- app/Jobs/ReconcileAdapterRunsJob.php | 135 ++++++- app/Jobs/RestoreAssignmentsJob.php | 347 +++++++++++++++++- app/Services/OperationRunService.php | 69 ++++ app/Support/OperationCatalog.php | 2 + .../OpsUx/AssignmentJobFingerprint.php | 139 +++++++ database/seeders/PlatformUserSeeder.php | 8 +- database/seeders/PoliciesSeeder.php | 27 +- .../checklists/requirements.md | 42 +++ .../contracts/openapi.yaml | 10 + .../data-model.md | 49 +++ .../plan.md | 140 +++++++ .../quickstart.md | 56 +++ .../research.md | 71 ++++ .../spec.md | 143 ++++++++ .../tasks.md | 177 +++++++++ tests/Feature/Database/PoliciesSeederTest.php | 3 +- .../Feature/Filament/TenantRbacWizardTest.php | 8 + .../Operations/AssignmentJobDedupeTest.php | 182 +++++++++ .../AssignmentRunSummaryCountsTest.php | 146 ++++++++ .../ReconcileAdapterRunsJobTrackingTest.php | 84 +++++ .../Seed/PoliciesSeederExternalIdTest.php | 29 ++ tests/Unit/RbacOnboardingServiceTest.php | 30 +- 23 files changed, 2146 insertions(+), 83 deletions(-) create mode 100644 app/Support/OpsUx/AssignmentJobFingerprint.php create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/checklists/requirements.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/contracts/openapi.yaml create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/data-model.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/plan.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/quickstart.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/research.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md create mode 100644 specs/096-ops-polish-assignment-dedupe-system-tracking/tasks.md create mode 100644 tests/Feature/Operations/AssignmentJobDedupeTest.php create mode 100644 tests/Feature/Operations/AssignmentRunSummaryCountsTest.php create mode 100644 tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php create mode 100644 tests/Feature/Seed/PoliciesSeederExternalIdTest.php diff --git a/app/Jobs/FetchAssignmentsJob.php b/app/Jobs/FetchAssignmentsJob.php index 72350d6..7323b25 100644 --- a/app/Jobs/FetchAssignmentsJob.php +++ b/app/Jobs/FetchAssignmentsJob.php @@ -2,21 +2,36 @@ namespace App\Jobs; -use App\Jobs\Middleware\TrackOperationRun; use App\Models\BackupItem; use App\Models\OperationRun; +use App\Models\Tenant; +use App\Models\User; use App\Services\AssignmentBackupService; +use App\Services\OperationRunService; +use App\Support\OperationRunOutcome; +use App\Support\OperationRunStatus; +use App\Support\OpsUx\AssignmentJobFingerprint; +use App\Support\OpsUx\RunFailureSanitizer; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; +use RuntimeException; class FetchAssignmentsJob implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + private const string OPERATION_TYPE = 'assignments.fetch'; + + private const int DEDUPE_COOLDOWN_MINUTES = 15; + + private const int EXECUTION_LOCK_TTL_SECONDS = 900; + public ?OperationRun $operationRun = null; /** @@ -42,72 +57,305 @@ public function __construct( $this->operationRun = $operationRun; } - /** - * @return array - */ - public function middleware(): array - { - return [new TrackOperationRun]; + public static function dispatchTracked( + BackupItem $backupItem, + array $policyPayload, + ?User $initiator = null, + ): OperationRun { + $tenant = $backupItem->tenant; + + if (! $tenant instanceof Tenant) { + throw new RuntimeException('BackupItem tenant context is required to dispatch assignment fetch job.'); + } + + /** @var OperationRunService $operationRunService */ + $operationRunService = app(OperationRunService::class); + + $identityInputs = self::operationRunIdentityInputs( + tenantId: (int) $tenant->getKey(), + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: (string) ($tenant->external_id ?? ''), + policyExternalId: (string) $backupItem->policy_identifier, + ); + + $run = $operationRunService->ensureRunWithIdentityCooldown( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + context: self::operationRunContext($backupItem), + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + initiator: $initiator, + ); + + if ($run->wasRecentlyCreated) { + dispatch(new self( + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: (string) ($tenant->external_id ?? ''), + policyExternalId: (string) $backupItem->policy_identifier, + policyPayload: $policyPayload, + operationRun: $run, + )); + } + + return $run; } /** * Execute the job. */ - public function handle(AssignmentBackupService $assignmentBackupService): void - { + public function handle( + AssignmentBackupService $assignmentBackupService, + OperationRunService $operationRunService, + ): void { + $backupItem = BackupItem::query() + ->with('tenant') + ->find($this->backupItemId); + + if (! $backupItem instanceof BackupItem) { + Log::warning('FetchAssignmentsJob: BackupItem not found', [ + 'backup_item_id' => $this->backupItemId, + ]); + + return; + } + + $tenant = $backupItem->tenant; + + if (! $tenant instanceof Tenant) { + Log::warning('FetchAssignmentsJob: Tenant not found for BackupItem', [ + 'backup_item_id' => $this->backupItemId, + ]); + + return; + } + + $fingerprint = AssignmentJobFingerprint::forFetch( + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: $this->tenantExternalId, + policyExternalId: $this->policyExternalId, + ); + + $identityInputs = self::operationRunIdentityInputs( + tenantId: (int) $tenant->getKey(), + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: $this->tenantExternalId, + policyExternalId: $this->policyExternalId, + ); + + if (! $this->operationRun instanceof OperationRun) { + $this->operationRun = $operationRunService->ensureRunWithIdentityCooldown( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + context: self::operationRunContext($backupItem), + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + ); + } + + $canonicalRun = $operationRunService->findCanonicalRunWithIdentity( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + ); + + if ($canonicalRun instanceof OperationRun && $canonicalRun->status === OperationRunStatus::Completed->value) { + Log::info('FetchAssignmentsJob: Skipping because identity is in cooldown window', [ + 'backup_item_id' => (int) $backupItem->getKey(), + 'operation_run_id' => (int) $canonicalRun->getKey(), + ]); + + return; + } + + if ($canonicalRun instanceof OperationRun + && $this->operationRun instanceof OperationRun + && (int) $canonicalRun->getKey() !== (int) $this->operationRun->getKey() + ) { + Log::info('FetchAssignmentsJob: Skipping non-canonical execution', [ + 'backup_item_id' => (int) $backupItem->getKey(), + 'canonical_run_id' => (int) $canonicalRun->getKey(), + 'job_run_id' => (int) $this->operationRun->getKey(), + ]); + + return; + } + + $run = $this->operationRun instanceof OperationRun ? $this->operationRun : $canonicalRun; + + if (! $run instanceof OperationRun) { + return; + } + + $executionIdentityKey = AssignmentJobFingerprint::executionIdentityKey( + jobType: self::OPERATION_TYPE, + tenantId: (int) $tenant->getKey(), + fingerprint: $fingerprint, + operationRunId: (int) $run->getKey(), + ); + + $lock = Cache::lock('assignment-job:'.$executionIdentityKey, self::EXECUTION_LOCK_TTL_SECONDS); + + if (! $lock->get()) { + Log::info('FetchAssignmentsJob: Overlap prevented for duplicate execution', [ + 'backup_item_id' => (int) $backupItem->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'execution_identity' => $executionIdentityKey, + ]); + + return; + } + try { - $backupItem = BackupItem::find($this->backupItemId); - - if ($backupItem === null) { - Log::warning('FetchAssignmentsJob: BackupItem not found', [ - 'backup_item_id' => $this->backupItemId, - ]); - - return; + if ($run->status !== OperationRunStatus::Completed->value) { + $operationRunService->updateRun($run, OperationRunStatus::Running->value); } - $tenant = $backupItem->tenant; - - if ($tenant === null) { - Log::warning('FetchAssignmentsJob: Tenant not found for BackupItem', [ - 'backup_item_id' => $this->backupItemId, - ]); - - return; - } - - // Only process Settings Catalog policies if ($backupItem->policy_type !== 'settingsCatalogPolicy') { Log::info('FetchAssignmentsJob: Skipping non-Settings Catalog policy', [ - 'backup_item_id' => $this->backupItemId, - 'policy_type' => $backupItem->policy_type, + 'backup_item_id' => (int) $backupItem->getKey(), + 'policy_type' => (string) $backupItem->policy_type, ]); + $this->completeRun( + operationRunService: $operationRunService, + run: $run, + fetchFailed: false, + ); + return; } $assignmentBackupService->enrichWithAssignments( backupItem: $backupItem, tenant: $tenant, - policyType: $backupItem->policy_type, - policyId: $backupItem->policy_identifier, + policyType: (string) $backupItem->policy_type, + policyId: (string) $backupItem->policy_identifier, policyPayload: $this->policyPayload, - includeAssignments: true + includeAssignments: true, ); - Log::info('FetchAssignmentsJob: Successfully enriched BackupItem', [ - 'backup_item_id' => $this->backupItemId, - 'assignment_count' => $backupItem->getAssignmentCount(), + $backupItem->refresh(); + $metadata = is_array($backupItem->metadata) ? $backupItem->metadata : []; + + $fetchFailed = (bool) ($metadata['assignments_fetch_failed'] ?? false); + $reasonCandidate = $metadata['assignments_fetch_error_code'] + ?? $metadata['assignments_fetch_error'] + ?? ProviderReasonCodes::UnknownError; + + $reasonCode = RunFailureSanitizer::normalizeReasonCode( + $this->normalizeReasonCandidate($reasonCandidate), + ); + + $this->completeRun( + operationRunService: $operationRunService, + run: $run, + fetchFailed: $fetchFailed, + failures: $fetchFailed + ? [[ + 'code' => 'assignments.fetch_failed', + 'reason_code' => $reasonCode, + 'message' => (string) ($metadata['assignments_fetch_error'] ?? 'Assignments fetch failed.'), + ]] + : [], + ); + + Log::info('FetchAssignmentsJob: Successfully processed', [ + 'backup_item_id' => (int) $backupItem->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'assignment_count' => $backupItem->getAssignmentCountAttribute(), + 'fetch_failed' => $fetchFailed, ]); - } catch (\Throwable $e) { + } catch (\Throwable $throwable) { Log::error('FetchAssignmentsJob: Failed to enrich BackupItem', [ - 'backup_item_id' => $this->backupItemId, - 'error' => $e->getMessage(), - 'trace' => $e->getTraceAsString(), + 'backup_item_id' => (int) $backupItem->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'error' => $throwable->getMessage(), ]); - // Don't retry - fail soft - $this->fail($e); + $safeMessage = RunFailureSanitizer::sanitizeMessage($throwable->getMessage()); + + $operationRunService->updateRun( + $run, + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Failed->value, + summaryCounts: [ + 'total' => 1, + 'processed' => 0, + 'failed' => 1, + ], + failures: [[ + 'code' => 'assignments.fetch_failed', + 'reason_code' => RunFailureSanitizer::normalizeReasonCode($throwable->getMessage()), + 'message' => $safeMessage !== '' ? $safeMessage : 'Assignments fetch failed.', + ]], + ); + } finally { + $lock->release(); } } + + /** + * @return array + */ + private static function operationRunIdentityInputs( + int $tenantId, + int $backupItemId, + string $tenantExternalId, + string $policyExternalId, + ): array { + return [ + 'tenant_id' => $tenantId, + 'job_type' => self::OPERATION_TYPE, + 'fingerprint' => AssignmentJobFingerprint::forFetch( + backupItemId: $backupItemId, + tenantExternalId: $tenantExternalId, + policyExternalId: $policyExternalId, + ), + ]; + } + + /** + * @return array + */ + private static function operationRunContext(BackupItem $backupItem): array + { + return [ + 'backup_set_id' => is_numeric($backupItem->backup_set_id) ? (int) $backupItem->backup_set_id : null, + 'backup_item_id' => (int) $backupItem->getKey(), + 'policy_id' => is_numeric($backupItem->policy_id) ? (int) $backupItem->policy_id : null, + 'policy_identifier' => (string) $backupItem->policy_identifier, + ]; + } + + /** + * @param array $failures + */ + private function completeRun( + OperationRunService $operationRunService, + OperationRun $run, + bool $fetchFailed, + array $failures = [], + ): void { + $operationRunService->updateRun( + $run, + status: OperationRunStatus::Completed->value, + outcome: $fetchFailed ? OperationRunOutcome::Failed->value : OperationRunOutcome::Succeeded->value, + summaryCounts: [ + 'total' => 1, + 'processed' => $fetchFailed ? 0 : 1, + 'failed' => $fetchFailed ? 1 : 0, + ], + failures: $failures, + ); + } + + private function normalizeReasonCandidate(mixed $reasonCandidate): string + { + if (is_string($reasonCandidate) && trim($reasonCandidate) !== '') { + return trim($reasonCandidate); + } + + return ProviderReasonCodes::UnknownError; + } } diff --git a/app/Jobs/ReconcileAdapterRunsJob.php b/app/Jobs/ReconcileAdapterRunsJob.php index 1ba1a5b..4250de4 100644 --- a/app/Jobs/ReconcileAdapterRunsJob.php +++ b/app/Jobs/ReconcileAdapterRunsJob.php @@ -2,9 +2,15 @@ namespace App\Jobs; +use App\Models\Workspace; use App\Services\AdapterRunReconciler; +use App\Services\OperationRunService; +use App\Support\OperationRunOutcome; +use App\Support\OperationRunStatus; +use App\Support\OpsUx\RunFailureSanitizer; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; +use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Support\Facades\Log; use Throwable; @@ -12,40 +18,137 @@ class ReconcileAdapterRunsJob implements ShouldQueue { use Queueable; + private const string OPERATION_TYPE = 'ops.reconcile_adapter_runs'; + + private const int LOCK_TTL_SECONDS = 900; + /** * Create a new job instance. */ - public function __construct() + public function __construct( + public ?int $workspaceId = null, + public ?string $slotKey = null, + ) { + $this->slotKey = $slotKey ?: now()->startOfMinute()->toDateTimeString(); + } + + /** + * @return array + */ + public function middleware(): array { - // + return [ + (new WithoutOverlapping(self::OPERATION_TYPE)) + ->expireAfter(self::LOCK_TTL_SECONDS) + ->dontRelease(), + ]; } /** * Execute the job. */ - public function handle(): void - { - try { - /** @var AdapterRunReconciler $reconciler */ - $reconciler = app(AdapterRunReconciler::class); + public function handle( + AdapterRunReconciler $reconciler, + OperationRunService $operationRunService, + ): void { + $workspace = $this->resolveWorkspace(); - $result = $reconciler->reconcile([ - 'older_than_minutes' => 60, - 'limit' => 50, - 'dry_run' => false, - ]); + if (! $workspace instanceof Workspace) { + Log::warning('ReconcileAdapterRunsJob skipped because no workspace was found.'); + + return; + } + + $operationRun = $operationRunService->ensureWorkspaceRunWithIdentity( + workspace: $workspace, + type: self::OPERATION_TYPE, + identityInputs: [ + 'job' => self::OPERATION_TYPE, + 'slot' => (string) $this->slotKey, + ], + context: [ + 'job' => self::OPERATION_TYPE, + 'slot' => (string) $this->slotKey, + 'trigger' => 'schedule', + ], + ); + + if ($operationRun->status !== OperationRunStatus::Completed->value) { + $operationRunService->updateRun($operationRun, OperationRunStatus::Running->value); + } + + try { + $result = $this->reconcile($reconciler); + + $candidates = (int) ($result['candidates'] ?? 0); + $reconciled = (int) ($result['reconciled'] ?? 0); + $skipped = (int) ($result['skipped'] ?? 0); + + $operationRunService->updateRun( + $operationRun, + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Succeeded->value, + summaryCounts: [ + 'total' => $candidates, + 'processed' => $reconciled + $skipped, + 'failed' => 0, + ], + ); Log::info('ReconcileAdapterRunsJob completed', [ - 'candidates' => (int) ($result['candidates'] ?? 0), - 'reconciled' => (int) ($result['reconciled'] ?? 0), - 'skipped' => (int) ($result['skipped'] ?? 0), + 'operation_run_id' => (int) $operationRun->getKey(), + 'candidates' => $candidates, + 'reconciled' => $reconciled, + 'skipped' => $skipped, ]); } catch (Throwable $e) { + $safeMessage = RunFailureSanitizer::sanitizeMessage($e->getMessage()); + + $operationRunService->updateRun( + $operationRun, + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Failed->value, + summaryCounts: [ + 'total' => 0, + 'processed' => 0, + 'failed' => 1, + ], + failures: [[ + 'code' => 'ops.reconcile_adapter_runs.failed', + 'reason_code' => RunFailureSanitizer::normalizeReasonCode($e->getMessage()), + 'message' => $safeMessage !== '' ? $safeMessage : 'Adapter run reconciliation failed.', + ]], + ); + Log::warning('ReconcileAdapterRunsJob failed', [ - 'error' => $e->getMessage(), + 'operation_run_id' => (int) $operationRun->getKey(), + 'error' => $safeMessage !== '' ? $safeMessage : 'Adapter run reconciliation failed.', ]); throw $e; } } + + /** + * @return array{candidates:int, reconciled:int, skipped:int} + */ + protected function reconcile(AdapterRunReconciler $reconciler): array + { + return $reconciler->reconcile([ + 'older_than_minutes' => 60, + 'limit' => 50, + 'dry_run' => false, + ]); + } + + private function resolveWorkspace(): ?Workspace + { + if (is_int($this->workspaceId) && $this->workspaceId > 0) { + return Workspace::query()->find($this->workspaceId); + } + + return Workspace::query() + ->orderBy('id') + ->first(); + } } diff --git a/app/Jobs/RestoreAssignmentsJob.php b/app/Jobs/RestoreAssignmentsJob.php index 92b6f67..af5ea98 100644 --- a/app/Jobs/RestoreAssignmentsJob.php +++ b/app/Jobs/RestoreAssignmentsJob.php @@ -2,22 +2,35 @@ namespace App\Jobs; -use App\Jobs\Middleware\TrackOperationRun; use App\Models\OperationRun; use App\Models\RestoreRun; use App\Models\Tenant; +use App\Models\User; use App\Services\AssignmentRestoreService; +use App\Services\OperationRunService; +use App\Support\OperationRunOutcome; +use App\Support\OperationRunStatus; +use App\Support\OpsUx\AssignmentJobFingerprint; +use App\Support\OpsUx\RunFailureSanitizer; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; +use RuntimeException; class RestoreAssignmentsJob implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + private const string OPERATION_TYPE = 'assignments.restore'; + + private const int DEDUPE_COOLDOWN_MINUTES = 15; + + private const int EXECUTION_LOCK_TTL_SECONDS = 900; + public ?OperationRun $operationRun = null; public int $tries = 1; @@ -42,23 +55,76 @@ public function __construct( $this->operationRun = $operationRun; } - /** - * @return array - */ - public function middleware(): array - { - return [new TrackOperationRun]; + public static function dispatchTracked( + int $restoreRunId, + Tenant $tenant, + string $policyType, + string $policyId, + array $assignments, + array $groupMapping, + array $foundationMapping = [], + ?string $actorEmail = null, + ?string $actorName = null, + ?User $initiator = null, + ): OperationRun { + /** @var OperationRunService $operationRunService */ + $operationRunService = app(OperationRunService::class); + + $identityInputs = self::operationRunIdentityInputs( + restoreRunId: $restoreRunId, + tenantId: (int) $tenant->getKey(), + policyType: $policyType, + policyId: $policyId, + assignments: $assignments, + groupMapping: $groupMapping, + foundationMapping: $foundationMapping, + ); + + $run = $operationRunService->ensureRunWithIdentityCooldown( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + context: self::operationRunContext( + restoreRunId: $restoreRunId, + policyType: $policyType, + policyId: $policyId, + assignments: $assignments, + ), + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + initiator: $initiator, + ); + + if ($run->wasRecentlyCreated) { + dispatch(new self( + restoreRunId: $restoreRunId, + tenantId: (int) $tenant->getKey(), + policyType: $policyType, + policyId: $policyId, + assignments: $assignments, + groupMapping: $groupMapping, + foundationMapping: $foundationMapping, + actorEmail: $actorEmail, + actorName: $actorName, + operationRun: $run, + )); + } + + return $run; } /** * Execute the job. + * + * @return array{outcomes: array>, summary: array{success:int,failed:int,skipped:int}} */ - public function handle(AssignmentRestoreService $assignmentRestoreService): array - { - $restoreRun = RestoreRun::find($this->restoreRunId); - $tenant = Tenant::find($this->tenantId); + public function handle( + AssignmentRestoreService $assignmentRestoreService, + OperationRunService $operationRunService, + ): array { + $restoreRun = RestoreRun::query()->find($this->restoreRunId); + $tenant = Tenant::query()->find($this->tenantId); - if (! $restoreRun || ! $tenant) { + if (! $tenant instanceof Tenant || ! $restoreRun instanceof RestoreRun) { Log::warning('RestoreAssignmentsJob missing context', [ 'restore_run_id' => $this->restoreRunId, 'tenant_id' => $this->tenantId, @@ -70,8 +136,110 @@ public function handle(AssignmentRestoreService $assignmentRestoreService): arra ]; } + $identityInputs = self::operationRunIdentityInputs( + restoreRunId: $this->restoreRunId, + tenantId: $this->tenantId, + policyType: $this->policyType, + policyId: $this->policyId, + assignments: $this->assignments, + groupMapping: $this->groupMapping, + foundationMapping: $this->foundationMapping, + ); + + $fingerprint = AssignmentJobFingerprint::forRestore( + restoreRunId: $this->restoreRunId, + tenantId: $this->tenantId, + policyType: $this->policyType, + policyId: $this->policyId, + assignments: $this->assignments, + groupMapping: $this->groupMapping, + foundationMapping: $this->foundationMapping, + ); + + if (! $this->operationRun instanceof OperationRun) { + $this->operationRun = $operationRunService->ensureRunWithIdentityCooldown( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + context: self::operationRunContext( + restoreRunId: $this->restoreRunId, + policyType: $this->policyType, + policyId: $this->policyId, + assignments: $this->assignments, + ), + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + ); + } + + $canonicalRun = $operationRunService->findCanonicalRunWithIdentity( + tenant: $tenant, + type: self::OPERATION_TYPE, + identityInputs: $identityInputs, + cooldownMinutes: self::DEDUPE_COOLDOWN_MINUTES, + ); + + if ($canonicalRun instanceof OperationRun && $canonicalRun->status === OperationRunStatus::Completed->value) { + Log::info('RestoreAssignmentsJob: Skipping because identity is in cooldown window', [ + 'restore_run_id' => $this->restoreRunId, + 'operation_run_id' => (int) $canonicalRun->getKey(), + ]); + + return [ + 'outcomes' => [], + 'summary' => ['success' => 0, 'failed' => 0, 'skipped' => 0], + ]; + } + + if ($canonicalRun instanceof OperationRun + && $this->operationRun instanceof OperationRun + && (int) $canonicalRun->getKey() !== (int) $this->operationRun->getKey() + ) { + Log::info('RestoreAssignmentsJob: Skipping non-canonical execution', [ + 'restore_run_id' => $this->restoreRunId, + 'canonical_run_id' => (int) $canonicalRun->getKey(), + 'job_run_id' => (int) $this->operationRun->getKey(), + ]); + + return [ + 'outcomes' => [], + 'summary' => ['success' => 0, 'failed' => 0, 'skipped' => 0], + ]; + } + + $run = $this->operationRun instanceof OperationRun ? $this->operationRun : $canonicalRun; + + if (! $run instanceof OperationRun) { + throw new RuntimeException('OperationRun is required for RestoreAssignmentsJob execution.'); + } + + $executionIdentityKey = AssignmentJobFingerprint::executionIdentityKey( + jobType: self::OPERATION_TYPE, + tenantId: (int) $tenant->getKey(), + fingerprint: $fingerprint, + operationRunId: (int) $run->getKey(), + ); + + $lock = Cache::lock('assignment-job:'.$executionIdentityKey, self::EXECUTION_LOCK_TTL_SECONDS); + + if (! $lock->get()) { + Log::info('RestoreAssignmentsJob: Overlap prevented for duplicate execution', [ + 'restore_run_id' => $this->restoreRunId, + 'operation_run_id' => (int) $run->getKey(), + 'execution_identity' => $executionIdentityKey, + ]); + + return [ + 'outcomes' => [], + 'summary' => ['success' => 0, 'failed' => 0, 'skipped' => 0], + ]; + } + try { - return $assignmentRestoreService->restore( + if ($run->status !== OperationRunStatus::Completed->value) { + $operationRunService->updateRun($run, OperationRunStatus::Running->value); + } + + $result = $assignmentRestoreService->restore( tenant: $tenant, policyType: $this->policyType, policyId: $this->policyId, @@ -82,20 +250,167 @@ public function handle(AssignmentRestoreService $assignmentRestoreService): arra actorEmail: $this->actorEmail, actorName: $this->actorName, ); - } catch (\Throwable $e) { + + $summary = is_array($result['summary'] ?? null) ? $result['summary'] : []; + $success = (int) ($summary['success'] ?? 0); + $failed = (int) ($summary['failed'] ?? 0); + $skipped = (int) ($summary['skipped'] ?? 0); + $total = $success + $failed + $skipped; + $processed = $success + $failed; + + $outcome = OperationRunOutcome::Succeeded->value; + + if ($failed > 0 && $success > 0) { + $outcome = OperationRunOutcome::PartiallySucceeded->value; + } elseif ($failed > 0) { + $outcome = OperationRunOutcome::Failed->value; + } + + $failures = $this->extractFailures( + outcomes: is_array($result['outcomes'] ?? null) ? $result['outcomes'] : [], + fallbackMessage: 'Assignments restore failed.', + ); + + $operationRunService->updateRun( + $run, + status: OperationRunStatus::Completed->value, + outcome: $outcome, + summaryCounts: [ + 'total' => $total, + 'processed' => $processed, + 'failed' => $failed, + ], + failures: $failures, + ); + + return [ + 'outcomes' => is_array($result['outcomes'] ?? null) ? $result['outcomes'] : [], + 'summary' => [ + 'success' => $success, + 'failed' => $failed, + 'skipped' => $skipped, + ], + ]; + } catch (\Throwable $throwable) { Log::error('RestoreAssignmentsJob failed', [ 'restore_run_id' => $this->restoreRunId, 'policy_id' => $this->policyId, - 'error' => $e->getMessage(), + 'operation_run_id' => (int) $run->getKey(), + 'error' => $throwable->getMessage(), ]); + $safeMessage = RunFailureSanitizer::sanitizeMessage($throwable->getMessage()); + + $operationRunService->updateRun( + $run, + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Failed->value, + summaryCounts: [ + 'total' => max(1, count($this->assignments)), + 'processed' => 0, + 'failed' => max(1, count($this->assignments)), + ], + failures: [[ + 'code' => 'assignments.restore_failed', + 'reason_code' => RunFailureSanitizer::normalizeReasonCode($throwable->getMessage()), + 'message' => $safeMessage !== '' ? $safeMessage : 'Assignments restore failed.', + ]], + ); + return [ 'outcomes' => [[ 'status' => 'failed', - 'reason' => $e->getMessage(), + 'reason' => $safeMessage !== '' ? $safeMessage : 'Assignments restore failed.', ]], 'summary' => ['success' => 0, 'failed' => 1, 'skipped' => 0], ]; + } finally { + $lock->release(); } } + + /** + * @param array> $assignments + * @param array $groupMapping + * @param array $foundationMapping + * @return array + */ + private static function operationRunIdentityInputs( + int $restoreRunId, + int $tenantId, + string $policyType, + string $policyId, + array $assignments, + array $groupMapping, + array $foundationMapping = [], + ): array { + return [ + 'tenant_id' => $tenantId, + 'job_type' => self::OPERATION_TYPE, + 'fingerprint' => AssignmentJobFingerprint::forRestore( + restoreRunId: $restoreRunId, + tenantId: $tenantId, + policyType: $policyType, + policyId: $policyId, + assignments: $assignments, + groupMapping: $groupMapping, + foundationMapping: $foundationMapping, + ), + ]; + } + + /** + * @param array> $assignments + * @return array + */ + private static function operationRunContext( + int $restoreRunId, + string $policyType, + string $policyId, + array $assignments, + ): array { + return [ + 'restore_run_id' => $restoreRunId, + 'policy_type' => trim($policyType), + 'policy_id' => trim($policyId), + 'assignment_item_count' => count($assignments), + ]; + } + + /** + * @param array> $outcomes + * @return array + */ + private function extractFailures(array $outcomes, string $fallbackMessage): array + { + $failures = []; + + foreach ($outcomes as $outcome) { + if (! is_array($outcome)) { + continue; + } + + if (($outcome['status'] ?? null) !== 'failed') { + continue; + } + + $messageCandidate = is_string($outcome['reason'] ?? null) + ? (string) $outcome['reason'] + : (is_string($outcome['message'] ?? null) ? (string) $outcome['message'] : $fallbackMessage); + + $message = RunFailureSanitizer::sanitizeMessage($messageCandidate); + + $failures[] = [ + 'code' => 'assignments.restore_failed', + 'reason_code' => RunFailureSanitizer::normalizeReasonCode($messageCandidate), + 'message' => $message !== '' ? $message : $fallbackMessage, + ]; + } + + if ($failures === []) { + return []; + } + + return array_slice($failures, 0, 10); + } } diff --git a/app/Services/OperationRunService.php b/app/Services/OperationRunService.php index e7596d3..6c77483 100644 --- a/app/Services/OperationRunService.php +++ b/app/Services/OperationRunService.php @@ -184,6 +184,75 @@ public function ensureRunWithIdentity( } } + public function ensureRunWithIdentityCooldown( + Tenant $tenant, + string $type, + array $identityInputs, + array $context, + int $cooldownMinutes = 15, + ?User $initiator = null, + ): OperationRun { + $existing = $this->findCanonicalRunWithIdentity( + tenant: $tenant, + type: $type, + identityInputs: $identityInputs, + cooldownMinutes: $cooldownMinutes, + ); + + if ($existing instanceof OperationRun) { + return $existing; + } + + return $this->ensureRunWithIdentity( + tenant: $tenant, + type: $type, + identityInputs: $identityInputs, + context: $context, + initiator: $initiator, + ); + } + + public function findCanonicalRunWithIdentity( + Tenant $tenant, + string $type, + array $identityInputs, + int $cooldownMinutes = 15, + ): ?OperationRun { + $workspaceId = (int) ($tenant->workspace_id ?? 0); + + if ($workspaceId <= 0) { + throw new InvalidArgumentException('Tenant must belong to a workspace to resolve operation run identity.'); + } + + $hash = $this->calculateHash((int) $tenant->getKey(), $type, $identityInputs); + + $activeRun = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('workspace_id', $workspaceId) + ->where('run_identity_hash', $hash) + ->whereIn('status', [OperationRunStatus::Queued->value, OperationRunStatus::Running->value]) + ->orderByDesc('id') + ->first(); + + if ($activeRun instanceof OperationRun) { + return $activeRun; + } + + $cooldownMinutes = max(1, $cooldownMinutes); + $cutoff = now()->subMinutes($cooldownMinutes); + + return OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('workspace_id', $workspaceId) + ->where('run_identity_hash', $hash) + ->where('status', OperationRunStatus::Completed->value) + ->whereNotNull('completed_at') + ->where('completed_at', '>=', $cutoff) + ->orderByDesc('completed_at') + ->orderByDesc('id') + ->first(); + } + public function ensureRunWithIdentityStrict( Tenant $tenant, string $type, diff --git a/app/Support/OperationCatalog.php b/app/Support/OperationCatalog.php index 67ed751..8bebac5 100644 --- a/app/Support/OperationCatalog.php +++ b/app/Support/OperationCatalog.php @@ -34,6 +34,7 @@ public static function labels(): array 'restore.execute' => 'Restore execution', 'assignments.fetch' => 'Assignment fetch', 'assignments.restore' => 'Assignment restore', + 'ops.reconcile_adapter_runs' => 'Reconcile adapter runs', 'directory_role_definitions.sync' => 'Role definitions sync', 'restore_run.delete' => 'Delete restore runs', 'restore_run.restore' => 'Restore restore runs', @@ -67,6 +68,7 @@ public static function expectedDurationSeconds(string $operationType): ?int 'entra_group_sync' => 120, 'drift_generate_findings' => 240, 'assignments.fetch', 'assignments.restore' => 60, + 'ops.reconcile_adapter_runs' => 120, default => null, }; } diff --git a/app/Support/OpsUx/AssignmentJobFingerprint.php b/app/Support/OpsUx/AssignmentJobFingerprint.php new file mode 100644 index 0000000..d79f47f --- /dev/null +++ b/app/Support/OpsUx/AssignmentJobFingerprint.php @@ -0,0 +1,139 @@ + $backupItemId, + 'tenant_external_id' => trim($tenantExternalId), + 'policy_external_id' => trim($policyExternalId), + ]); + } + + /** + * @param array> $assignments + * @param array $groupMapping + * @param array $foundationMapping + */ + public static function forRestore( + int $restoreRunId, + int $tenantId, + string $policyType, + string $policyId, + array $assignments, + array $groupMapping, + array $foundationMapping = [], + ): string { + return self::hash('assignments.restore', [ + 'restore_run_id' => $restoreRunId, + 'tenant_id' => $tenantId, + 'policy_type' => trim($policyType), + 'policy_id' => trim($policyId), + 'assignments' => self::normalizeAssignments($assignments), + 'group_mapping' => $groupMapping, + 'foundation_mapping' => $foundationMapping, + ]); + } + + public static function executionIdentityKey( + string $jobType, + int $tenantId, + string $fingerprint, + ?int $operationRunId = null, + ): string { + if (is_int($operationRunId) && $operationRunId > 0) { + return 'operation_run:'.$operationRunId; + } + + return 'tenant:'.$tenantId + .'|job_type:'.trim($jobType) + .'|fingerprint:'.trim($fingerprint); + } + + /** + * @param array $identityInputs + */ + private static function hash(string $jobType, array $identityInputs): string + { + $payload = [ + 'job_type' => trim($jobType), + 'identity' => self::normalize($identityInputs), + ]; + + $json = json_encode($payload, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + + return hash('sha256', (string) $json); + } + + /** + * @param array> $assignments + * @return array> + */ + private static function normalizeAssignments(array $assignments): array + { + $normalized = []; + + foreach ($assignments as $assignment) { + if (! is_array($assignment)) { + continue; + } + + $target = is_array($assignment['target'] ?? null) ? $assignment['target'] : []; + + $normalized[] = [ + 'id' => is_scalar($assignment['id'] ?? null) ? (string) $assignment['id'] : '', + 'target_type' => is_scalar($target['@odata.type'] ?? null) ? (string) $target['@odata.type'] : '', + 'group_id' => is_scalar($target['groupId'] ?? null) ? (string) $target['groupId'] : '', + 'assignment_filter_id' => is_scalar($assignment['deviceAndAppManagementAssignmentFilterId'] ?? null) + ? (string) $assignment['deviceAndAppManagementAssignmentFilterId'] + : (is_scalar($target['deviceAndAppManagementAssignmentFilterId'] ?? null) ? (string) $target['deviceAndAppManagementAssignmentFilterId'] : ''), + 'assignment_filter_type' => is_scalar($assignment['deviceAndAppManagementAssignmentFilterType'] ?? null) + ? (string) $assignment['deviceAndAppManagementAssignmentFilterType'] + : (is_scalar($target['deviceAndAppManagementAssignmentFilterType'] ?? null) ? (string) $target['deviceAndAppManagementAssignmentFilterType'] : ''), + ]; + } + + usort($normalized, static function (array $left, array $right): int { + $leftJson = json_encode($left, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + $rightJson = json_encode($right, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + + return strcmp((string) $leftJson, (string) $rightJson); + }); + + return $normalized; + } + + private static function normalize(mixed $value): mixed + { + if (! is_array($value)) { + return $value; + } + + if (array_is_list($value)) { + $items = array_map(static fn (mixed $item): mixed => self::normalize($item), $value); + + usort($items, static function (mixed $left, mixed $right): int { + $leftJson = json_encode($left, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + $rightJson = json_encode($right, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + + return strcmp((string) $leftJson, (string) $rightJson); + }); + + return array_values($items); + } + + ksort($value); + + foreach ($value as $key => $item) { + $value[$key] = self::normalize($item); + } + + return $value; + } +} diff --git a/database/seeders/PlatformUserSeeder.php b/database/seeders/PlatformUserSeeder.php index 670c36a..4b9e10c 100644 --- a/database/seeders/PlatformUserSeeder.php +++ b/database/seeders/PlatformUserSeeder.php @@ -4,6 +4,7 @@ use App\Models\PlatformUser; use App\Models\Tenant; +use App\Models\Workspace; use Illuminate\Database\Seeder; use Illuminate\Support\Facades\Hash; @@ -14,9 +15,14 @@ class PlatformUserSeeder extends Seeder */ public function run(): void { + $workspace = Workspace::query()->firstOrCreate( + ['slug' => 'default'], + ['name' => 'Default Workspace', 'slug' => 'default'], + ); + Tenant::query()->updateOrCreate( ['external_id' => 'platform'], - ['name' => 'Platform'], + ['name' => 'Platform', 'workspace_id' => (int) $workspace->getKey()], ); PlatformUser::query()->updateOrCreate( diff --git a/database/seeders/PoliciesSeeder.php b/database/seeders/PoliciesSeeder.php index 74ee9d5..604bcd0 100644 --- a/database/seeders/PoliciesSeeder.php +++ b/database/seeders/PoliciesSeeder.php @@ -4,7 +4,9 @@ use App\Models\Policy; use App\Models\Tenant; +use App\Models\Workspace; use Illuminate\Database\Seeder; +use Illuminate\Support\Str; class PoliciesSeeder extends Seeder { @@ -12,15 +14,36 @@ public function run(): void { $seedTenantId = env('INTUNE_TENANT_ID', 'local-tenant'); + $workspace = Workspace::query()->firstOrCreate( + ['slug' => 'default'], + ['name' => 'Default Workspace', 'slug' => 'default'], + ); + $tenant = Tenant::firstOrCreate([ 'tenant_id' => $seedTenantId, ], [ + 'workspace_id' => (int) $workspace->getKey(), 'name' => 'Default Tenant', - 'external_id' => $seedTenantId, + 'external_id' => (string) Str::uuid(), 'domain' => null, 'metadata' => [], ]); + if ($tenant->workspace_id === null) { + $tenant->forceFill([ + 'workspace_id' => (int) $workspace->getKey(), + ])->saveQuietly(); + } + + $externalId = (string) ($tenant->external_id ?? ''); + $isUuidV4 = Str::isUuid($externalId) && substr($externalId, 14, 1) === '4'; + + if (! $isUuidV4) { + $tenant->forceFill([ + 'external_id' => (string) Str::uuid(), + ])->saveQuietly(); + } + $supported = config('tenantpilot.supported_policy_types', []); $now = now(); @@ -31,11 +54,13 @@ public function run(): void Policy::updateOrCreate( [ + 'workspace_id' => (int) $workspace->getKey(), 'tenant_id' => $tenant->id, 'external_id' => $externalId, 'policy_type' => $policyType, ], [ + 'workspace_id' => (int) $workspace->getKey(), 'display_name' => $type['label'] ?? ucfirst($policyType), 'platform' => $platform, 'last_synced_at' => $now, diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/checklists/requirements.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/checklists/requirements.md new file mode 100644 index 0000000..ffbe150 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/checklists/requirements.md @@ -0,0 +1,42 @@ +# Specification Quality Checklist: 096 — Ops Polish Bundle + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-02-15 +**Feature**: ../spec.md + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan`. +- Ops bundle must cover: + - Assignment job summaries persisted + - Assignment job dedupe + - System job run tracking for reconcile-adapter-runs + - Seed workflow succeeds and tenants have external IDs + - Automated tests for each scope item + +Validation result: PASS (all checklist items complete) diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/contracts/openapi.yaml b/specs/096-ops-polish-assignment-dedupe-system-tracking/contracts/openapi.yaml new file mode 100644 index 0000000..5ab8833 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/contracts/openapi.yaml @@ -0,0 +1,10 @@ +openapi: 3.1.0 +info: + title: TenantPilot - Spec 096 (Ops Polish Bundle) + version: "0.0.0" + description: | + Spec 096 introduces no new HTTP API endpoints. + + Scope is background queue jobs + OperationRun tracking + seeder behavior. +paths: {} +components: {} diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/data-model.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/data-model.md new file mode 100644 index 0000000..5a1c49a --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/data-model.md @@ -0,0 +1,49 @@ +# Phase 1 — Data Model (096 Ops Polish Bundle) + +No new tables are required. The feature reuses existing operational ledger and seed data structures. + +## Entity: `operation_runs` (tenant-scoped and workspace-scoped) + +**Purpose:** Canonical ledger for background operations (status, timestamps, outcomes, counters, failures). + +**Key fields (existing):** +- `id` (PK) +- `workspace_id` (FK, required) +- `tenant_id` (FK, nullable) + - Tenant-scoped operations: non-null + - Workspace-scoped operations (housekeeping): null +- `user_id` (FK, nullable) — initiator user when applicable +- `initiator_name` (string) — stored label for system/user +- `type` (string) — operation type (e.g., `assignments.fetch`, `assignments.restore`, `ops.reconcile_adapter_runs`) +- `status` (string) — `queued` | `running` | `completed` (see `OperationRunStatus`) +- `outcome` (string) — `pending` | `succeeded` | `failed` (see `OperationRunOutcome`) +- `run_identity_hash` (string) — deterministic identity hash +- `summary_counts` (json/jsonb array) — normalized counters (keys constrained by `OperationCatalog::allowedSummaryKeys()`) +- `failure_summary` (json/jsonb array) — entries like `{ code: string, message: string }` (sanitized, stable) +- `context` (json/jsonb object) — non-secret context, may include selection metadata +- `started_at`, `completed_at` (timestamps) +- `created_at`, `updated_at` + +**Constraints / indexes (existing):** +- Active-run dedupe is enforced at the DB layer using partial unique indexes: + - Tenant runs: unique on `(tenant_id, run_identity_hash)` for active statuses. + - Workspace runs: unique on `(workspace_id, run_identity_hash)` when `tenant_id IS NULL` for active statuses. + +**Feature usage:** +- Assignment jobs: ensure a stable `run_identity_hash` based on the clarified identity rule; persist `summary_counts` at terminal completion. +- Reconcile housekeeping job: create/reuse a workspace-scoped run (tenant_id null) and persist success/failure + summary counts. + +## Entity: `tenants` + +**Purpose:** Tenant scope boundary; owns tenant-scoped `operation_runs` and policies. + +**Field (in scope):** +- `external_id` (string) + +**Feature requirement:** +- Seeded tenants must have `external_id` set to a UUID v4 string. + +## Notes on validation / sanitization + +- Summary counters must be normalized/sanitized before persistence (existing `OperationRunService` behavior). +- Failure summaries must store stable reason codes + sanitized messages (no secrets/tokens/PII/raw payload dumps). diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/plan.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/plan.md new file mode 100644 index 0000000..d353a57 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/plan.md @@ -0,0 +1,140 @@ +# Implementation Plan: 096 — Ops Polish Bundle (Assignment job summaries + job dedupe + system job tracking + seeder DX) + +**Branch**: `096-ops-polish-assignment-dedupe-system-tracking` | **Date**: 2026-02-15 | **Spec**: ./spec.md +**Input**: Feature specification from `specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md` + +**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts. + +## Summary + +Improve operational reliability and observability for assignment-related jobs and a housekeeping job by: + +- Persisting durable `OperationRun.summary_counts` for assignment fetch / restore runs (final-attempt semantics, no double counting across retries). +- Enforcing server-side deduplication for assignment jobs using a stable identity and the existing DB-level active-run unique indexes. +- Tracking `ReconcileAdapterRunsJob` as a workspace-scoped `OperationRun` (`type = ops.reconcile_adapter_runs`) with stable reason codes + sanitized errors. +- Fixing seed DX so seeded tenants always have a UUID v4 `external_id`. + +## Technical Context + + + +**Language/Version**: PHP 8.4.x, Laravel 12 +**Primary Dependencies**: Filament v5, Livewire v4, Laravel Sail (dev), PostgreSQL (dev), Microsoft Graph abstraction (existing) +**Storage**: PostgreSQL (JSONB used for `operation_runs.summary_counts`, `failure_summary`, `context`) +**Testing**: Pest v4 (via `vendor/bin/sail artisan test --compact`) +**Target Platform**: Linux container runtime (Dokploy deploy); macOS for local dev +**Project Type**: Laravel monolith (web + workers) +**Performance Goals**: Deduplication checks must be O(1) per dispatch/execute; no extra remote calls added +**Constraints**: No secrets in dedupe fingerprints, logs, or failure summaries; queued jobs remain safe under concurrency +**Scale/Scope**: Background ops for multiple tenants; correctness > throughput + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- Inventory-first: PASS (no inventory semantics changed) +- Read/write separation: PASS (no new “write flows”; internal counters + run tracking + seeding) +- Graph contract path: PASS (no new Graph calls; no render-time external calls) +- Deterministic capabilities: PASS (no capability logic changes) +- RBAC-UX planes/isolation: PASS (no routes/pages added) +- Workspace/tenant isolation: PASS (all `OperationRun` reads/writes remain scoped via existing services; workspace-scoped run used only for housekeeping) +- Destructive confirmation standard: N/A (no Filament actions in scope) +- Global search tenant safety: N/A (no global search changes) +- Run observability standard: PASS (adds/strengthens `OperationRun` coverage + stable failures) +- Automation locks + idempotency: PASS (dedupe enforced via existing active-run DB unique indexes + job-level skip) +- Data minimization & safe logging: PASS (fingerprints are non-secret; failure messages sanitized) +- BADGE-001: N/A (no badge domains changed) +- Filament Action Surface Contract: N/A (no Filament UI changed) + +## Project Structure + +### Documentation (this feature) + +```text +specs/096-ops-polish-assignment-dedupe-system-tracking/ +├── plan.md # This file (/speckit.plan command output) +├── research.md # Phase 0 output (/speckit.plan command) +├── data-model.md # Phase 1 output (/speckit.plan command) +├── quickstart.md # Phase 1 output (/speckit.plan command) +├── contracts/ # Phase 1 output (/speckit.plan command) +└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan) +``` + +### Source Code (repository root) + + +```text +app/ +├── Jobs/ +│ ├── FetchAssignmentsJob.php +│ ├── RestoreAssignmentsJob.php +│ └── ReconcileAdapterRunsJob.php +├── Jobs/Middleware/ +│ └── TrackOperationRun.php +├── Services/ +│ └── OperationRunService.php +└── Support/ + ├── OperationCatalog.php + └── OperationRunType.php + +database/seeders/ +└── PoliciesSeeder.php + +tests/Feature/ +└── (new/updated Pest coverage for summary persistence, dedupe, workspace job tracking, seeding) +``` + +**Structure Decision**: Laravel monolith. Feature work touches queue jobs + run tracking services + seeders + Pest tests. + +## Complexity Tracking + +> **Fill ONLY if Constitution Check has violations that must be justified** + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|-------------------------------------| +| [e.g., 4th project] | [current need] | [why 3 projects insufficient] | +| [e.g., Repository pattern] | [specific problem] | [why direct DB access insufficient] | + +## Phase 0 — Research (output: `research.md`) + +1. Confirm current run tracking + dedupe primitives in repo (`OperationRunService`, active-run unique indexes). +2. Confirm how assignment operations currently persist summaries (services vs jobs) and align on “final attempt wins”. +3. Confirm workspace-scoped `OperationRun` support for tenantless scheduled jobs. + +## Phase 1 — Design (output: `data-model.md`, `contracts/*`, `quickstart.md`) + +1. Data model: no schema changes expected; document how to use existing `operation_runs` fields for this feature. +2. Job dedupe design: + - Identity rule: prefer `operation_run_id`; otherwise `tenant_id + job_type + stable input fingerprint`. + - Enforcement: + - Dispatch-time: `OperationRunService::ensureRunWithIdentity(...)` (tenant-scoped) reuses the same active run. + - Execute-time: job must detect “not the canonical run” and skip to avoid overlap. + - Window: 15 minutes cooldown (blocks re-runs for the same identity until the window elapses; in addition to active-run overlap prevention). +3. Summary persistence design: + - Persist `summary_counts` at terminal completion via `OperationRunService::updateRun(...)` so retries overwrite. +4. Housekeeping job tracking: + - Use `OperationRunService::ensureWorkspaceRunWithIdentity(...)` with `type = ops.reconcile_adapter_runs`. + - Persist stable failure codes + sanitized messages via existing sanitizer patterns. +5. Seeder DX: + - Ensure `tenants.external_id` is always UUID v4 for seeded tenants (do not reuse `INTUNE_TENANT_ID` string for `external_id`). + +## Phase 2 — Implementation Planning (maps to `tasks.md`) + +1. Add/adjust helpers for assignment job identity derivation and job-level overlap prevention. +2. Update assignment jobs to persist summary counts on completion (and on terminal failure) with “final attempt” semantics. +3. Update `ReconcileAdapterRunsJob` to be `OperationRun`-tracked (workspace-scoped), with stable failure code(s). +4. Fix `PoliciesSeeder` to generate UUID v4 `external_id` for the seed tenant. +5. Add/adjust Pest tests covering: + - summary persistence for fetch/restore runs + - dedupe/overlap prevention within window + - workspace-scoped tracking for reconcile job + - seed workflow success + UUID `external_id` diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/quickstart.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/quickstart.md new file mode 100644 index 0000000..ec10cc0 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/quickstart.md @@ -0,0 +1,56 @@ +# Quickstart (096 Ops Polish Bundle) + +This spec is background-operations only (no new routes/pages). Verification is via Pest tests and seed workflows. + +## Prereqs + +- Start Sail: `vendor/bin/sail up -d` + +## Run targeted tests (recommended during implementation) + +- Run a single spec-focused file (once created): + - `vendor/bin/sail artisan test --compact tests/Feature/OpsPolishBundleTest.php` + +- Or run by filter: + - `vendor/bin/sail artisan test --compact --filter=assignment` + - `vendor/bin/sail artisan test --compact --filter=dedupe` + - `vendor/bin/sail artisan test --compact --filter=reconcile` + +## Verify seeding DX + +- Reset + seed: + - `vendor/bin/sail artisan migrate:fresh --seed --no-interaction` + +- Expectation: + - Seed completes without DB constraint errors. + - Seeded tenant has `external_id` formatted as UUID v4. + +## Formatting + +- Run Pint on changed files: + - `vendor/bin/sail bin pint --dirty` + +## Strategic Health Audit (evidence-based checklist) + +This repo does not have a single “audit” command. For Spec 096 we treat the audit as a small bundle of objective gates. + +### Gate 1 — Full suite + +- `vendor/bin/sail artisan test --compact` + +### Gate 2 — PostgreSQL schema/constraints suite + +- `vendor/bin/sail composer test:pgsql` + +### Gate 3 — Seed DX + +- `vendor/bin/sail artisan migrate:fresh --seed --no-interaction` + +**Pass criteria:** all 3 gates succeed. + +### Evidence links (tests) + +- A (summary persistence): `tests/Feature/Operations/AssignmentRunSummaryCountsTest.php` +- B (dedupe + cooldown): `tests/Feature/Operations/AssignmentJobDedupeTest.php` +- C (reconcile tracking + overlap): `tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php` +- D (seed UUID + DX): `tests/Feature/Seed/PoliciesSeederExternalIdTest.php` diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/research.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/research.md new file mode 100644 index 0000000..77783a3 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/research.md @@ -0,0 +1,71 @@ +# Phase 0 — Research (096 Ops Polish Bundle) + +This feature is an operations / background-job hardening pass. The design intentionally reuses existing run observability and dedupe primitives already present in the codebase. + +## Decision 1 — Use `OperationRunService` + DB unique indexes for dedupe + +**Decision:** Use `OperationRunService::ensureRunWithIdentity(...)` (tenant-scoped) and `OperationRunService::ensureWorkspaceRunWithIdentity(...)` (workspace-scoped) as the canonical dedupe mechanism, backed by the existing partial unique indexes for active runs. + +**Rationale:** +- The repo already enforces “active-run dedupe MUST be enforced at DB level” via partial unique indexes and the `ensureRun*` helpers. +- DB enforcement remains correct under concurrency and across multiple workers. +- Keeps the single source of truth in `operation_runs` (Monitoring → Operations) rather than adding a second dedupe store. + +**Alternatives considered:** +- Laravel job uniqueness (e.g., `ShouldBeUnique` / cache lock) — rejected because it introduces a second dedupe primitive outside the canonical `OperationRun` ledger and may behave differently across environments. +- Scheduler-level overlap prevention only — rejected because the spec requires dedupe at execution time and must handle duplicate dispatch / redelivery. + +## Decision 2 — Dedupe identity rule (per spec clarifications) + +**Decision:** Derive job identity as: +- Prefer `operation_run_id` when available. +- Otherwise `tenant_id + job_type + stable input fingerprint`. + +**Rationale:** +- `operation_run_id` is already stable and non-secret. +- Fallback fingerprint avoids secrets, stays deterministic, and is suitable for both logging and DB identity hashing. + +**Alternatives considered:** +- Fingerprinting full payloads — rejected to avoid secrets/PII and to keep dedupe stable even if non-essential context changes. + +## Decision 3 — Enforce dedupe at execute time (not just dispatch) + +**Decision:** Add an execution-time guard so a job skips early when it is not the canonical active run for its identity. + +**Rationale:** +- Covers duplicate job dispatch/redelivery even if a caller fails to reuse the same `OperationRun` at dispatch time. +- Aligns with spec FR-006 and the constitution’s “queued/scheduled ops use locks + idempotency”. + +**Alternatives considered:** +- Rely on dispatch-only dedupe — rejected because duplicate jobs can still be enqueued and run concurrently. + +## Decision 4 — Summary counters use “final attempt wins” semantics + +**Decision:** Persist `OperationRun.summary_counts` at terminal completion by overwriting with normalized counts (final attempt reflects truth; retries do not double count). + +**Rationale:** +- Matches clarified requirement (“final attempt”) and avoids needing cross-attempt reconciliation. +- Fits existing patterns in `OperationRunService::updateRun(...)` which sanitizes/normalizes summary keys. + +**Alternatives considered:** +- Incremental counters (`incrementSummaryCounts`) across attempts — rejected because it risks double-counting under retries unless attempt IDs are tracked. + +## Decision 5 — Housekeeping job tracking is workspace-scoped + +**Decision:** Track `ReconcileAdapterRunsJob` via a workspace-scoped `OperationRun` (`tenant_id = null`) using `type = ops.reconcile_adapter_runs`. + +**Rationale:** +- The job is not tenant-specific and reconciles across runs; workspace-scoped runs are explicitly supported by the schema + service. + +**Alternatives considered:** +- Create one run per tenant — rejected because it would misrepresent the job’s actual unit of work and inflate noise. + +## Decision 6 — Seed tenant external ID must be UUID v4 + +**Decision:** Ensure the seed tenant’s `external_id` is generated as UUID v4 regardless of `INTUNE_TENANT_ID`. + +**Rationale:** +- Matches clarified requirement and avoids coupling a human-readable env value to a UUID-constrained field. + +**Alternatives considered:** +- Reuse `INTUNE_TENANT_ID` for `external_id` — rejected because it is not guaranteed UUID formatted. diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md new file mode 100644 index 0000000..5e6ba07 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md @@ -0,0 +1,143 @@ +# Feature Specification: 096 — Ops Polish Bundle (Assignment job summaries + job dedupe + system job tracking + seeder DX) + +**Feature Branch**: `096-ops-polish-assignment-dedupe-system-tracking` +**Created**: 2026-02-15 +**Status**: Draft +**Input**: User description: "096 — Ops Polish Bundle (Assignment job summaries + job dedupe + system job tracking + seeder DX)" + +## Spec Scope Fields *(mandatory)* + +- **Scope**: tenant +- **Primary Routes**: None (background operations only) +- **Data Ownership**: tenant-owned operational records (run tracking + run outcomes) and tenant seed data +- **RBAC**: No new permissions; no changes to user-facing authorization behavior (existing gates/policies remain the source of truth for starting operations) + +## Clarifications + +### Session 2026-02-15 + +- Q: What is the dedupe identity rule for assignment jobs? → A: Use `operation_run_id` when available; otherwise dedupe by `tenant_id + job_type + stable input fingerprint` (no secrets). +- Q: If an assignment job retries, how should `OperationRun` summary counters be persisted? → A: On completion (success or terminal failure), write/overwrite the run’s counters so they reflect the final attempt. +- Q: What deduplication duration should we enforce for assignment jobs? → A: 15 minutes. +- Q: For seeded tenants, what format should `tenants.external_id` use? → A: UUID string (v4). +- Q: What `OperationRun.type` should we use for `ReconcileAdapterRunsJob` tracking? → A: `ops.reconcile_adapter_runs`. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Assignment runs show durable summaries (Priority: P2) + +As an operator, I want assignment-related background runs to persist consistent summary counters so that run outcomes can be audited reliably (especially under retries). + +**Why this priority**: These jobs already run in production; missing summaries reduce observability and make incident triage slower. + +**Independent Test**: Dispatch one assignment job run, then verify the recorded run summary includes total/processed/failed and remains correct after retry simulation. + +**Acceptance Scenarios**: + +1. **Given** an assignment fetch run completes, **When** its run record is inspected, **Then** it includes non-null summary counters for total, processed, and failed. +2. **Given** an assignment restore run completes, **When** its run record is inspected, **Then** it includes non-null summary counters for total, processed, and failed. +3. **Given** an assignment run retries due to transient failure, **When** it ultimately completes, **Then** its summary counters do not double-count work across attempts. + +--- + +### User Story 2 - Duplicate dispatches do not overlap (Priority: P2) + +As an operator, I want assignment jobs to be deduplicated by identity so accidental double dispatch (or queue redelivery) does not cause concurrent overlapping work. + +**Why this priority**: Duplicate concurrency increases the risk of conflicting writes, rate limiting, and confusing operational outcomes. + +**Independent Test**: Attempt to dispatch the same job identity twice and assert only one execution proceeds while the other is deduped/skipped. + +**Acceptance Scenarios**: + +1. **Given** an assignment job with a stable identity is dispatched twice in a short window, **When** workers attempt to execute both, **Then** only one execution proceeds (no concurrent overlap) for that identity. +2. **Given** an assignment job with a stable identity completed recently, **When** the same identity is dispatched again within the deduplication duration, **Then** the new execution is skipped/deduped. +3. **Given** the dedupe duration has elapsed since the last terminal completion for an identity, **When** the same identity is legitimately run again, **Then** the new execution is allowed to proceed. + +--- + +### User Story 3 - Housekeeping runs are tracked like everything else (Priority: P3) + +As an operator, I want housekeeping/system jobs to produce the same run tracking as other operations so that the operations ledger is complete. + +**Why this priority**: This is operational completeness. It is not ship-blocking but improves consistency and reduces blind spots. + +**Independent Test**: Execute a housekeeping run and verify a run record is created/updated with success/failure outcome details. + +**Acceptance Scenarios**: + +1. **Given** the reconcile-adapter-runs job executes successfully, **When** the operations ledger is inspected, **Then** a run record exists with a success outcome. +2. **Given** the reconcile-adapter-runs job fails, **When** the operations ledger is inspected, **Then** the run record includes a stable reason code and a sanitized error message suitable for operators. + +--- + +### User Story 4 - Fresh seed flows work without manual intervention (Priority: P3) + +As a developer, I want local and CI seed flows to run successfully so that onboarding and test environments are reproducible. + +**Why this priority**: Broken seeding slows development and increases setup variability. + +**Independent Test**: Run a clean database reset with seeding and confirm it completes without constraint errors. + +**Acceptance Scenarios**: + +1. **Given** a clean database, **When** a full reset-and-seed workflow is executed, **Then** it succeeds without requiring manual edits. +2. **Given** seeded tenants exist, **When** their records are validated, **Then** required identifiers are populated, and `external_id` is a UUID v4 string. + +### Edge Cases + +- Retries: If a job attempt fails and retries, summary counters must remain consistent (no double counting). +- Partial failure: If some items fail, the run must record failures without obscuring successful processing. +- Deduplication window: Dedupe should block concurrent overlap but must not prevent legitimate future runs after the window. +- Error reporting: Failure messages must be sanitized and stable enough to support searching and alerting. + +## Requirements *(mandatory)* + +**Constitution alignment (required):** This feature touches queued/background work and run observability. It must: +- maintain tenant isolation (no cross-tenant leakage in run tracking), +- ensure run observability is complete for the jobs in scope, +- and add automated tests for the changed operational behavior. + +**Constitution alignment (RBAC-UX):** No new UI surfaces are added and no authorization behavior is changed. Any existing authorization checks for starting operations remain server-side and unchanged. + +**Constitution alignment (Filament Action Surfaces):** Not applicable (no Filament Resources/Pages/RelationManagers are added or modified). + +### Functional Requirements + +- **FR-001**: The system MUST persist summary counters (total, processed, failed) for assignment fetch runs upon completion. +- **FR-002**: The system MUST persist summary counters (total, processed, failed) for assignment restore runs upon completion. +- **FR-003**: Summary counters MUST be idempotent-friendly: on completion (success or terminal failure), the run summary counters MUST reflect the final attempt and retries MUST NOT double-count totals for the same run identity. +- **FR-004**: Assignment jobs MUST enforce server-side deduplication by a stable, non-secret job identity to prevent concurrent overlap. +- **FR-004a**: The job identity MUST be derived as: `operation_run_id` when available; otherwise `tenant_id + job_type + stable input fingerprint`. +- **FR-004b**: The stable input fingerprint MUST be deterministic and MUST NOT include secrets. +- **FR-005**: The deduplication duration MUST cover expected worst-case runtime while allowing legitimate future executions after it expires. +- **FR-005a**: The deduplication duration MUST be 15 minutes. +- **FR-006**: Deduplication MUST be enforced at execution time (not solely by UI gating or caller-side checks). +- **FR-007**: The reconcile-adapter-runs housekeeping job MUST create/update an operational run record each time it executes. +- **FR-007a**: The reconcile-adapter-runs housekeeping job MUST use `OperationRun.type = ops.reconcile_adapter_runs`. +- **FR-008**: On failure, the housekeeping run record MUST include a stable reason code and a sanitized operator-facing error message. +- **FR-009**: The seed workflow MUST populate required tenant identifiers so database constraints are satisfied. +- **FR-009a**: Seeded tenants MUST have `external_id` populated as a UUID string (v4). +- **FR-010**: A clean reset-and-seed workflow MUST succeed without manual intervention. + +### Assumptions & Dependencies + +- The system already has a durable operations ledger concept (run tracking) that can store outcomes and summary counters. +- Assignment fetch/restore jobs already produce item-level totals (or can derive them) without changing business behavior. +- Dedupe is evaluated based on a stable identity that is safe to store and log (no secrets). +- No new UI, routes, or end-user workflows are introduced by this work. + +### Key Entities *(include if feature involves data)* + +- **Operation Run**: A durable record of a background operation execution, its outcome, and its summary counters. +- **Job Identity**: A stable identifier used to deduplicate concurrent executions of the same logical work. +- **Tenant**: The scope boundary for operational records and seeded data. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: 100% of completed assignment fetch and restore runs show persisted summary counters (total/processed/failed) in the operations ledger. +- **SC-002**: Duplicate dispatch attempts for the same assignment job identity result in at most one concurrent execution within the deduplication window. +- **SC-003**: 100% of reconcile-adapter-runs executions produce an operational run record with a success or failure outcome. +- **SC-004**: A clean reset-and-seed workflow completes successfully in CI and locally without database constraint failures. diff --git a/specs/096-ops-polish-assignment-dedupe-system-tracking/tasks.md b/specs/096-ops-polish-assignment-dedupe-system-tracking/tasks.md new file mode 100644 index 0000000..cd0be68 --- /dev/null +++ b/specs/096-ops-polish-assignment-dedupe-system-tracking/tasks.md @@ -0,0 +1,177 @@ +# Tasks: 096 — Ops Polish Bundle (Assignment job summaries + job dedupe + system job tracking + seeder DX) + +**Input**: Design documents from `specs/096-ops-polish-assignment-dedupe-system-tracking/` +**Prerequisites**: `plan.md` (required), `spec.md` (required), `research.md`, `data-model.md`, `contracts/`, `quickstart.md` + +**Tests**: REQUIRED (Pest) for runtime behavior changes. +**Operations**: This feature modifies queued/scheduled work, so tasks must ensure canonical `OperationRun` creation/reuse + DB-level active-run dedupe + safe failure summaries. +**RBAC / Filament**: No authorization or Filament UI changes in scope. + +--- + +## Phase 1: Setup (Docs + hygiene) + +**Purpose**: Ensure spec artifacts and plan alignment are clean before code work. + +- [X] T001 Fix duplicated FR numbering in specs/096-ops-polish-assignment-dedupe-system-tracking/spec.md +- [X] T002 Sync plan references after edits in specs/096-ops-polish-assignment-dedupe-system-tracking/plan.md (ensure dedupe window semantics are described as a cooldown) + +--- + +## Phase 2: Foundational (Blocking prerequisites) + +**Purpose**: Shared primitives used by all user stories. + +- [X] T003 Add operation label for `ops.reconcile_adapter_runs` in app/Support/OperationCatalog.php +- [X] T004 Add expected duration for `ops.reconcile_adapter_runs` in app/Support/OperationCatalog.php +- [X] T005 Do not add an enum case for `ops.reconcile_adapter_runs` in app/Support/OperationRunType.php (use the string type; enum is not a complete registry) +- [X] T006 Implement a stable, non-secret fingerprint helper for assignment job identity in app/Support/OpsUx/AssignmentJobFingerprint.php +- [X] T007 [P] Wire FetchAssignmentsJob identity inputs to use AssignmentJobFingerprint in app/Jobs/FetchAssignmentsJob.php +- [X] T008 [P] Wire RestoreAssignmentsJob identity inputs to use AssignmentJobFingerprint in app/Jobs/RestoreAssignmentsJob.php + +**Checkpoint**: Foundation ready — story implementation can begin. + +--- + +## Phase 3: User Story 1 — Assignment runs show durable summaries (Priority: P2) 🎯 MVP + +**Goal**: Persist consistent `OperationRun.summary_counts` for assignment fetch/restore runs (final attempt wins). + +**Independent Test**: Dispatch each job; assert `operation_runs.summary_counts` contains non-null `total`, `processed`, `failed`, and does not double-count across retries. + +### Tests (write first) + +- [X] T009 [P] [US1] Add fetch summary persistence test in tests/Feature/Operations/AssignmentRunSummaryCountsTest.php +- [X] T010 [P] [US1] Add restore summary persistence test in tests/Feature/Operations/AssignmentRunSummaryCountsTest.php + +### Implementation + +- [X] T011 [US1] Persist terminal summary counts for fetch runs in app/Jobs/FetchAssignmentsJob.php via app/Services/OperationRunService.php::updateRun(...) +- [X] T012 [US1] Persist terminal summary counts for restore runs in app/Jobs/RestoreAssignmentsJob.php via app/Services/OperationRunService.php::updateRun(...) +- [X] T013 [US1] Ensure summary counts reflect final attempt (overwrite on completion) in app/Services/OperationRunService.php usage from app/Jobs/FetchAssignmentsJob.php +- [X] T014 [US1] Ensure summary counts reflect final attempt (overwrite on completion) in app/Services/OperationRunService.php usage from app/Jobs/RestoreAssignmentsJob.php + +**Checkpoint**: Assignment runs always write durable counters. + +--- + +## Phase 4: User Story 2 — Duplicate dispatches do not overlap (Priority: P2) + +**Goal**: Prevent concurrent overlap for the same logical assignment job identity. + +**Independent Test**: Dispatch the same job identity twice and verify only one execution performs work; the other is skipped/reuses the canonical active run. + +### Tests (write first) + +- [X] T015 [P] [US2] Add dedupe test for fetch job in tests/Feature/Operations/AssignmentJobDedupeTest.php +- [X] T016 [P] [US2] Add dedupe test for restore job in tests/Feature/Operations/AssignmentJobDedupeTest.php + +### Implementation + +- [X] T017 [US2] Implement dispatch-time dedupe (reuse active `OperationRun`) for fetch in app/Jobs/FetchAssignmentsJob.php using app/Services/OperationRunService.php::ensureRunWithIdentity(...) +- [X] T018 [US2] Implement dispatch-time dedupe (reuse active `OperationRun`) for restore in app/Jobs/RestoreAssignmentsJob.php using app/Services/OperationRunService.php::ensureRunWithIdentity(...) +- [X] T019 [US2] Implement execute-time guard (skip when not canonical run) for fetch in app/Jobs/FetchAssignmentsJob.php +- [X] T020 [US2] Implement execute-time guard (skip when not canonical run) for restore in app/Jobs/RestoreAssignmentsJob.php +- [X] T021 [US2] Enforce dedupe window semantics (15 minutes) by reusing a recently completed identity (cooldown) in app/Services/OperationRunService.php (new helper) and call it from app/Jobs/FetchAssignmentsJob.php +- [X] T022 [US2] Enforce dedupe window semantics (15 minutes) by reusing a recently completed identity (cooldown) in app/Services/OperationRunService.php (new helper) and call it from app/Jobs/RestoreAssignmentsJob.php + +**Checkpoint**: Duplicate dispatch/redelivery cannot cause overlapping work. + +--- + +## Phase 5: User Story 3 — Housekeeping runs are tracked like everything else (Priority: P3) + +**Goal**: `ReconcileAdapterRunsJob` writes a workspace-scoped `OperationRun` with stable failure codes + sanitized messages. + +**Independent Test**: Execute the job successfully and with a forced failure; verify an `OperationRun` exists for `type = ops.reconcile_adapter_runs` with correct outcome and failure summary. + +### Tests (write first) + +- [X] T023 [P] [US3] Add reconcile job success tracking test in tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php +- [X] T024 [P] [US3] Add reconcile job failure tracking test in tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php + +### Implementation + +- [X] T025 [US3] Create/reuse workspace-scoped run for housekeeping in app/Jobs/ReconcileAdapterRunsJob.php via app/Services/OperationRunService.php::ensureWorkspaceRunWithIdentity(...) +- [X] T026 [US3] Persist terminal outcome + sanitized failure summary for housekeeping in app/Jobs/ReconcileAdapterRunsJob.php using app/Services/OperationRunService.php::updateRun(...) +- [X] T027 [US3] Include stable failure code (e.g., `ops.reconcile_adapter_runs.failed`) for housekeeping failures in app/Jobs/ReconcileAdapterRunsJob.php + +**Checkpoint**: Housekeeping shows up in the operations ledger consistently. + +--- + +## Phase 6: User Story 4 — Fresh seed flows work without manual intervention (Priority: P3) + +**Goal**: Seeding creates tenants with UUID v4 `external_id` and does not violate constraints. + +**Independent Test**: Run `migrate:fresh --seed` and assert the seeded tenant(s) have UUID v4 `external_id`. + +### Tests (write first) + +- [X] T028 [P] [US4] Add seed external_id UUID test in tests/Feature/Seed/PoliciesSeederExternalIdTest.php + +### Implementation + +- [X] T029 [US4] Generate UUID v4 `external_id` for seed tenant in database/seeders/PoliciesSeeder.php (do not reuse `INTUNE_TENANT_ID`) +- [X] T034 [US4] Ensure seed workflow sets `workspace_id` for seeded tenant + policies to satisfy NOT NULL constraints in database/seeders/PoliciesSeeder.php +- [X] T035 [US4] Ensure platform tenant seed sets `workspace_id` (default workspace) in database/seeders/PlatformUserSeeder.php + +**Checkpoint**: Seed workflow is reliable locally and in CI. + +--- + +## Phase 7: Polish & Cross-Cutting Concerns + +**Purpose**: Consistency, formatting, and verification. + +- [X] T030 Run formatter on changed files (app/**, database/**, tests/**) via `vendor/bin/sail bin pint --dirty` +- [X] T031 Run spec-focused tests in tests/Feature/Operations/AssignmentRunSummaryCountsTest.php and tests/Feature/Operations/AssignmentJobDedupeTest.php +- [X] T032 Run spec-focused tests in tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php and tests/Feature/Seed/PoliciesSeederExternalIdTest.php + +--- + +## Addendum: Constitution-required idempotency (queued/scheduled jobs) + +- [X] T033 [US3] Add overlap-prevention for ReconcileAdapterRunsJob so concurrent dispatch/schedule overlap cannot execute twice (lock/guard must be server-side) + +--- + +## Dependencies & Execution Order + +### User Story completion order + +- US1 (P2) → US2 (P2) → US3 (P3) → US4 (P3) + +### Why + +- US1 establishes durable counters used by ops. +- US2 dedupe builds on the same identity/counter plumbing. +- US3 and US4 are independent after foundational primitives. + +--- + +## Parallel execution examples + +### US1 + +- T009 and T010 can run in parallel (same file, but split by responsibility). + +### US2 + +- T015 and T016 can run in parallel. +- T019 and T020 can run in parallel. + +### US3 + +- T023 and T024 can run in parallel. + +### US4 + +- T028 can run in parallel with US3 implementation tasks. + +--- + +## Implementation strategy (MVP) + +- MVP scope: US1 only (Phase 3) after completing Phases 1–2. +- After MVP validation: implement US2 next to reduce operational risk. diff --git a/tests/Feature/Database/PoliciesSeederTest.php b/tests/Feature/Database/PoliciesSeederTest.php index befa69c..9cb3071 100644 --- a/tests/Feature/Database/PoliciesSeederTest.php +++ b/tests/Feature/Database/PoliciesSeederTest.php @@ -2,6 +2,7 @@ use App\Models\Tenant; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Str; uses(RefreshDatabase::class); @@ -19,5 +20,5 @@ expect($tenant)->not->toBeNull(); expect($tenant->tenant_id)->toBe('test-tenant-id'); - expect($tenant->external_id)->toBe('test-tenant-id'); + expect(Str::isUuid((string) $tenant->external_id))->toBeTrue(); }); diff --git a/tests/Feature/Filament/TenantRbacWizardTest.php b/tests/Feature/Filament/TenantRbacWizardTest.php index 479184e..46c1862 100644 --- a/tests/Feature/Filament/TenantRbacWizardTest.php +++ b/tests/Feature/Filament/TenantRbacWizardTest.php @@ -106,6 +106,8 @@ public function getServicePrincipalPermissions(array $options = []): GraphRespon public function request(string $method, string $path, array $options = []): GraphResponse { + $path = ltrim($path, '/'); + $filter = $options['query']['$filter'] ?? ''; if ($method === 'GET' && $path === 'servicePrincipals') { @@ -211,6 +213,8 @@ public function getServicePrincipalPermissions(array $options = []): GraphRespon public function request(string $method, string $path, array $options = []): GraphResponse { + $path = ltrim($path, '/'); + if ($method === 'GET' && $path === 'servicePrincipals') { return new GraphResponse(true, ['value' => [['id' => 'sp-1']]]); } @@ -322,6 +326,8 @@ public function getServicePrincipalPermissions(array $options = []): GraphRespon public function request(string $method, string $path, array $options = []): GraphResponse { + $path = ltrim($path, '/'); + if ($method === 'GET' && $path === 'servicePrincipals') { return new GraphResponse(true, ['value' => [['id' => 'sp-1']]]); } @@ -454,6 +460,8 @@ public function getServicePrincipalPermissions(array $options = []): GraphRespon public function request(string $method, string $path, array $options = []): GraphResponse { + $path = ltrim($path, '/'); + $filter = $options['query']['$filter'] ?? ''; if ($method === 'GET' && $path === 'servicePrincipals') { diff --git a/tests/Feature/Operations/AssignmentJobDedupeTest.php b/tests/Feature/Operations/AssignmentJobDedupeTest.php new file mode 100644 index 0000000..3601d29 --- /dev/null +++ b/tests/Feature/Operations/AssignmentJobDedupeTest.php @@ -0,0 +1,182 @@ +create(); + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_type' => 'settingsCatalogPolicy', + 'policy_identifier' => 'policy-fetch-dedupe', + 'metadata' => [], + ]); + + Bus::fake(); + + $firstRun = FetchAssignmentsJob::dispatchTracked( + backupItem: $backupItem, + policyPayload: ['id' => 'policy-fetch-dedupe'], + ); + + $secondRun = FetchAssignmentsJob::dispatchTracked( + backupItem: $backupItem, + policyPayload: ['id' => 'policy-fetch-dedupe'], + ); + + expect((int) $firstRun->getKey())->toBe((int) $secondRun->getKey()); + Bus::assertDispatched(FetchAssignmentsJob::class, 1); + + $assignmentBackupService = \Mockery::mock(AssignmentBackupService::class); + $assignmentBackupService->shouldReceive('enrichWithAssignments') + ->once() + ->andReturnUsing(function (BackupItem $item): BackupItem { + $metadata = is_array($item->metadata) ? $item->metadata : []; + $metadata['assignments_fetch_failed'] = false; + + $item->update([ + 'metadata' => $metadata, + 'assignments' => [['id' => 'assignment-fetch-1']], + ]); + + return $item->refresh(); + }); + + $firstJob = new FetchAssignmentsJob( + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: (string) $tenant->external_id, + policyExternalId: (string) $backupItem->policy_identifier, + policyPayload: ['id' => 'policy-fetch-dedupe'], + operationRun: $firstRun, + ); + + $duplicateJob = new FetchAssignmentsJob( + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: (string) $tenant->external_id, + policyExternalId: (string) $backupItem->policy_identifier, + policyPayload: ['id' => 'policy-fetch-dedupe'], + operationRun: $firstRun, + ); + + $firstJob->handle($assignmentBackupService, app(OperationRunService::class)); + $duplicateJob->handle($assignmentBackupService, app(OperationRunService::class)); + + Bus::fake(); + + $cooldownRun = FetchAssignmentsJob::dispatchTracked( + backupItem: $backupItem, + policyPayload: ['id' => 'policy-fetch-dedupe'], + ); + + expect((int) $cooldownRun->getKey())->toBe((int) $firstRun->getKey()); + Bus::assertNotDispatched(FetchAssignmentsJob::class); +}); + +test('restore assignment job dedupes dispatch and execution, and reuses recent completion during cooldown', function (): void { + $tenant = Tenant::factory()->create(); + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + ]); + + $assignments = [ + ['id' => 'assignment-1'], + ['id' => 'assignment-2'], + ]; + + Bus::fake(); + + $firstRun = RestoreAssignmentsJob::dispatchTracked( + restoreRunId: (int) $restoreRun->getKey(), + tenant: $tenant, + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-dedupe', + assignments: $assignments, + groupMapping: [], + ); + + $secondRun = RestoreAssignmentsJob::dispatchTracked( + restoreRunId: (int) $restoreRun->getKey(), + tenant: $tenant, + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-dedupe', + assignments: $assignments, + groupMapping: [], + ); + + expect((int) $firstRun->getKey())->toBe((int) $secondRun->getKey()); + Bus::assertDispatched(RestoreAssignmentsJob::class, 1); + + $assignmentRestoreService = \Mockery::mock(AssignmentRestoreService::class); + $assignmentRestoreService->shouldReceive('restore') + ->once() + ->andReturn([ + 'outcomes' => [ + ['status' => 'success'], + ['status' => 'success'], + ], + 'summary' => [ + 'success' => 2, + 'failed' => 0, + 'skipped' => 0, + ], + ]); + + $firstJob = new RestoreAssignmentsJob( + restoreRunId: (int) $restoreRun->getKey(), + tenantId: (int) $tenant->getKey(), + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-dedupe', + assignments: $assignments, + groupMapping: [], + foundationMapping: [], + operationRun: $firstRun, + ); + + $duplicateJob = new RestoreAssignmentsJob( + restoreRunId: (int) $restoreRun->getKey(), + tenantId: (int) $tenant->getKey(), + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-dedupe', + assignments: $assignments, + groupMapping: [], + foundationMapping: [], + operationRun: $firstRun, + ); + + $firstJob->handle($assignmentRestoreService, app(OperationRunService::class)); + $duplicateJob->handle($assignmentRestoreService, app(OperationRunService::class)); + + Bus::fake(); + + $cooldownRun = RestoreAssignmentsJob::dispatchTracked( + restoreRunId: (int) $restoreRun->getKey(), + tenant: $tenant, + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-dedupe', + assignments: $assignments, + groupMapping: [], + ); + + expect((int) $cooldownRun->getKey())->toBe((int) $firstRun->getKey()); + Bus::assertNotDispatched(RestoreAssignmentsJob::class); +}); diff --git a/tests/Feature/Operations/AssignmentRunSummaryCountsTest.php b/tests/Feature/Operations/AssignmentRunSummaryCountsTest.php new file mode 100644 index 0000000..8efd948 --- /dev/null +++ b/tests/Feature/Operations/AssignmentRunSummaryCountsTest.php @@ -0,0 +1,146 @@ +create(); + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_type' => 'settingsCatalogPolicy', + 'policy_identifier' => 'policy-fetch-summary', + 'metadata' => [], + ]); + + $operationRun = OperationRun::factory()->for($tenant)->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'assignments.fetch', + 'status' => 'queued', + 'outcome' => 'pending', + 'summary_counts' => [ + 'total' => 99, + 'processed' => 99, + 'failed' => 99, + ], + ]); + + $assignmentBackupService = \Mockery::mock(AssignmentBackupService::class); + $assignmentBackupService->shouldReceive('enrichWithAssignments') + ->once() + ->andReturnUsing(function (BackupItem $item): BackupItem { + $metadata = is_array($item->metadata) ? $item->metadata : []; + $metadata['assignments_fetch_failed'] = false; + + $item->update([ + 'metadata' => $metadata, + 'assignments' => [ + ['id' => 'assignment-1'], + ], + ]); + + return $item->refresh(); + }); + + $job = new FetchAssignmentsJob( + backupItemId: (int) $backupItem->getKey(), + tenantExternalId: (string) $tenant->external_id, + policyExternalId: (string) $backupItem->policy_identifier, + policyPayload: ['id' => (string) $backupItem->policy_identifier], + operationRun: $operationRun, + ); + + $job->handle($assignmentBackupService, app(OperationRunService::class)); + + $operationRun->refresh(); + + expect($operationRun->status)->toBe('completed') + ->and($operationRun->outcome)->toBe('succeeded') + ->and($operationRun->summary_counts)->toMatchArray([ + 'total' => 1, + 'processed' => 1, + 'failed' => 0, + ]); +}); + +test('restore assignment job persists terminal summary counts and overwrites previous values', function (): void { + $tenant = Tenant::factory()->create(); + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + ]); + + $operationRun = OperationRun::factory()->for($tenant)->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'assignments.restore', + 'status' => 'queued', + 'outcome' => 'pending', + 'summary_counts' => [ + 'total' => 88, + 'processed' => 88, + 'failed' => 88, + ], + ]); + + $assignmentRestoreService = \Mockery::mock(AssignmentRestoreService::class); + $assignmentRestoreService->shouldReceive('restore') + ->once() + ->andReturn([ + 'outcomes' => [ + ['status' => 'success'], + ['status' => 'success'], + ['status' => 'failed', 'reason' => 'Graph request failed'], + ], + 'summary' => [ + 'success' => 2, + 'failed' => 1, + 'skipped' => 0, + ], + ]); + + $job = new RestoreAssignmentsJob( + restoreRunId: (int) $restoreRun->getKey(), + tenantId: (int) $tenant->getKey(), + policyType: 'settingsCatalogPolicy', + policyId: 'policy-restore-summary', + assignments: [ + ['id' => 'assignment-1'], + ['id' => 'assignment-2'], + ['id' => 'assignment-3'], + ], + groupMapping: [], + foundationMapping: [], + operationRun: $operationRun, + ); + + $job->handle($assignmentRestoreService, app(OperationRunService::class)); + + $operationRun->refresh(); + + expect($operationRun->status)->toBe('completed') + ->and($operationRun->outcome)->toBe('partially_succeeded') + ->and($operationRun->summary_counts)->toMatchArray([ + 'total' => 3, + 'processed' => 3, + 'failed' => 1, + ]); +}); diff --git a/tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php b/tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php new file mode 100644 index 0000000..2859562 --- /dev/null +++ b/tests/Feature/Operations/ReconcileAdapterRunsJobTrackingTest.php @@ -0,0 +1,84 @@ +create(); + + $job = new class((int) $workspace->getKey(), '2026-02-15 10:00:00') extends ReconcileAdapterRunsJob + { + protected function reconcile(AdapterRunReconciler $reconciler): array + { + return [ + 'candidates' => 4, + 'reconciled' => 3, + 'skipped' => 1, + ]; + } + }; + + $job->handle(new AdapterRunReconciler, app(OperationRunService::class)); + + $run = OperationRun::query() + ->where('workspace_id', (int) $workspace->getKey()) + ->whereNull('tenant_id') + ->where('type', 'ops.reconcile_adapter_runs') + ->first(); + + expect($run)->not->toBeNull(); + expect($run?->status)->toBe('completed'); + expect($run?->outcome)->toBe('succeeded'); + expect($run?->summary_counts ?? [])->toMatchArray([ + 'total' => 4, + 'processed' => 4, + 'failed' => 0, + ]); +}); + +test('reconcile adapter runs job tracks failure with stable code and sanitized message', function (): void { + $workspace = Workspace::factory()->create(); + + $job = new class((int) $workspace->getKey(), '2026-02-15 10:30:00') extends ReconcileAdapterRunsJob + { + protected function reconcile(AdapterRunReconciler $reconciler): array + { + throw new RuntimeException('Authorization: Bearer highly-sensitive-token-for-user@example.com'); + } + }; + + expect(fn () => $job->handle(new AdapterRunReconciler, app(OperationRunService::class))) + ->toThrow(\RuntimeException::class); + + $run = OperationRun::query() + ->where('workspace_id', (int) $workspace->getKey()) + ->whereNull('tenant_id') + ->where('type', 'ops.reconcile_adapter_runs') + ->first(); + + expect($run)->not->toBeNull(); + expect($run?->status)->toBe('completed'); + expect($run?->outcome)->toBe('failed'); + + $failure = $run?->failure_summary[0] ?? []; + + expect($failure['code'] ?? null)->toBe('ops.reconcile_adapter_runs.failed'); + expect((string) ($failure['message'] ?? ''))->not->toContain('Bearer'); + expect((string) ($failure['message'] ?? ''))->not->toContain('@example.com'); +}); + +test('reconcile adapter runs job enforces server-side overlap middleware', function (): void { + $job = new ReconcileAdapterRunsJob; + + $hasWithoutOverlapping = collect($job->middleware()) + ->contains(fn (mixed $middleware): bool => $middleware instanceof WithoutOverlapping); + + expect($hasWithoutOverlapping)->toBeTrue(); +}); diff --git a/tests/Feature/Seed/PoliciesSeederExternalIdTest.php b/tests/Feature/Seed/PoliciesSeederExternalIdTest.php new file mode 100644 index 0000000..a783651 --- /dev/null +++ b/tests/Feature/Seed/PoliciesSeederExternalIdTest.php @@ -0,0 +1,29 @@ +set('tenantpilot.supported_policy_types', []); + + $_ENV['INTUNE_TENANT_ID'] = 'seed-tenant-id'; + $_SERVER['INTUNE_TENANT_ID'] = 'seed-tenant-id'; + putenv('INTUNE_TENANT_ID=seed-tenant-id'); + + $this->artisan('db:seed', ['--class' => Database\Seeders\PoliciesSeeder::class]) + ->assertExitCode(0); + + $tenant = Tenant::query()->first(); + + expect($tenant)->not->toBeNull(); + expect($tenant?->tenant_id)->toBe('seed-tenant-id'); + + $externalId = (string) ($tenant?->external_id ?? ''); + + expect(Str::isUuid($externalId))->toBeTrue(); + expect(substr($externalId, 14, 1))->toBe('4'); + expect(in_array(strtolower(substr($externalId, 19, 1)), ['8', '9', 'a', 'b'], true))->toBeTrue(); +}); diff --git a/tests/Unit/RbacOnboardingServiceTest.php b/tests/Unit/RbacOnboardingServiceTest.php index 06e8054..c770c6f 100644 --- a/tests/Unit/RbacOnboardingServiceTest.php +++ b/tests/Unit/RbacOnboardingServiceTest.php @@ -49,6 +49,8 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -91,7 +93,9 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['POST', 'groups/group-1/members/$ref'] => new GraphResponse(false, [], 400, [['message' => 'One or more added object references already exist']]), @@ -147,7 +151,9 @@ function fakeTenant(): Tenant $message = 'HTTP request returned status code 400: {"error":{"code":"Request_BadRequest","message":"One or more added object references already exist for the following modified properties: \'members\'."}}'; - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) use ($message) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) use ($message) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['POST', 'groups/group-1/members/$ref'] => new GraphResponse(false, [], 400, [$message]), @@ -194,6 +200,8 @@ function fakeTenant(): Tenant ]; $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) use ($groupId, $servicePrincipalId, $error) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => $servicePrincipalId]]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -262,7 +270,9 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -299,7 +309,9 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -335,7 +347,9 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -383,6 +397,8 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) use (&$checked) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]), @@ -426,7 +442,9 @@ function fakeTenant(): Tenant $graph = \Mockery::mock(GraphClientInterface::class); - $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path) { + $graph->shouldReceive('request')->andReturnUsing(function (string $method, string $path, array $options = []) { + $path = ltrim($path, '/'); + return match ([$method, $path]) { ['GET', 'servicePrincipals'] => new GraphResponse(true, ['value' => [['id' => 'sp-1']]]), ['GET', 'groups'] => new GraphResponse(true, ['value' => []]),