fix: ensure negative cache results are stored in TrustHosts middleware
Problem:
- Cache::remember() does not cache null return values
- When no FQDN was configured, the closure returned null
- This caused DB queries on every request, defeating the cache
Solution:
- Use empty string ('') as sentinel value instead of null
- Convert sentinel back to null after retrieving from cache
- Now both positive and negative results are cached properly
Changes:
- Return empty string from closure instead of null
- Add explicit sentinel-to-null conversion after cache retrieval
- Add test to verify negative caching works correctly
This ensures zero DB queries even when FQDN is not configured.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
922884e6d3
commit
5ce0670ca4
2 changed files with 34 additions and 3 deletions
|
|
@ -19,6 +19,7 @@ public function hosts(): array
|
||||||
$trustedHosts = [];
|
$trustedHosts = [];
|
||||||
|
|
||||||
// Trust the configured FQDN from InstanceSettings (cached to avoid DB query on every request)
|
// Trust the configured FQDN from InstanceSettings (cached to avoid DB query on every request)
|
||||||
|
// Use empty string as sentinel value instead of null so negative results are cached
|
||||||
$fqdnHost = Cache::remember('instance_settings_fqdn_host', 300, function () {
|
$fqdnHost = Cache::remember('instance_settings_fqdn_host', 300, function () {
|
||||||
try {
|
try {
|
||||||
$settings = InstanceSettings::get();
|
$settings = InstanceSettings::get();
|
||||||
|
|
@ -26,16 +27,19 @@ public function hosts(): array
|
||||||
$url = Url::fromString($settings->fqdn);
|
$url = Url::fromString($settings->fqdn);
|
||||||
$host = $url->getHost();
|
$host = $url->getHost();
|
||||||
|
|
||||||
return $host ?: null;
|
return $host ?: '';
|
||||||
}
|
}
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
// If instance settings table doesn't exist yet (during installation),
|
// If instance settings table doesn't exist yet (during installation),
|
||||||
// return null to fall back to APP_URL only
|
// return empty string (sentinel) so this result is cached
|
||||||
}
|
}
|
||||||
|
|
||||||
return null;
|
return '';
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Convert sentinel value back to null for consumption
|
||||||
|
$fqdnHost = $fqdnHost !== '' ? $fqdnHost : null;
|
||||||
|
|
||||||
if ($fqdnHost) {
|
if ($fqdnHost) {
|
||||||
$trustedHosts[] = $fqdnHost;
|
$trustedHosts[] = $fqdnHost;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -200,3 +200,30 @@
|
||||||
expect($hosts1)->toBe($hosts2);
|
expect($hosts1)->toBe($hosts2);
|
||||||
expect($hosts2)->toContain('coolify.example.com');
|
expect($hosts2)->toContain('coolify.example.com');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('caches negative results when no FQDN is configured', function () {
|
||||||
|
// Create instance settings without FQDN
|
||||||
|
InstanceSettings::updateOrCreate(
|
||||||
|
['id' => 0],
|
||||||
|
['fqdn' => null]
|
||||||
|
);
|
||||||
|
|
||||||
|
// Clear cache first
|
||||||
|
Cache::forget('instance_settings_fqdn_host');
|
||||||
|
|
||||||
|
// First call - should query database and cache empty string sentinel
|
||||||
|
$middleware1 = new TrustHosts($this->app);
|
||||||
|
$hosts1 = $middleware1->hosts();
|
||||||
|
|
||||||
|
// Verify empty string sentinel is cached (not null, which wouldn't be cached)
|
||||||
|
expect(Cache::has('instance_settings_fqdn_host'))->toBeTrue();
|
||||||
|
expect(Cache::get('instance_settings_fqdn_host'))->toBe('');
|
||||||
|
|
||||||
|
// Subsequent calls should use cached sentinel value
|
||||||
|
$middleware2 = new TrustHosts($this->app);
|
||||||
|
$hosts2 = $middleware2->hosts();
|
||||||
|
|
||||||
|
expect($hosts1)->toBe($hosts2);
|
||||||
|
// Should only contain APP_URL pattern, not any FQDN
|
||||||
|
expect($hosts2)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue