feat: always capture policy when adding to backup

Admin-first: always run orchestrated capture when adding policies to a backup set so backups reflect current Intune state. Still avoids redundant PolicyVersion growth via orchestrator snapshot-hash reuse. Adds feature tests and updates spec/plan/tasks.
This commit is contained in:
Ahmed Darrazi 2026-01-02 15:32:00 +01:00
parent 76e10fc404
commit 1145e45fb9
6 changed files with 262 additions and 16 deletions

View File

@ -5,6 +5,7 @@
use App\Models\BackupItem; use App\Models\BackupItem;
use App\Models\BackupSet; use App\Models\BackupSet;
use App\Models\Policy; use App\Models\Policy;
use App\Models\PolicyVersion;
use App\Models\Tenant; use App\Models\Tenant;
use App\Services\AssignmentBackupService; use App\Services\AssignmentBackupService;
use Carbon\CarbonImmutable; use Carbon\CarbonImmutable;
@ -289,13 +290,46 @@ private function snapshotPolicy(
$captured = $captureResult['captured']; $captured = $captureResult['captured'];
$payload = $captured['payload']; $payload = $captured['payload'];
$metadata = $captured['metadata'] ?? []; $metadata = $captured['metadata'] ?? [];
$metadataWarnings = $captured['warnings'] ?? [];
// Validate snapshot return [
$validation = $this->snapshotValidator->validate(is_array($payload) ? $payload : []); $this->createBackupItemFromVersion(
tenant: $tenant,
backupSet: $backupSet,
policy: $policy,
version: $version,
payload: is_array($payload) ? $payload : [],
assignments: $captured['assignments'] ?? null,
scopeTags: $captured['scope_tags'] ?? null,
metadata: is_array($metadata) ? $metadata : [],
warnings: $captured['warnings'] ?? [],
),
null,
];
}
/**
* @param array<string, mixed> $payload
* @param array<string, mixed> $metadata
* @param array<int, string> $warnings
* @param array{ids:array<int, string>,names:array<int, string>}|null $scopeTags
*/
private function createBackupItemFromVersion(
Tenant $tenant,
BackupSet $backupSet,
Policy $policy,
PolicyVersion $version,
array $payload,
?array $assignments,
?array $scopeTags,
array $metadata,
array $warnings = [],
): BackupItem {
$metadataWarnings = $warnings;
$validation = $this->snapshotValidator->validate($payload);
$metadataWarnings = array_merge($metadataWarnings, $validation['warnings']); $metadataWarnings = array_merge($metadataWarnings, $validation['warnings']);
$odataWarning = BackupItem::odataTypeWarning(is_array($payload) ? $payload : [], $policy->policy_type, $policy->platform); $odataWarning = BackupItem::odataTypeWarning($payload, $policy->policy_type, $policy->platform);
if ($odataWarning) { if ($odataWarning) {
$metadataWarnings[] = $odataWarning; $metadataWarnings[] = $odataWarning;
@ -305,29 +339,23 @@ private function snapshotPolicy(
$metadata['warnings'] = array_values(array_unique($metadataWarnings)); $metadata['warnings'] = array_values(array_unique($metadataWarnings));
} }
$capturedScopeTags = $captured['scope_tags'] ?? null; if (is_array($scopeTags)) {
if (is_array($capturedScopeTags)) { $metadata['scope_tag_ids'] = $scopeTags['ids'] ?? null;
$metadata['scope_tag_ids'] = $capturedScopeTags['ids'] ?? null; $metadata['scope_tag_names'] = $scopeTags['names'] ?? null;
$metadata['scope_tag_names'] = $capturedScopeTags['names'] ?? null;
} }
// Create BackupItem as a copy/reference of the PolicyVersion return BackupItem::create([
$backupItem = BackupItem::create([
'tenant_id' => $tenant->id, 'tenant_id' => $tenant->id,
'backup_set_id' => $backupSet->id, 'backup_set_id' => $backupSet->id,
'policy_id' => $policy->id, 'policy_id' => $policy->id,
'policy_version_id' => $version->id, // Link to version 'policy_version_id' => $version->id,
'policy_identifier' => $policy->external_id, 'policy_identifier' => $policy->external_id,
'policy_type' => $policy->policy_type, 'policy_type' => $policy->policy_type,
'platform' => $policy->platform, 'platform' => $policy->platform,
'payload' => $payload, 'payload' => $payload,
'metadata' => $metadata, 'metadata' => $metadata,
// Copy assignments from version (already captured) 'assignments' => $assignments,
// Note: scope_tags are only stored in PolicyVersion
'assignments' => $captured['assignments'] ?? null,
]); ]);
return [$backupItem, null];
} }
/** /**

View File

@ -0,0 +1,9 @@
# Specification Quality Checklist: Backup Version Reuse
**Created**: 2026-01-02
**Feature**: [spec.md](../spec.md)
- [x] User story and acceptance scenarios defined
- [x] Requirements are testable and unambiguous
- [x] Scope bounded
- [x] No implementation details required by spec

View File

@ -0,0 +1,18 @@
# Plan: Backup Version Reuse (016)
**Branch**: `016-backup-version-reuse`
**Date**: 2026-01-02
**Input**: [spec.md](./spec.md)
## Goal
Reduce unnecessary `PolicyVersion` creation when policies are added to backup sets by reusing an existing suitable latest version where safe.
## Approach
1. Always capture from Intune when a policy is added to a backup set (admin expectation: "backup = current state").
2. Rely on `PolicyCaptureOrchestrator` snapshot-hash reuse to avoid redundant `PolicyVersion` creation when nothing changed.
3. Still respect capture options (assignments / scope tags) via orchestrator backfill behavior.
4. Add tests for both reuse and capture paths.
## Out of scope
- UI toggles/config flags unless required.
- Cross-policy dedup or historical compaction.

View File

@ -0,0 +1,29 @@
# Feature Specification: Backup Version Reuse (016)
**Feature Branch**: `016-backup-version-reuse`
**Created**: 2026-01-02
**Status**: Draft
## User Scenarios & Testing
### User Story 1 — Avoid unnecessary version growth (Priority: P1)
As an admin, I want adding policies to a backup set to reuse an existing recent policy version when safe, so backups dont create redundant versions and operations stay fast.
**Acceptance Scenarios**
1. Given a policy already has an identical captured snapshot, when I add it to a backup set, then the backup item links to the existing version (no new version is created).
2. Given a policy has no suitable version, when I add it to a backup set, then a new version is captured and linked.
## Requirements
### Functional Requirements
- **FR-001**: Adding policies to a backup set SHOULD avoid creating redundant `PolicyVersion` records by reusing an existing version when the captured snapshot is unchanged.
- **FR-002**: If reuse is not safe/possible, the system MUST capture a new `PolicyVersion` as it does today.
- **FR-003**: Reuse MUST respect capture options:
- If assignments are requested, the reused version must include assignments.
- If scope tags are requested, the reused version must include scope tags.
- **FR-005**: Adding a policy to a backup set MUST capture from Intune to ensure the backup reflects the current state.
- **FR-004**: Behavior changes MUST be covered by automated tests.
## Success Criteria
- **SC-001**: Backups avoid creating redundant policy versions in the common case.
- **SC-002**: Backup correctness is preserved (no missing required data for restore/preview).

View File

@ -0,0 +1,18 @@
# Tasks: Backup Version Reuse (016)
**Branch**: `016-backup-version-reuse` | **Date**: 2026-01-02
**Input**: [spec.md](./spec.md), [plan.md](./plan.md)
## Phase 1: Setup
- [X] T001 Create spec/plan/tasks and checklist.
## Phase 2: Tests (TDD)
- [X] T002 Add tests for reusing an existing suitable PolicyVersion.
- [X] T003 Add tests for capturing a new PolicyVersion when reuse is not possible.
## Phase 3: Core
- [X] T004 Implement reuse decision + reuse path in BackupService.
## Phase 4: Verification
- [X] T005 Run targeted tests.
- [X] T006 Run Pint (`./vendor/bin/pint --dirty`).

View File

@ -0,0 +1,144 @@
<?php
use App\Models\BackupSet;
use App\Models\Policy;
use App\Models\PolicyVersion;
use App\Models\Tenant;
use App\Models\User;
use App\Services\Intune\BackupService;
use App\Services\Intune\PolicyCaptureOrchestrator;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Mockery\MockInterface;
uses(RefreshDatabase::class);
it('reuses latest policy version for backup when it is up-to-date and satisfies capture options', function () {
$tenant = Tenant::factory()->create();
$tenant->makeCurrent();
$user = User::factory()->create();
$this->actingAs($user);
$backupSet = BackupSet::factory()->create([
'tenant_id' => $tenant->id,
'status' => 'completed',
]);
$policy = Policy::factory()->create([
'tenant_id' => $tenant->id,
'last_synced_at' => now(),
'ignored_at' => null,
]);
$existingVersion = PolicyVersion::factory()->create([
'tenant_id' => $tenant->id,
'policy_id' => $policy->id,
'captured_at' => now(),
'snapshot' => ['id' => $policy->external_id, 'name' => $policy->display_name],
'assignments' => null,
'scope_tags' => null,
]);
$this->mock(PolicyCaptureOrchestrator::class, function (MockInterface $mock) use ($existingVersion) {
$mock->shouldReceive('capture')
->once()
->andReturn([
'version' => $existingVersion,
'captured' => [
'payload' => $existingVersion->snapshot,
'assignments' => $existingVersion->assignments,
'scope_tags' => $existingVersion->scope_tags,
'metadata' => [],
],
]);
});
$service = app(BackupService::class);
$service->addPoliciesToSet(
tenant: $tenant,
backupSet: $backupSet,
policyIds: [$policy->id],
actorEmail: $user->email,
actorName: $user->name,
includeAssignments: false,
includeScopeTags: false,
includeFoundations: false,
);
expect(PolicyVersion::query()->where('policy_id', $policy->id)->count())->toBe(1);
$item = $backupSet->items()->first();
expect($item)->not->toBeNull();
expect($item->policy_version_id)->toBe($existingVersion->id);
});
it('captures a new policy version for backup when no suitable existing version is available', function () {
$tenant = Tenant::factory()->create();
$tenant->makeCurrent();
$user = User::factory()->create();
$this->actingAs($user);
$backupSet = BackupSet::factory()->create([
'tenant_id' => $tenant->id,
'status' => 'completed',
]);
$policy = Policy::factory()->create([
'tenant_id' => $tenant->id,
'last_synced_at' => now(),
'ignored_at' => null,
]);
$staleVersion = PolicyVersion::factory()->create([
'tenant_id' => $tenant->id,
'policy_id' => $policy->id,
'version_number' => 1,
'captured_at' => now()->subDays(2),
'snapshot' => ['id' => $policy->external_id, 'name' => $policy->display_name],
]);
$policy->update(['last_synced_at' => now()]);
$this->mock(PolicyCaptureOrchestrator::class, function (MockInterface $mock) use ($policy, $tenant) {
$mock->shouldReceive('capture')
->once()
->andReturnUsing(function () use ($policy, $tenant) {
$newVersion = PolicyVersion::factory()->create([
'tenant_id' => $tenant->id,
'policy_id' => $policy->id,
'version_number' => 2,
'captured_at' => now(),
'snapshot' => ['id' => $policy->external_id, 'name' => $policy->display_name, 'changed' => true],
]);
return [
'version' => $newVersion,
'captured' => [
'payload' => $newVersion->snapshot,
'assignments' => null,
'scope_tags' => null,
'metadata' => [],
],
];
});
});
$service = app(BackupService::class);
$service->addPoliciesToSet(
tenant: $tenant,
backupSet: $backupSet,
policyIds: [$policy->id],
actorEmail: $user->email,
actorName: $user->name,
includeAssignments: false,
includeScopeTags: false,
includeFoundations: false,
);
$item = $backupSet->items()->first();
expect($item)->not->toBeNull();
expect($item->policy_version_id)->not->toBe($staleVersion->id);
});