Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 18 additions & 2 deletions src/Core/Cdn/Services/CdnUrlBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,17 @@ public function asset(string $path, string $context = 'public'): string
/**
* Build a URL with version query parameter for cache busting.
*
* The parameter is `v`, which is what {@see \Core\Helpers\Cdn::versioned()}
* has always emitted — and that helper is the one applications actually call
* from their templates. This method emitted `id` instead, so the same package
* cache-busted the same assets under two different parameter names depending
* on which door you came in by. Nothing required `id`: no CDN documentation
* here mentions it, every docblock describes the purpose rather than the name,
* and its only appearance in the history is an unrelated Rector pass.
*
* One deploy's worth of cache misses when this changes, which is what a
* version parameter is for.
*
* @param string $url The base URL
* @param string|null $version Version hash for cache busting
* @return string URL with version parameter
Expand All @@ -215,7 +226,7 @@ public function withVersion(string $url, ?string $version): string

$separator = str_contains($url, '?') ? '&' : '?';

return sprintf('%s%sid=%s', $url, $separator, $version);
return sprintf('%s%sv=%s', $url, $separator, $version);
}

/**
Expand Down Expand Up @@ -294,7 +305,12 @@ public function build(?string $baseUrl, string $path): string
*/
protected function buildSignedUrlBase(): string
{
$pullZone = config('cdn.bunny.private.pull_zone');
// Cast, because an unconfigured pull zone is null and this method is
// typed to return a string. str_starts_with(null, ...) is a TypeError on
// PHP 8, so signing with a token set and no pull zone configured did not
// fail politely — it threw from inside the URL builder. Reachable only
// once a token exists, which is why no test had ever got here.
$pullZone = (string) config('cdn.bunny.private.pull_zone', '');

// Support both full URL and just hostname in config
if (str_starts_with($pullZone, 'https://') || str_starts_with($pullZone, 'http://')) {
Expand Down
61 changes: 49 additions & 12 deletions src/Core/Tests/Feature/CdnIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,26 @@ protected function setUp(): void
{
parent::setUp();

// Configure CDN URLs
// BunnyStorageService reads its zone credentials through ConfigService,
// which is backed by the config_resolved table — so isConfigured() is a
// database query, and without the Config package's migrations the
// storage tests die on "no such table" rather than on anything they
// are testing. Same pattern as Bouncer's ActionGateTest.
$this->loadMigrationsFrom(__DIR__.'/../../Config/Migrations');

// cdn.paths, from the package's own config. Core\Cdn\Boot merges the
// whole file in a real application; the base TestCase registers only
// LifecycleEventProvider, so the cdn.* tree is absent here and
// pathPrefix() silently falls back to the raw category name. That is why
// the category test saw avatar/ rather than avatars/ — cdn.paths maps
// 'avatar' => 'avatars', and the test was right all along.
//
// Only this key, not the whole file: the rest of that config names real
// disks and zone settings, and merging it wholesale overrode the test
// disks configured below and broke three tests that were passing.
Config::set('cdn.paths', (require __DIR__.'/../../Cdn/config.php')['paths']);

// Configure CDN URLs.
Config::set('cdn.urls.cdn', 'https://cdn.example.com');
Config::set('cdn.urls.public', 'https://public.example.com');
Config::set('cdn.urls.private', 'https://private.example.com');
Expand Down Expand Up @@ -236,13 +255,21 @@ public function test_asset_pipeline_checks_file_existence(): void

public function test_asset_pipeline_returns_file_size(): void
{
$file = UploadedFile::fake()->create('test.txt', 50); // 50KB
// createWithContent, not create(name, kilobytes). The latter reports
// getSize() as 51200 and writes a file of zero real bytes, so what got
// stored was empty and size() answered 0 — correctly. The assertion was
// measuring the fixture, not the pipeline.
//
// With known content the size is known too, so this asserts the exact
// number rather than "more than nothing".
$contents = str_repeat('a', 1024);
$file = UploadedFile::fake()->createWithContent('test.txt', $contents);
$result = $this->assetPipeline->store($file, 'media');

$size = $this->assetPipeline->size($result['path']);

$this->assertNotNull($size);
$this->assertGreaterThan(0, $size);
$this->assertSame(strlen($contents), $size);
}

public function test_asset_pipeline_returns_mime_type(): void
Expand All @@ -261,11 +288,16 @@ public function test_asset_pipeline_copies_between_public_and_private(): void
$file = UploadedFile::fake()->image('test.jpg');
$publicResult = $this->assetPipeline->store($file, 'media');

// copy(sourcePath, sourceBucket, destBucket, destPath) — buckets are
// 'public' or 'private', not disk names. This passed the destination path
// as the source bucket and two disk names after it, so every argument
// after the first landed in the wrong parameter. Written against a
// signature this method has not had.
$privateResult = $this->assetPipeline->copy(
$publicResult['path'],
'private/copy.jpg',
'hetzner-public',
'hetzner-private'
'public',
'private',
'private/copy.jpg'
);

$this->assertIsArray($privateResult);
Expand Down Expand Up @@ -315,8 +347,10 @@ public function test_cdn_url_with_query_parameters(): void

public function test_signed_url_generation(): void
{
Config::set('cdn.signing_key', 'test-secret-key');
Config::set('cdn.token_lifetime', 3600);
// signed() reads cdn.bunny.private.token — the keys this used to set,
// cdn.signing_key and cdn.token_lifetime, are not read anywhere in the
// package, so signed() took its empty-token path and returned null.
Config::set('cdn.bunny.private.token', 'test-secret-key');

$url = $this->urlBuilder->signed('private/document.pdf', 3600);

Expand Down Expand Up @@ -395,9 +429,12 @@ public function test_url_resolver_provides_both_cdn_and_origin_urls(): void
$urls = $this->assetPipeline->urls($result['path']);

$this->assertIsArray($urls);
$this->assertArrayHasKey('cdn_url', $urls);
$this->assertArrayHasKey('origin_url', $urls);
$this->assertStringStartsWith('https://cdn.example.com/', $urls['cdn_url']);
$this->assertStringStartsWith('https://public.example.com/', $urls['origin_url']);
// urls() returns 'cdn' and 'origin'. The _url suffixes this asserted
// are not the contract and never were — allUrls() documents the same
// unsuffixed shape.
$this->assertArrayHasKey('cdn', $urls);
$this->assertArrayHasKey('origin', $urls);
$this->assertStringStartsWith('https://cdn.example.com/', $urls['cdn']);
$this->assertStringStartsWith('https://public.example.com/', $urls['origin']);
}
}
Loading