Skip to content

Commit e64c668

Browse files
Fix: Gracefully handle cache drivers that do not support tagging (#75)
* fix: gracefully handle cache drivers that do not support tagging The default config enables cache tags, but drivers like "file" or "database" do not support tagging. This change: - Types the Cache constructor $tags parameter as array and normalises the config value in GeoIP so null/missing values become an empty array - Replaces the hardcoded driver list in Clear command with a dynamic supportsTags() check that covers all current and future drivers - Updates config comment to explain auto-fallback behaviour - Adds a test covering tagged-cache construction on a non-tagging driver Addresses the same issue as Torann#255. * address review feedback: improve test, add logging, clarify error message - Fix test to use file cache driver (which genuinely lacks tag support) instead of array driver, so the fallback path is actually exercised - Log a warning in GeoIP constructor when cache tags are configured but the active driver does not support tagging - Improve geoip:clear error message to explain why selective clearing is unavailable * ci: drop PHP 8.2 from test matrix composer.json already requires ^8.3, so 8.2 builds are unnecessary. * ci: run php-cs-fixer from composer instead of Docker image The Docker image (oskarstark/php-cs-fixer-ga) does not have access to the project's composer dependencies, causing a fatal error because the custom IxDFCodingStandard\PhpCsFixer\Config class is not found. Use the locally installed php-cs-fixer from vendor/bin instead. * ci: improve code style workflow with fork safety and Rector - Replace Docker-based php-cs-fixer with composer scripts - Add Rector auto-fix step alongside php-cs-fixer - Add fork-safe dual-job setup: auto-fix for internal PRs, dry-run check for fork PRs - Add rector:dry and cs:check composer scripts for CI dry-run jobs * style: auto-fix (rector + php-cs-fixer) * ci: run code style workflow on PHP 8.3 (minimum supported version) * chore: demote cache tag fallback log to info level --------- Co-authored-by: GitHub Actions <actions@github.com>
1 parent d50c71b commit e64c668

9 files changed

Lines changed: 108 additions & 53 deletions

File tree

Lines changed: 60 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,71 @@
1-
name: Coding style
1+
name: Code Style
22

33
on:
4-
pull_request:
4+
push:
5+
branches: [main]
56
paths:
67
- '**.php'
7-
- '.composer.json'
8-
- '.php_cs'
9-
- '.github/workflows/php-coding-style.yml'
10-
push:
8+
- 'composer.json'
9+
pull_request:
1110
paths:
1211
- '**.php'
13-
- '.composer.json'
14-
- '.php_cs'
15-
- '.github/workflows/php-coding-style.yml'
16-
branches-ignore:
17-
- main # protected
12+
- 'composer.json'
1813

1914
jobs:
20-
style:
15+
fix:
16+
name: Auto-fix code style
17+
# Skip fork PRs — GITHUB_TOKEN can't push to external forks.
18+
# Fork contributors: run `composer rector && composer cs` locally.
19+
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
2120
runs-on: ubuntu-latest
21+
steps:
22+
- name: Checkout code
23+
uses: actions/checkout@v6
24+
with:
25+
ref: ${{ github.head_ref }}
26+
token: ${{ github.token }}
27+
28+
- name: Setup PHP
29+
uses: shivammathur/setup-php@v2
30+
with:
31+
php-version: 8.3
32+
coverage: none
33+
34+
- name: Install composer dependencies
35+
run: composer install --prefer-dist --no-progress --no-interaction
36+
37+
- name: Rector
38+
run: composer rector
2239

40+
- name: PHP CS Fixer
41+
run: composer cs
42+
43+
- name: Commit fixes
44+
uses: stefanzweifel/git-auto-commit-action@v7
45+
with:
46+
commit_message: 'style: auto-fix (rector + php-cs-fixer)'
47+
commit_author: 'GitHub Actions <actions@github.com>'
48+
49+
check:
50+
name: Verify code style (forks)
51+
# Only run the check job on fork PRs where we can't auto-fix.
52+
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository
53+
runs-on: ubuntu-latest
2354
steps:
24-
- name: Checkout code
25-
uses: actions/checkout@v6
26-
27-
- name: Setup PHP
28-
uses: shivammathur/setup-php@v2
29-
with:
30-
php-version: 8.4
31-
coverage: none
32-
env:
33-
COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }}
34-
35-
- name: Install composer dependencies
36-
run: composer install --no-interaction --no-progress --no-scripts
37-
38-
- name: Lint facade docblocks
39-
run: php -f vendor/bin/facade.php InteractionDesignFoundation\\GeoIP\\Facades\\GeoIP
40-
41-
- name: Fix style
42-
uses: docker://oskarstark/php-cs-fixer-ga
43-
with:
44-
args: --config=.php-cs-fixer.php --allow-risky=yes
45-
46-
- name: Commit changes
47-
uses: stefanzweifel/git-auto-commit-action@v7
48-
with:
49-
commit_message: Fix coding style
55+
- name: Checkout code
56+
uses: actions/checkout@v6
57+
58+
- name: Setup PHP
59+
uses: shivammathur/setup-php@v2
60+
with:
61+
php-version: 8.3
62+
coverage: none
63+
64+
- name: Install composer dependencies
65+
run: composer install --prefer-dist --no-progress --no-interaction
66+
67+
- name: Rector (dry-run)
68+
run: composer rector:dry
69+
70+
- name: PHP CS check
71+
run: composer cs:check

