feat: add custom cache driver configuration #34
Conversation
Add the ability to specify a custom cache driver for Blasp via config or environment variable. This is useful for environments like Laravel Vapor where DynamoDB has size limits that can be exceeded when caching profanity expressions. - Add `cache_driver` config option with `BLASP_CACHE_DRIVER` env support - Refactor ConfigurationLoader to use configurable cache store - Add tests for cache driver configuration - Update README with cache driver documentation Based on PR #25 by @dimzeta - adapted for Blasp v3.0 architecture. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a configurable cache driver option (env/config) and routes all cache operations in ConfigurationLoader through a new getCache() accessor; updates README and tests to validate custom driver selection, key tracking, and cache clearing behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Config/ConfigurationLoader.php (1)
25-35: Consider more explicit null check.The current implementation works, but using a truthy check means an empty string
""would callCache::store("")instead of using the default. While unlikely to occur in practice, a more explicit check would be clearer.Apply this diff for more explicit behavior:
private static function getCache(): \Illuminate\Contracts\Cache\Repository { $driver = config('blasp.cache_driver'); - return $driver ? Cache::store($driver) : Cache::store(); + return $driver !== null ? Cache::store($driver) : Cache::store(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)config/config.php(1 hunks)src/Config/ConfigurationLoader.php(5 hunks)tests/CacheDriverConfigurationTest.php(1 hunks)tests/TestCase.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/CacheDriverConfigurationTest.php (3)
src/Facades/Blasp.php (1)
Blasp(21-127)src/Config/ConfigurationLoader.php (3)
ConfigurationLoader(21-369)load(50-93)clearCache(358-368)tests/TestCase.php (2)
TestCase(9-31)setUp(19-30)
src/Config/ConfigurationLoader.php (3)
src/Registries/DetectionStrategyRegistry.php (1)
get(39-48)src/Config/MultiLanguageDetectionConfig.php (1)
getCurrentLanguage(47-50)src/Contracts/MultiLanguageConfigInterface.php (1)
getCurrentLanguage(14-14)
🔇 Additional comments (14)
README.md (1)
269-288: LGTM! Clear and helpful documentation.The cache driver configuration section is well-written and provides practical context for why this feature is needed, particularly for environments like Laravel Vapor with DynamoDB limitations.
config/config.php (1)
26-40: LGTM! Well-documented configuration option.The cache driver configuration is properly implemented with comprehensive inline documentation. The
nulldefault (when env var is not set) correctly allows fallback to Laravel's default cache driver.tests/TestCase.php (1)
29-29: LGTM! Consistent test setup.The cache driver configuration is properly propagated to the test environment, following the same pattern as other configuration settings.
src/Config/ConfigurationLoader.php (4)
261-272: LGTM! Cache retrieval properly refactored.The cache retrieval operation is correctly routed through the
getCache()helper, ensuring it respects the configured cache driver.
309-334: LGTM! Cache storage properly refactored.The cache storage operation is correctly routed through the
getCache()helper. The formatting cleanup is also appropriate.
342-351: LGTM! Efficient cache instance reuse.The implementation correctly caches the repository instance in a local variable, avoiding redundant
getCache()calls within the method. This is an efficient pattern.
358-368: LGTM! Cache clearing properly refactored.The cache clearing logic is correctly refactored to use the configured cache driver. All tracked keys and the tracking key itself are properly cleared from the correct cache store.
tests/CacheDriverConfigurationTest.php (7)
1-27: LGTM! Proper test setup with good isolation.The test class is properly structured with appropriate mocking and cache flushing to ensure test isolation. The mock expression generator returning an empty array is sufficient for testing cache driver behavior.
28-36: LGTM! Validates default cache driver behavior.The test correctly verifies that when
cache_driveris not configured, the default Laravel cache driver is used and cache key tracking works as expected.
38-49: LGTM! Validates custom cache driver selection.The test correctly verifies that when a custom cache driver is configured, all cache operations use that driver. The assertion correctly checks the 'array' store specifically.
51-68: LGTM! Validates cache clearing with custom driver.The test correctly verifies that the
clearCache()method respects the configured cache driver and clears from the correct cache store.
70-88: LGTM! Smart test for cache persistence across instances.The test cleverly uses different mock behavior for the second loader to prove that cached data is reused. This validates that caching works correctly across different
ConfigurationLoaderinstances.
90-109: LGTM! Thorough validation of cache key tracking.The test comprehensively validates that cache key tracking works with custom drivers, including verifying that all tracked keys actually exist in the cache store.
111-127: LGTM! Validates cache clearing respects current driver.The test verifies that
clearCache()clears from the currently configured cache driver. While the test doesn't explicitly switch drivers mid-test, it effectively validates that the staticclearCache()method uses the current configuration.
Address CodeRabbit nitpick - use `!== null` instead of truthy check to make intent clearer and handle edge case of empty string config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Config/ConfigurationLoader.php (1)
25-35:getCache()abstraction is clean; consider whether to fail fast or fall back on invalid driver namesThe helper cleanly centralizes cache store selection and honors
blasp.cache_driverwhile defaulting to the main store when unset, which fits the PR goals.One thing to think about: if
blasp.cache_driveris set to a non-existent store,Cache::store($driver)will raise anInvalidArgumentExceptionand break configuration loading at runtime. That may be exactly what you want (fail-fast on misconfiguration), but if you’d prefer to degrade more gracefully you could optionally catch that exception and fall back to the default store (possibly logging a warning) instead of hard-failing.For example, if you decide you want a soft fallback:
- private static function getCache(): \Illuminate\Contracts\Cache\Repository - { - $driver = config('blasp.cache_driver'); - - return $driver !== null ? Cache::store($driver) : Cache::store(); - } + private static function getCache(): \Illuminate\Contracts\Cache\Repository + { + $driver = config('blasp.cache_driver'); + + if ($driver === null) { + return Cache::store(); + } + + try { + return Cache::store($driver); + } catch (\InvalidArgumentException $e) { + // Optionally log here, then fall back to default store + return Cache::store(); + } + }If strict failure on bad configuration is desired, the current implementation is fine as-is — just worth making that decision explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Config/ConfigurationLoader.php(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Config/ConfigurationLoader.php (3)
src/Registries/DetectionStrategyRegistry.php (1)
get(39-48)src/Config/MultiLanguageDetectionConfig.php (1)
getCurrentLanguage(47-50)src/Contracts/MultiLanguageConfigInterface.php (1)
getCurrentLanguage(14-14)
🔇 Additional comments (1)
src/Config/ConfigurationLoader.php (1)
264-265: Consistent routing of cache operations throughgetCache()aligns with configurable driver supportAll cache reads/writes/clears now go through
self::getCache()(loadFromCacheOrGenerate,cacheConfiguration,trackCacheKey,clearCache), so the configuredblasp.cache_driveris honored uniformly for:
- loading and storing both single-language and multi-language configs,
- tracking cached keys in
blasp_cache_keys, and- clearing all tracked entries via
clearCache().This keeps behavior coherent across stores and matches the PR objective of allowing a custom cache driver without leaving any operations on the default store.
Also applies to: 332-333, 344-350, 360-368
Summary
Changes
cache_driverconfig option toconfig/config.phpBLASP_CACHE_DRIVERenvironment variable supportConfigurationLoaderto use a configurable cache store viagetCache()methodUsage
Credits
This PR is based on the original work in #25 by @dimzeta, adapted for Blasp v3.0 architecture.
Closes #25
Test plan
BLASP_CACHE_DRIVER=arrayenvironment variableBLASP_CACHE_DRIVER=redisif available🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.