fix(backup-scheduling): handle duplicate run slots

This commit is contained in:
Ahmed Darrazi 2026-01-05 10:25:05 +01:00
parent 4d3fcd28a9
commit c69b459c18
3 changed files with 159 additions and 68 deletions

View File

@ -41,6 +41,7 @@
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\UniqueConstraintViolationException;
use Illuminate\Support\Facades\Bus;
use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;
@ -320,19 +321,10 @@ public static function table(Table $table): Table
$user = auth()->user();
$scheduledFor = CarbonImmutable::now('UTC')->startOfMinute();
$run = null;
for ($i = 0; $i < 5; $i++) {
$exists = BackupScheduleRun::query()
->where('backup_schedule_id', $record->id)
->where('scheduled_for', $scheduledFor)
->exists();
if (! $exists) {
break;
}
$scheduledFor = $scheduledFor->addMinute();
}
try {
$run = BackupScheduleRun::create([
'backup_schedule_id' => $record->id,
'tenant_id' => $tenant->getKey(),
@ -340,6 +332,21 @@ public static function table(Table $table): Table
'status' => BackupScheduleRun::STATUS_RUNNING,
'summary' => null,
]);
break;
} catch (UniqueConstraintViolationException) {
$scheduledFor = $scheduledFor->addMinute();
}
}
if (! $run instanceof BackupScheduleRun) {
Notification::make()
->title('Run already queued')
->body('Please wait a moment and try again.')
->warning()
->send();
return;
}
app(AuditLogger::class)->log(
tenant: $tenant,
@ -387,19 +394,10 @@ public static function table(Table $table): Table
$user = auth()->user();
$scheduledFor = CarbonImmutable::now('UTC')->startOfMinute();
$run = null;
for ($i = 0; $i < 5; $i++) {
$exists = BackupScheduleRun::query()
->where('backup_schedule_id', $record->id)
->where('scheduled_for', $scheduledFor)
->exists();
if (! $exists) {
break;
}
$scheduledFor = $scheduledFor->addMinute();
}
try {
$run = BackupScheduleRun::create([
'backup_schedule_id' => $record->id,
'tenant_id' => $tenant->getKey(),
@ -407,6 +405,21 @@ public static function table(Table $table): Table
'status' => BackupScheduleRun::STATUS_RUNNING,
'summary' => null,
]);
break;
} catch (UniqueConstraintViolationException) {
$scheduledFor = $scheduledFor->addMinute();
}
}
if (! $run instanceof BackupScheduleRun) {
Notification::make()
->title('Retry already queued')
->body('Please wait a moment and try again.')
->warning()
->send();
return;
}
app(AuditLogger::class)->log(
tenant: $tenant,
@ -470,19 +483,10 @@ public static function table(Table $table): Table
/** @var BackupSchedule $record */
foreach ($records as $record) {
$scheduledFor = CarbonImmutable::now('UTC')->startOfMinute();
$run = null;
for ($i = 0; $i < 5; $i++) {
$exists = BackupScheduleRun::query()
->where('backup_schedule_id', $record->id)
->where('scheduled_for', $scheduledFor)
->exists();
if (! $exists) {
break;
}
$scheduledFor = $scheduledFor->addMinute();
}
try {
$run = BackupScheduleRun::create([
'backup_schedule_id' => $record->id,
'tenant_id' => $tenant->getKey(),
@ -490,6 +494,15 @@ public static function table(Table $table): Table
'status' => BackupScheduleRun::STATUS_RUNNING,
'summary' => null,
]);
break;
} catch (UniqueConstraintViolationException) {
$scheduledFor = $scheduledFor->addMinute();
}
}
if (! $run instanceof BackupScheduleRun) {
continue;
}
$createdRunIds[] = (int) $run->id;
@ -522,11 +535,17 @@ public static function table(Table $table): Table
]));
}
Notification::make()
$notification = Notification::make()
->title('Runs dispatched')
->body(sprintf('Queued %d run(s).', count($createdRunIds)))
->success()
->send();
->body(sprintf('Queued %d run(s).', count($createdRunIds)));
if (count($createdRunIds) === 0) {
$notification->warning();
} else {
$notification->success();
}
$notification->send();
}),
BulkAction::make('bulk_retry')
->label('Retry')
@ -548,19 +567,10 @@ public static function table(Table $table): Table
/** @var BackupSchedule $record */
foreach ($records as $record) {
$scheduledFor = CarbonImmutable::now('UTC')->startOfMinute();
$run = null;
for ($i = 0; $i < 5; $i++) {
$exists = BackupScheduleRun::query()
->where('backup_schedule_id', $record->id)
->where('scheduled_for', $scheduledFor)
->exists();
if (! $exists) {
break;
}
$scheduledFor = $scheduledFor->addMinute();
}
try {
$run = BackupScheduleRun::create([
'backup_schedule_id' => $record->id,
'tenant_id' => $tenant->getKey(),
@ -568,6 +578,15 @@ public static function table(Table $table): Table
'status' => BackupScheduleRun::STATUS_RUNNING,
'summary' => null,
]);
break;
} catch (UniqueConstraintViolationException) {
$scheduledFor = $scheduledFor->addMinute();
}
}
if (! $run instanceof BackupScheduleRun) {
continue;
}
$createdRunIds[] = (int) $run->id;
@ -600,11 +619,17 @@ public static function table(Table $table): Table
]));
}
Notification::make()
$notification = Notification::make()
->title('Retries dispatched')
->body(sprintf('Queued %d run(s).', count($createdRunIds)))
->success()
->send();
->body(sprintf('Queued %d run(s).', count($createdRunIds)));
if (count($createdRunIds) === 0) {
$notification->warning();
} else {
$notification->success();
}
$notification->send();
}),
DeleteBulkAction::make('bulk_delete')
->visible(fn (): bool => static::currentTenantRole()?->canManageBackupSchedules() ?? false),

