Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical configuration migration and unsafe cache-path issues, plus a moderate cache-path overlap issue, block approval.
2 open issues
What changed in this PR
This pull request replaces cache namespaces with named caches managed by CacheManager, updating cache consumers, configuration, commands, controllers, and tests accordingly.
Changes:
- Refactors cache implementations and management APIs around validated named caches.
- Updates cache loading, clearing, statistics, update handling, and page caching.
- Migrates configuration schemas and removes obsolete namespace-specific APIs.
- Adds compatibility handling for deprecated cache configuration.
| File | Description |
|---|---|
tests/Unit/Services/Loaders/ConfigServiceLoaderTest.php |
Tests deprecated cache configuration migration and cache-file loading. |
tests/Unit/Services/Loaders/CacheServiceLoaderTest.php |
Tests named cache construction, validation, handlers, TTLs, and paths. |
tests/Unit/Controllers/PageControllerTest.php |
Updates page-cache tests for named cache configuration and TTL compatibility. |
tests/Unit/Cache/FilesCacheTest.php |
Updates file-cache tests for direct paths instead of namespaces. |
tests/Unit/Cache/CacheTestCase.php |
Removes namespace-specific shared cache tests and updates cache factory signatures. |
tests/Unit/Cache/CacheManagerTest.php |
Tests named cache registration, validation, lookup, and reserved names. |
tests/Unit/Cache/ArrayCacheTest.php |
Updates array-cache construction tests. |
formwork/src/Services/Loaders/ConfigServiceLoader.php |
Loads cached configuration and rewrites deprecated cache options. |
formwork/src/Services/Loaders/CacheServiceLoader.php |
Builds and registers configured named caches. |
formwork/src/Panel/Controllers/UpdatesController.php |
Clears all managed caches after updates. |
formwork/src/Panel/Controllers/CacheController.php |
Supports clearing named caches from the panel. |
formwork/src/Controllers/PageController.php |
Uses the pages cache configuration and supports the new page TTL option. |
formwork/src/Commands/UpdatesCommand.php |
Clears all managed caches after CLI updates. |
formwork/src/Commands/CacheCommand.php |
Generalizes cache clearing and statistics to named caches. |
formwork/src/Cache/NamespacedCacheInterface.php |
Removes the obsolete namespaced cache interface. |
formwork/src/Cache/FilesCache.php |
Removes namespace handling and uses the configured path directly. |
formwork/src/Cache/Exceptions/InvalidNamespaceException.php |
Removes the obsolete namespace exception. |
formwork/src/Cache/CacheManager.php |
Adds named-cache registration, validation, lookup, and reserved-name handling. |
formwork/src/Cache/ArrayCache.php |
Removes namespace handling from in-memory caches. |
formwork/src/Cache/AbstractCache.php |
Switches cache implementations to PSR cache interfaces. |
formwork/schemes/config/system.yaml |
Updates the system configuration schema for the pages cache. |
formwork/config/system.yaml |
Defines the pages cache as a named cache. |
formwork/config/services/services.php |
Registers CacheManager and exposes the pages cache service. |
Review effort: Lite
Comment on lines
+92
to
+94
| if (!is_string($path) || $path === '') { | ||
| throw new InvalidArgumentException(sprintf('Invalid "path" option for cache "%s": expected non-empty string, %s given', $name, get_debug_type($path))); | ||
| } |
Comment on lines
+115
to
+117
| foreach (['enabled' => 'enabled', 'path' => 'path', 'time' => 'ttl'] as $deprecated => $replacement) { | ||
| if (!isset($data['system']['cache']) || !is_array($data['system']['cache']) || !array_key_exists($deprecated, $data['system']['cache']) || is_array($data['system']['cache'][$deprecated])) { | ||
| continue; |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the cache subsystem to simplify cache management, improve extensibility, and align with PSR standards. The main changes include removing the concept of cache namespaces in favor of named caches, and refactoring the CLI commands to support arbitrary named caches. Additionally, configuration and UI schemas are updated to reflect these changes.
Cache subsystem refactor:
NamespacedCacheInterface, replacing them with named caches managed byCacheManager. [1] [2] [3] [4] [5] [6]CacheManagerto manage caches by name, enforce name validation, and support adding, retrieving, and listing named caches.CLI and command improvements:
CacheCommandto operate on arbitrary named caches, removed special handling for "pages" cache, and generalized cache clearing and stats reporting. [1] [2] [3] [4] [5] [6]UpdatesCommandto use the new cache manager.Configuration and UI schema updates:
system.yamlconfig and schema to remove global cache settings, moving "pages" cache settings undercache.pageswith newenabled,path, andttloptions. [1] [2] [3] [4] [5]cache.pagesas a named cache instance from the cache manager.Code cleanup: