From d581768281370e04050e6097712eed008f5b9513 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 13 Sep 2026 00:46:08 +0000 Subject: [PATCH] refactor: share zone resolve, truncate, and API error helpers Move resolveZoneId/truncate/formatApiError onto InteractsWithCloudflare, modernize dns:delete and dns:update onto the shared traits (no exit(1)), and use formatApiError across zone/tunnel failure paths. Co-authored-by: Jordan Partridge --- .../Concerns/InteractsWithCloudflare.php | 42 +++++++++++++ app/Commands/DnsCreateCommand.php | 29 ++------- app/Commands/DnsDeleteCommand.php | 61 ++++++------------- app/Commands/DnsListCommand.php | 36 ++--------- app/Commands/DnsUpdateCommand.php | 60 +++++------------- app/Commands/TunnelCreateCommand.php | 2 +- app/Commands/TunnelDeleteCommand.php | 2 +- app/Commands/TunnelExposeCommand.php | 2 +- app/Commands/TunnelGetCommand.php | 10 +-- app/Commands/TunnelListCommand.php | 2 +- app/Commands/ZonesCommand.php | 2 +- tests/Feature/DnsTunnelCrudCommandsTest.php | 17 +++--- 12 files changed, 104 insertions(+), 161 deletions(-) diff --git a/app/Commands/Concerns/InteractsWithCloudflare.php b/app/Commands/Concerns/InteractsWithCloudflare.php index de291e1..d96e371 100644 --- a/app/Commands/Concerns/InteractsWithCloudflare.php +++ b/app/Commands/Concerns/InteractsWithCloudflare.php @@ -5,6 +5,7 @@ namespace App\Commands\Concerns; use App\Integrations\Cloudflare\CloudflareConnector; +use Saloon\Http\Response; trait InteractsWithCloudflare { @@ -145,4 +146,45 @@ protected function resolveZoneIdForHostname(CloudflareConnector $connector, stri return null; } + + /** + * Resolve a zone ID or domain name to a zone ID. + */ + protected function resolveZoneId(CloudflareConnector $connector, string $zone): ?string + { + if (preg_match('/^[a-f0-9]{32}$/', $zone) === 1) { + return $zone; + } + + $response = $connector->zones()->list($zone); + + if ($response->successful()) { + $zones = $response->json('result', []); + + return $zones[0]['id'] ?? null; + } + + return null; + } + + protected function truncate(string $value, int $length): string + { + return strlen($value) > $length + ? substr($value, 0, $length - 3).'...' + : $value; + } + + /** + * Prefer Cloudflare errors.0.message when present. + */ + protected function formatApiError(Response $response): string + { + $message = $response->json('errors.0.message'); + + if (is_string($message) && $message !== '') { + return $message; + } + + return $response->body(); + } } diff --git a/app/Commands/DnsCreateCommand.php b/app/Commands/DnsCreateCommand.php index 6cc96ed..f691de5 100644 --- a/app/Commands/DnsCreateCommand.php +++ b/app/Commands/DnsCreateCommand.php @@ -6,7 +6,6 @@ use App\Commands\Concerns\InteractsWithCloudflare; use App\Commands\Concerns\OutputsJson; -use App\Integrations\Cloudflare\CloudflareConnector; use LaravelZero\Framework\Commands\Command; class DnsCreateCommand extends Command @@ -32,17 +31,13 @@ public function handle(): int return self::FAILURE; } - $zone = $this->argument('zone'); - - if (! preg_match('/^[a-f0-9]{32}$/', $zone)) { - $zoneId = $this->resolveZoneId($connector, $zone); - if (! $zoneId) { - return $this->jsonFail("Zone not found: {$zone}"); - } - $zone = $zoneId; + $zone = (string) $this->argument('zone'); + $zoneId = $this->resolveZoneId($connector, $zone); + if (! $zoneId) { + return $this->jsonFail("Zone not found: {$zone}"); } - $response = $connector->dns($zone)->create( + $response = $connector->dns($zoneId)->create( $this->argument('type'), $this->argument('name'), $this->argument('content'), @@ -51,7 +46,7 @@ public function handle(): int ); if (! $response->successful()) { - return $this->jsonFail('Failed to create DNS record: '.$response->body()); + return $this->jsonFail('Failed to create DNS record: '.$this->formatApiError($response)); } $record = $response->json('result'); @@ -71,16 +66,4 @@ public function handle(): int return self::SUCCESS; } - - protected function resolveZoneId(CloudflareConnector $connector, string $name): ?string - { - $response = $connector->zones()->list($name); - if ($response->successful()) { - $zones = $response->json('result', []); - - return $zones[0]['id'] ?? null; - } - - return null; - } } diff --git a/app/Commands/DnsDeleteCommand.php b/app/Commands/DnsDeleteCommand.php index c57f040..05316c6 100644 --- a/app/Commands/DnsDeleteCommand.php +++ b/app/Commands/DnsDeleteCommand.php @@ -4,11 +4,15 @@ namespace App\Commands; -use App\Integrations\Cloudflare\CloudflareConnector; +use App\Commands\Concerns\InteractsWithCloudflare; +use App\Commands\Concerns\OutputsJson; use LaravelZero\Framework\Commands\Command; class DnsDeleteCommand extends Command { + use InteractsWithCloudflare; + use OutputsJson; + protected $signature = 'dns:delete {zone : Zone ID or domain name} {id : DNS record ID to delete} @@ -20,21 +24,19 @@ class DnsDeleteCommand extends Command public function handle(): int { $connector = $this->getConnector(); + if ($connector === null) { + return self::FAILURE; + } + $zone = (string) $this->argument('zone'); $recordId = (string) $this->argument('id'); - // If zone looks like a domain, resolve it to ID - if (! preg_match('/^[a-f0-9]{32}$/', $zone)) { - $zoneId = $this->resolveZoneId($connector, $zone); - if (! $zoneId) { - $this->error("Zone not found: {$zone}"); - - return self::FAILURE; - } - $zone = $zoneId; + $zoneId = $this->resolveZoneId($connector, $zone); + if (! $zoneId) { + return $this->jsonFail("Zone not found: {$zone}"); } - if (! $this->option('force')) { + if (! $this->option('force') && ! $this->wantsJson()) { if (! $this->confirm("Delete DNS record {$recordId}?")) { $this->info('Cancelled.'); @@ -42,49 +44,20 @@ public function handle(): int } } - $response = $connector->dns($zone)->delete($recordId); + $response = $connector->dns($zoneId)->delete($recordId); if (! $response->successful()) { - $this->error('Failed to delete DNS record: '.$response->body()); - - return self::FAILURE; + return $this->jsonFail('Failed to delete DNS record: '.$this->formatApiError($response)); } $result = $response->json('result'); - if ($this->option('json')) { - $this->line(json_encode($result ?? ['id' => $recordId], JSON_PRETTY_PRINT)); - - return self::SUCCESS; + if ($this->wantsJson()) { + return $this->jsonSuccess($result ?? ['id' => $recordId]); } $this->info('DNS record deleted successfully.'); return self::SUCCESS; } - - protected function resolveZoneId(CloudflareConnector $connector, string $name): ?string - { - $response = $connector->zones()->list($name); - if ($response->successful()) { - $zones = $response->json('result', []); - - return $zones[0]['id'] ?? null; - } - - return null; - } - - protected function getConnector(): CloudflareConnector - { - $token = env('CLOUDFLARE_API_TOKEN'); - $accountId = env('CLOUDFLARE_ACCOUNT_ID'); - - if (! $token) { - $this->error('CLOUDFLARE_API_TOKEN not set'); - exit(1); - } - - return new CloudflareConnector($token, $accountId); - } } diff --git a/app/Commands/DnsListCommand.php b/app/Commands/DnsListCommand.php index 21e094b..8b022f3 100644 --- a/app/Commands/DnsListCommand.php +++ b/app/Commands/DnsListCommand.php @@ -6,7 +6,6 @@ use App\Commands\Concerns\InteractsWithCloudflare; use App\Commands\Concerns\OutputsJson; -use App\Integrations\Cloudflare\CloudflareConnector; use LaravelZero\Framework\Commands\Command; class DnsListCommand extends Command @@ -29,23 +28,19 @@ public function handle(): int return self::FAILURE; } - $zone = $this->argument('zone'); - - if (! preg_match('/^[a-f0-9]{32}$/', $zone)) { - $zoneId = $this->resolveZoneId($connector, $zone); - if (! $zoneId) { - return $this->jsonFail("Zone not found: {$zone}"); - } - $zone = $zoneId; + $zone = (string) $this->argument('zone'); + $zoneId = $this->resolveZoneId($connector, $zone); + if (! $zoneId) { + return $this->jsonFail("Zone not found: {$zone}"); } - $response = $connector->dns($zone)->list( + $response = $connector->dns($zoneId)->list( $this->option('type'), $this->option('name') ); if (! $response->successful()) { - return $this->jsonFail('Failed to list DNS records: '.$response->body()); + return $this->jsonFail('Failed to list DNS records: '.$this->formatApiError($response)); } $records = $response->json('result', []); @@ -73,23 +68,4 @@ public function handle(): int return self::SUCCESS; } - - protected function resolveZoneId(CloudflareConnector $connector, string $name): ?string - { - $response = $connector->zones()->list($name); - if ($response->successful()) { - $zones = $response->json('result', []); - - return $zones[0]['id'] ?? null; - } - - return null; - } - - protected function truncate(string $value, int $length): string - { - return strlen($value) > $length - ? substr($value, 0, $length - 3).'...' - : $value; - } } diff --git a/app/Commands/DnsUpdateCommand.php b/app/Commands/DnsUpdateCommand.php index ee4ea60..d3c411c 100644 --- a/app/Commands/DnsUpdateCommand.php +++ b/app/Commands/DnsUpdateCommand.php @@ -4,11 +4,15 @@ namespace App\Commands; -use App\Integrations\Cloudflare\CloudflareConnector; +use App\Commands\Concerns\InteractsWithCloudflare; +use App\Commands\Concerns\OutputsJson; use LaravelZero\Framework\Commands\Command; class DnsUpdateCommand extends Command { + use InteractsWithCloudflare; + use OutputsJson; + protected $signature = 'dns:update {zone : Zone ID or domain name} {id : DNS record ID to update} @@ -24,20 +28,17 @@ class DnsUpdateCommand extends Command public function handle(): int { $connector = $this->getConnector(); - $zone = (string) $this->argument('zone'); - - // If zone looks like a domain, resolve it to ID - if (! preg_match('/^[a-f0-9]{32}$/', $zone)) { - $zoneId = $this->resolveZoneId($connector, $zone); - if (! $zoneId) { - $this->error("Zone not found: {$zone}"); + if ($connector === null) { + return self::FAILURE; + } - return self::FAILURE; - } - $zone = $zoneId; + $zone = (string) $this->argument('zone'); + $zoneId = $this->resolveZoneId($connector, $zone); + if (! $zoneId) { + return $this->jsonFail("Zone not found: {$zone}"); } - $response = $connector->dns($zone)->update( + $response = $connector->dns($zoneId)->update( (string) $this->argument('id'), (string) $this->argument('type'), (string) $this->argument('name'), @@ -47,17 +48,13 @@ public function handle(): int ); if (! $response->successful()) { - $this->error('Failed to update DNS record: '.$response->body()); - - return self::FAILURE; + return $this->jsonFail('Failed to update DNS record: '.$this->formatApiError($response)); } $record = $response->json('result'); - if ($this->option('json')) { - $this->line(json_encode($record, JSON_PRETTY_PRINT)); - - return self::SUCCESS; + if ($this->wantsJson()) { + return $this->jsonSuccess($record); } $this->info('DNS record updated successfully!'); @@ -72,29 +69,4 @@ public function handle(): int return self::SUCCESS; } - - protected function resolveZoneId(CloudflareConnector $connector, string $name): ?string - { - $response = $connector->zones()->list($name); - if ($response->successful()) { - $zones = $response->json('result', []); - - return $zones[0]['id'] ?? null; - } - - return null; - } - - protected function getConnector(): CloudflareConnector - { - $token = env('CLOUDFLARE_API_TOKEN'); - $accountId = env('CLOUDFLARE_ACCOUNT_ID'); - - if (! $token) { - $this->error('CLOUDFLARE_API_TOKEN not set'); - exit(1); - } - - return new CloudflareConnector($token, $accountId); - } } diff --git a/app/Commands/TunnelCreateCommand.php b/app/Commands/TunnelCreateCommand.php index a534808..016744f 100644 --- a/app/Commands/TunnelCreateCommand.php +++ b/app/Commands/TunnelCreateCommand.php @@ -35,7 +35,7 @@ public function handle(): int $response = $connector->tunnels()->create($name); if (! $response->successful()) { - return $this->jsonFail('Failed to create tunnel: '.$response->body()); + return $this->jsonFail('Failed to create tunnel: '.$this->formatApiError($response)); } $tunnel = $response->json('result') ?? []; diff --git a/app/Commands/TunnelDeleteCommand.php b/app/Commands/TunnelDeleteCommand.php index 4b07623..cd568c6 100644 --- a/app/Commands/TunnelDeleteCommand.php +++ b/app/Commands/TunnelDeleteCommand.php @@ -44,7 +44,7 @@ public function handle(): int $response = $connector->tunnels()->delete($tunnelId); if (! $response->successful()) { - return $this->jsonFail('Failed to delete tunnel: '.$response->body()); + return $this->jsonFail('Failed to delete tunnel: '.$this->formatApiError($response)); } if ($this->wantsJson()) { diff --git a/app/Commands/TunnelExposeCommand.php b/app/Commands/TunnelExposeCommand.php index 0c422eb..8fc2e25 100644 --- a/app/Commands/TunnelExposeCommand.php +++ b/app/Commands/TunnelExposeCommand.php @@ -39,7 +39,7 @@ public function handle(): int $response = $connector->tunnels()->create($name); if (! $response->successful()) { - return $this->jsonFail('Failed to create tunnel: '.$response->body()); + return $this->jsonFail('Failed to create tunnel: '.$this->formatApiError($response)); } $tunnel = $response->json('result') ?? []; diff --git a/app/Commands/TunnelGetCommand.php b/app/Commands/TunnelGetCommand.php index 0389f56..678518b 100644 --- a/app/Commands/TunnelGetCommand.php +++ b/app/Commands/TunnelGetCommand.php @@ -31,17 +31,13 @@ public function handle(): int $response = $connector->tunnels()->get($tunnelId); if (! $response->successful()) { - $this->error('Failed to get tunnel: '.$response->body()); - - return self::FAILURE; + return $this->jsonFail('Failed to get tunnel: '.$this->formatApiError($response)); } $tunnel = $response->json('result'); - if ($this->option('json')) { - $this->line(json_encode($tunnel, JSON_PRETTY_PRINT)); - - return self::SUCCESS; + if ($this->wantsJson()) { + return $this->jsonSuccess($tunnel); } $this->table(['Field', 'Value'], [ diff --git a/app/Commands/TunnelListCommand.php b/app/Commands/TunnelListCommand.php index b57e510..a315fb9 100644 --- a/app/Commands/TunnelListCommand.php +++ b/app/Commands/TunnelListCommand.php @@ -28,7 +28,7 @@ public function handle(): int $response = $connector->tunnels()->list(); if (! $response->successful()) { - return $this->jsonFail('Failed to list tunnels: '.$response->body()); + return $this->jsonFail('Failed to list tunnels: '.$this->formatApiError($response)); } $tunnels = $response->json('result', []); diff --git a/app/Commands/ZonesCommand.php b/app/Commands/ZonesCommand.php index 57231c9..c52e1b0 100644 --- a/app/Commands/ZonesCommand.php +++ b/app/Commands/ZonesCommand.php @@ -29,7 +29,7 @@ public function handle(): int $response = $connector->zones()->list($this->option('name')); if (! $response->successful()) { - return $this->jsonFail('Failed to list zones: '.$response->body()); + return $this->jsonFail('Failed to list zones: '.$this->formatApiError($response)); } $zones = $response->json('result', []); diff --git a/tests/Feature/DnsTunnelCrudCommandsTest.php b/tests/Feature/DnsTunnelCrudCommandsTest.php index d174bcd..99a60cc 100644 --- a/tests/Feature/DnsTunnelCrudCommandsTest.php +++ b/tests/Feature/DnsTunnelCrudCommandsTest.php @@ -78,8 +78,8 @@ function decodeCommandJson(string $output): array $payload = decodeCommandJson(Artisan::output()); expect($exit)->toBe(0) - ->and($payload)->toHaveKey('id') - ->and($payload['id'])->toBe($recordId); + ->and($payload['ok'])->toBeTrue() + ->and($payload['data']['id'])->toBe($recordId); $mock->assertSent(DeleteDnsRecord::class); }); @@ -154,12 +154,13 @@ function decodeCommandJson(string $output): array $payload = decodeCommandJson(Artisan::output()); expect($exit)->toBe(0) - ->and($payload['id'])->toBe($tunnelId) - ->and($payload['name'])->toBe('my-app') - ->and($payload['status'])->toBe('healthy') - ->and($payload['created_at'])->toBe('2024-01-15T12:00:00Z') - ->and($payload['connections'])->toHaveCount(1) - ->and($payload['connections'][0]['id'])->toBe('c1'); + ->and($payload['ok'])->toBeTrue() + ->and($payload['data']['id'])->toBe($tunnelId) + ->and($payload['data']['name'])->toBe('my-app') + ->and($payload['data']['status'])->toBe('healthy') + ->and($payload['data']['created_at'])->toBe('2024-01-15T12:00:00Z') + ->and($payload['data']['connections'])->toHaveCount(1) + ->and($payload['data']['connections'][0]['id'])->toBe('c1'); $mock->assertSent(GetTunnel::class); });