View File

@ -88,6 +88,7 @@ ### Tests (Pest)
- [X] T029 [P] [US3] Add feature test tests/Feature/BackupScheduling/ApplyRetentionJobTest.php (keeps last N backup_sets; soft-deletes older)
- [X] T038 [P] [US1] Add feature test tests/Feature/BackupScheduling/BackupScheduleBulkDeleteTest.php (bulk delete action regression)
- [X] T039 [P] [US1] Extend tests/Feature/BackupScheduling/BackupScheduleBulkDeleteTest.php (operator cannot bulk delete)
- [X] T041 [P] [US3] Make manual dispatch actions idempotent under concurrency in app/Filament/Resources/BackupScheduleResource.php (avoid unique constraint 500); add regression in tests/Feature/BackupScheduling/RunNowRetryActionsTest.php
### Implementation

View File

@ -5,6 +5,7 @@
use App\Models\BackupSchedule;
use App\Models\BackupScheduleRun;
use App\Models\User;
use Carbon\CarbonImmutable;
use Filament\Facades\Filament;
use Illuminate\Support\Facades\Queue;
use Livewire\Livewire;
@ -203,3 +204,67 @@
Queue::assertPushed(RunBackupScheduleJob::class, 2);
$this->assertDatabaseCount('notifications', 1);
});
test('operator can bulk retry even if a run already exists for this minute', function () {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
$scheduleA = BackupSchedule::query()->create([
'tenant_id' => $tenant->id,
'name' => 'Nightly A',
'is_enabled' => true,
'timezone' => 'UTC',
'frequency' => 'daily',
'time_of_day' => '01:00:00',
'days_of_week' => null,
'policy_types' => ['deviceConfiguration'],
'include_foundations' => true,
'retention_keep_last' => 30,
]);
$scheduleB = BackupSchedule::query()->create([
'tenant_id' => $tenant->id,
'name' => 'Nightly B',
'is_enabled' => true,
'timezone' => 'UTC',
'frequency' => 'daily',
'time_of_day' => '02:00:00',
'days_of_week' => null,
'policy_types' => ['deviceConfiguration'],
'include_foundations' => true,
'retention_keep_last' => 30,
]);
$scheduledFor = CarbonImmutable::now('UTC')->startOfMinute();
BackupScheduleRun::query()->create([
'backup_schedule_id' => $scheduleA->id,
'tenant_id' => $tenant->id,
'scheduled_for' => $scheduledFor->toDateTimeString(),
'status' => BackupScheduleRun::STATUS_RUNNING,
'summary' => null,
]);
$this->actingAs($user);
Filament::setTenant($tenant, true);
Livewire::test(ListBackupSchedules::class)
->callTableBulkAction('bulk_retry', collect([$scheduleA, $scheduleB]));
expect(BackupScheduleRun::query()->where('backup_schedule_id', $scheduleA->id)->count())
->toBe(2);
$newRunA = BackupScheduleRun::query()
->where('backup_schedule_id', $scheduleA->id)
->orderByDesc('id')
->first();
expect($newRunA)->not->toBeNull();
expect($newRunA->scheduled_for->setTimezone('UTC')->toDateTimeString())
->toBe($scheduledFor->addMinute()->toDateTimeString());
expect(BackupScheduleRun::query()->where('backup_schedule_id', $scheduleB->id)->count())
->toBe(1);
Queue::assertPushed(RunBackupScheduleJob::class, 2);
});