.github/workflows/run-tests.yml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,9 @@ jobs:
1212
strategy:
1313
fail-fast: true
1414
matrix:
15-
php: [8.5, 8.4, 8.3, 8.2]
15+
php: [8.5, 8.4, 8.3]
1616
laravel: [^12.0, ^13.0]
1717
dependencies: [lowest, highest]
18-
exclude:
19-
- laravel: ^13.0
20-
php: 8.2
2118

2219
name: P${{ matrix.php }} | L${{ matrix.laravel }} | ${{ matrix.dependencies == 'highest' && '↑' || '↓' }}
2320

composer.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@
7878
"phpstan": "vendor/bin/phpstan --level=0 --no-progress analyse --configuration phpstan.neon --memory-limit 2G",
7979
"psalm": "vendor/bin/psalm",
8080
"rector": "vendor/bin/rector process --config rector.php ",
81+
"rector:dry": "vendor/bin/rector process --config rector.php --dry-run",
82+
"cs:check": "@php -d memory_limit=-1 vendor/bin/php-cs-fixer fix --no-interaction --ansi --verbose --dry-run",
8183
"sa": "@psalm",
8284
"sa:bl": "@psalm --set-baseline=psalm-baseline.xml",
8385
"test": "vendor/bin/phpunit"

config/geoip.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@
128128
| Cache Tags
129129
|--------------------------------------------------------------------------
130130
|
131-
| Cache tags are not supported when using the file or database cache
132-
| drivers in Laravel. This is done so that only locations can be cleared.
131+
| Cache tags allow you to selectively clear only GeoIP cached locations
132+
| without affecting other cached data. If the active cache driver does
133+
| not support tagging (e.g. file, database), tags are automatically
134+
| ignored at runtime. Set to null or an empty array to disable tagging.
133135
|
134136
*/
135137

psalm-baseline.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
<MixedArgument>
7272
<code><![CDATA[$config]]></code>
7373
<code><![CDATA[$this->config('cache_expires', 30)]]></code>
74-
<code><![CDATA[$this->config('cache_tags')]]></code>
7574
<code><![CDATA[$this->config('default_location', [])]]></code>
7675
</MixedArgument>
7776
<MixedArgumentTypeCoercion>

src/Cache.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,9 @@ final class Cache
2424
/**
2525
* Create a new cache instance.
2626
*
27-
* @param CacheManager $cache
28-
* @param array $tags
29-
* @param int $expires
27+
* @param list<string> $tags
3028
*/
31-
public function __construct(CacheManager $cache, $tags, private readonly int $expires = 30)
29+
public function __construct(CacheManager $cache, array $tags, private readonly int $expires = 30)
3230
{
3331
$this->cache = ($tags === [] || !$cache->supportsTags()) ? $cache : $cache->tags($tags);
3432
}

src/Console/Clear.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ class Clear extends Command
2424
public function handle(): int
2525
{
2626
if ($this->isSupported() === false) {
27-
$this->output->error('Default cache system does not support tags');
27+
$this->output->error(
28+
'Cannot selectively clear GeoIP cache: either cache tags are not configured'
29+
.' or the active cache driver does not support tagging.'
30+
);
2831
return self::FAILURE;
2932
}
3033

@@ -40,8 +43,8 @@ public function handle(): int
4043
*/
4144
protected function isSupported(): bool
4245
{
43-
return (empty(app('geoip')->config('cache_tags')) === false)
44-
&& (in_array(config('cache.default'), ['file', 'database'], true) === false);
46+
return !empty(app('geoip')->config('cache_tags'))
47+
&& app(\Illuminate\Cache\CacheManager::class)->supportsTags();
4548
}
4649

4750
/**

src/GeoIP.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ public function __construct(
6464
private readonly LoggerInterface $logger,
6565
) {
6666
// Create caching instance
67+
/** @var list<string> $cacheTags */
68+
$cacheTags = array_values((array) $this->config('cache_tags', []));
69+
if ($cacheTags !== [] && !$cache->supportsTags()) {
70+
$this->logger->info('GeoIP: cache tags are configured but the active cache driver does not support tagging. Tags will be ignored.');
71+
}
72+
6773
$this->cache = new Cache(
6874
$cache,
69-
$this->config('cache_tags'),
75+
$cacheTags,
7076
$this->config('cache_expires', 30)
7177
);
7278
$this->cache->setPrefix((string) $this->config('cache_prefix'));

tests/CacheTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,32 @@ public function should_return_valid_location(): void
3232
$this->assertFalse($uncachedLocation->default);
3333
}
3434

35+
#[Test]
36+
public function should_fall_back_to_untagged_cache_when_driver_does_not_support_tags(): void
37+
{
38+
// Switch to file driver which does not support tagging
39+
config(['cache.default' => 'file']);
40+
$cacheManager = app(CacheManager::class);
41+
42+
$this->assertFalse($cacheManager->supportsTags(), 'File cache driver should not support tags');
43+
44+
// Tags are configured, but driver doesn't support them — should not throw
45+
$cache = new Cache($cacheManager, ['some-tag'], 30);
46+
47+
$location = new Location([
48+
'ip' => '81.2.69.142',
49+
'iso_code' => 'US',
50+
'lat' => 41.31,
51+
'lon' => -72.92,
52+
]);
53+
54+
$cache->set($location['ip'], $location);
55+
$cachedLocation = $cache->get($location['ip']);
56+
57+
$this->assertInstanceOf(Location::class, $cachedLocation);
58+
$this->assertSame('81.2.69.142', $cachedLocation->ip);
59+
}
60+
3561
#[Test]
3662
public function it_flushes_empty_cache(): void
3763
{

0 commit comments

Comments
 (0)