Skip to content

fix(cdn): stop the lower layer implementing the upper layer's interface #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Snider merged 3 commits into from
Aug 8, 2026
Merged

fix(cdn): stop the lower layer implementing the upper layer's interface #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
576b58e
fix(cdn): stop the lower layer implementing the upper layer's interface
Snider Aug 8, 2026
879da95
chore(psalm): drop the suppression that existed for the dependency no…
Snider Aug 8, 2026
1f29b44
fix(psalm): repair the config I broke, and remove only what should go
Snider Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions psalm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,13 @@
<referencedClass name="Core\Tenant\Models\User" />
<referencedClass name="Core\Tenant\Services\EntitlementService" />
<referencedClass name="Core\Config\Workspace" />
<!-- Pending Core\Service module (see plans/code/core/lint/RFC.md) -->
<referencedClass name="Core\Service\Contracts\HealthCheckable" />
<referencedClass name="Core\Service\HealthCheckResult" />
<!-- Pending Front\Client frontage subpackage -->
<referencedClass name="Core\Front\Client\Boot" />
<!-- Laravel framework classes Psalm CI doesn't always resolve -->
<referencedClass name="Illuminate\Foundation\Auth\User" />
</errorLevel>
</UndefinedClass>

<!-- Pending Core\Service module — referenced by Cdn BunnyStorageService and app variant -->
<MissingDependency>
<errorLevel type="suppress">
<directory name="src/Core/Cdn" />
</errorLevel>
</MissingDependency>

<!-- Suppress false positives from strict type analysis -->
<NoValue>
Expand Down
90 changes: 13 additions & 77 deletions src/Core/Cdn/Services/BunnyStorageService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
use Bunny\Storage\Client;
use Core\Config\ConfigService;
use Core\Crypt\LthnHash;
use Core\Service\Contracts\HealthCheckable;
use Core\Service\HealthCheckResult;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Storage;

Expand All @@ -27,9 +25,11 @@
* - Private zone: DRM/gated content
*
* Supports vBucket scoping for workspace-isolated CDN paths.
* Implements HealthCheckable for monitoring CDN connectivity.
* Zone reachability is exposed through checkZoneHealth() and isReachable();
* turning that into a service health report belongs to dappcore/service,
* which is the package that depends on this one.
*/
class BunnyStorageService implements HealthCheckable
class BunnyStorageService
{
protected ?Client $publicClient = null;

Expand Down Expand Up @@ -555,84 +555,19 @@ public function vBucketList(string $domain, string $path = '', string $zone = 'p
return $this->list($scopedPath, $zone);
}

// ─────────────────────────────────────────────────────────────────────────────
// Health Check (implements HealthCheckable)
// ─────────────────────────────────────────────────────────────────────────────

/**
* Perform a health check on the CDN storage zones.
*
* Tests connectivity by listing the root directory of configured storage zones.
* Returns a HealthCheckResult with status, latency, and zone information.
*/
public function healthCheck(): HealthCheckResult
{
$publicConfigured = $this->isConfigured('public');
$privateConfigured = $this->isConfigured('private');

if (! $publicConfigured && ! $privateConfigured) {
return HealthCheckResult::unknown('No CDN storage zones configured');
}

$results = [];
$startTime = microtime(true);
$hasError = false;
$isDegraded = false;

// Check public zone
if ($publicConfigured) {
$publicResult = $this->checkZoneHealth('public');
$results['public'] = $publicResult;
if (! $publicResult['success']) {
$hasError = true;
} elseif ($publicResult['latency_ms'] > 1000) {
$isDegraded = true;
}
}

// Check private zone
if ($privateConfigured) {
$privateResult = $this->checkZoneHealth('private');
$results['private'] = $privateResult;
if (! $privateResult['success']) {
$hasError = true;
} elseif ($privateResult['latency_ms'] > 1000) {
$isDegraded = true;
}
}

$totalLatency = (microtime(true) - $startTime) * 1000;

if ($hasError) {
return HealthCheckResult::unhealthy(
'One or more CDN storage zones are unreachable',
['zones' => $results],
$totalLatency
);
}

if ($isDegraded) {
return HealthCheckResult::degraded(
'CDN storage zones responding slowly',
['zones' => $results],
$totalLatency
);
}

return HealthCheckResult::healthy(
'All configured CDN storage zones operational',
['zones' => $results],
$totalLatency
);
}

/**
* Check health of a specific storage zone.
*
* Public because it is the seam the health-reporting decorator in
* dappcore/service consumes. That decorator composes this service rather
* than extending it, so a protected probe would be unreachable and the
* relocation would cost the detail — latency and the error string — that
* makes a health report worth reading.
*
* @param string $zone 'public' or 'private'
* @return array{success: bool, latency_ms: float, error?: string}
*/
protected function checkZoneHealth(string $zone): array
public function checkZoneHealth(string $zone): array
Comment on lines 567 to +570

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject invalid storage-zone names.

Line 575 selects the public client for every value except private. A caller that passes publci can receive a successful public-zone probe and report the wrong zone as healthy. Reject unsupported values before selecting a client.

Proposed fix 
 public function checkZoneHealth(string $zone): array
 {
+    if (! in_array($zone, ['public', 'private'], true)) {
+        return [
+            'success' => false,
+            'latency_ms' => 0.0,
+            'error' => 'Invalid storage zone',
+        ];
+    }
+
     $startTime = microtime(true);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Core/Cdn/Services/BunnyStorageService.php` around lines 567 - 570, Update
checkZoneHealth to validate that zone is exactly public or private before
selecting the storage client; reject unsupported values such as misspellings
instead of falling through to the public client, while preserving the existing
client-selection behavior for valid zones.

{
$startTime = microtime(true);

Expand Down Expand Up @@ -677,7 +612,8 @@ protected function checkZoneHealth(string $zone): array
/**
* Perform a quick connectivity check.
*
* Simpler than healthCheck() - just returns true/false.
* A boolean answer, for callers that only need reachability. The detailed
* per-zone probe is checkZoneHealth().
*
* @param string $zone 'public', 'private', or 'any' (default)
*/
Expand Down
20 changes: 18 additions & 2 deletions src/Core/Tests/Feature/CdnIntegrationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Core\Tests\Feature;

use Core\Cdn\Services\AssetPipeline;
use Core\Cdn\Services\BunnyStorageService;
use Core\Cdn\Services\CdnUrlBuilder;
use Core\Cdn\Services\StorageUrlResolver;
use Core\Tests\TestCase;
Expand Down Expand Up @@ -67,9 +68,24 @@ protected function setUp(): void
'visibility' => 'private',
]);

// Initialize services
// Initialize services.
//
// StorageUrlResolver takes the storage service first and the URL builder
// second, and the builder is the optional one. This passed the builder
// into the first parameter and omitted the required argument entirely,
// so every test in this file died in setUp on a TypeError — 30 of them,
// reported as 30 failures rather than as the one line they are.
//
// BunnyStorageService comes from the container, which is how production
// gets it: Core\Cdn\Boot registers StorageUrlResolver as a singleton and
// lets Laravel autowire the chain. Resolving it here rather than
// hand-building BunnyStorageService(ConfigService) keeps the test on the
// same path the application uses.
$this->urlBuilder = new CdnUrlBuilder();
$this->urlResolver = new StorageUrlResolver($this->urlBuilder);
$this->urlResolver = new StorageUrlResolver(
app(BunnyStorageService::class),
$this->urlBuilder,
);
$this->assetPipeline = new AssetPipeline($this->urlResolver);
}

Expand Down
Loading