From edcd44db89802c3586b2bde8aab5aaabd5d64216 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:10:04 +0200 Subject: [PATCH] fix: serialize database import starts per resource Hold a cache lock keyed by database UUID across the active-import check and remote_process so two concurrent requests cannot both queue a restore against the same database. --- app/Actions/Database/StartDatabaseImport.php | 24 ++++ .../StartDatabaseImportConcurrencyTest.php | 125 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tests/Feature/StartDatabaseImportConcurrencyTest.php diff --git a/app/Actions/Database/StartDatabaseImport.php b/app/Actions/Database/StartDatabaseImport.php index 33e9e5e178..eb4d4c33c6 100644 --- a/app/Actions/Database/StartDatabaseImport.php +++ b/app/Actions/Database/StartDatabaseImport.php @@ -4,6 +4,7 @@ namespace App\Actions\Database; use App\Enums\ProcessStatus; use App\Models\S3Storage; +use App\Models\Server; use App\Models\ServiceDatabase; use App\Models\SwarmDocker; use App\Rules\SafeWebhookUrl; @@ -13,6 +14,7 @@ use App\Support\DatabaseImport\DatabaseImportException; use App\Support\DatabaseImport\DatabaseImportSource; use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; use Lorisleiva\Actions\Concerns\AsAction; @@ -24,8 +26,15 @@ class StartDatabaseImport public const MAX_BYTES = 10 * 1024 * 1024 * 1024; + public const LOCK_SECONDS = 1800; + public function __construct(private readonly DatabaseImportCommandBuilder $commands) {} + public static function lockKey(string $resourceUuid): string + { + return "database-import:{$resourceUuid}"; + } + public function handle(Model $resource, DatabaseImportSource $source, int $teamId): Activity { if (! $this->commands->supports($resource)) { @@ -44,6 +53,21 @@ class StartDatabaseImport throw new DatabaseImportException('The database server or container is invalid.', 400); } + $lock = Cache::lock(self::lockKey($resource->uuid), self::LOCK_SECONDS); + + if (! $lock->get()) { + throw new DatabaseImportException('A database import is already running.', 409); + } + + try { + return $this->startImport($resource, $source, $teamId, $server, $container, $network); + } finally { + $lock->release(); + } + } + + private function startImport(Model $resource, DatabaseImportSource $source, int $teamId, Server $server, string $container, string $network): Activity + { $active = Activity::query()->where('properties->team_id', $teamId) ->where('properties->type_uuid', $resource->uuid) ->where('properties->operation', 'database_import') diff --git a/tests/Feature/StartDatabaseImportConcurrencyTest.php b/tests/Feature/StartDatabaseImportConcurrencyTest.php new file mode 100644 index 0000000000..d7d27a6c8c --- /dev/null +++ b/tests/Feature/StartDatabaseImportConcurrencyTest.php @@ -0,0 +1,125 @@ +set('cache.default', 'array'); + InstanceSettings::forceCreate(['id' => 0]); + $this->team = Team::factory()->create(); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::firstOrCreate( + ['server_id' => $this->server->id, 'network' => 'coolify'], + ['uuid' => (string) Str::uuid(), 'name' => 'docker'] + ); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); + $this->database = StandalonePostgresql::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'db', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'db', + 'image' => 'postgres:17', + 'status' => 'running', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); +}); + +function startImport(object $database, int $teamId, string $path = 'backup.sql'): Activity +{ + return app(StartDatabaseImport::class)->handle( + $database, + new DatabaseImportSource('server', path: $path), + $teamId, + ); +} + +test('a held import lock rejects a second start before restore commands run', function () { + $lock = Cache::lock(StartDatabaseImport::lockKey($this->database->uuid), StartDatabaseImport::LOCK_SECONDS); + expect($lock->get())->toBeTrue(); + + try { + expect(fn () => startImport($this->database, $this->team->id)) + ->toThrow(DatabaseImportException::class, 'A database import is already running.'); + } finally { + $lock->release(); + } +}); + +test('an already queued import is still rejected after the lock is acquired', function () { + Activity::create([ + 'log_name' => 'default', + 'description' => 'queued', + 'properties' => [ + 'team_id' => $this->team->id, + 'type_uuid' => $this->database->uuid, + 'operation' => 'database_import', + 'status' => ProcessStatus::QUEUED->value, + ], + ]); + + expect(fn () => startImport($this->database, $this->team->id)) + ->toThrow(DatabaseImportException::class, 'A database import is already running.'); +}); + +test('an in-progress import is rejected the same way as a queued import', function () { + Activity::create([ + 'log_name' => 'default', + 'description' => 'in progress', + 'properties' => [ + 'team_id' => $this->team->id, + 'type_uuid' => $this->database->uuid, + 'operation' => 'database_import', + 'status' => ProcessStatus::IN_PROGRESS->value, + ], + ]); + + expect(fn () => startImport($this->database, $this->team->id)) + ->toThrow(DatabaseImportException::class, 'A database import is already running.'); +}); + +test('a finished import does not hold the active-import guard', function () { + Activity::create([ + 'log_name' => 'default', + 'description' => 'finished', + 'properties' => [ + 'team_id' => $this->team->id, + 'type_uuid' => $this->database->uuid, + 'operation' => 'database_import', + 'status' => ProcessStatus::FINISHED->value, + ], + ]); + + expect(fn () => startImport($this->database, $this->team->id)) + ->toThrow(DatabaseImportException::class, 'The server path is invalid.'); +}); + +test('a failed start releases the import lock', function () { + expect(fn () => startImport($this->database, $this->team->id)) + ->toThrow(DatabaseImportException::class, 'The server path is invalid.'); + + $lock = Cache::lock(StartDatabaseImport::lockKey($this->database->uuid), StartDatabaseImport::LOCK_SECONDS); + try { + expect($lock->get())->toBeTrue(); + } finally { + $lock->release(); + } +});