-
-
Notifications
You must be signed in to change notification settings - Fork 1
Preload results in in-memory cache; Rename Cache -> FileCache
#8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
============================================
- Coverage 49.50% 44.65% -4.85%
- Complexity 61 75 +14
============================================
Files 8 10 +2
Lines 202 262 +60
============================================
+ Hits 100 117 +17
- Misses 102 145 +43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cache -> FileCache
Cache -> FileCacheCache -> FileCache
Cache -> FileCacheCache -> FileCache
| $promises = array_map( | ||
| fn (string $slug) => $this->isClosedAsync($slug), | ||
| $slugs, | ||
| ); | ||
|
|
||
| $this->loop->wait($promises); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Help wanted!
- How to assert the promises (to be exact, the underlying HTTP requests) are performed concurrently?
- Should I assert they are performed concurrently?
- How to refactor it for easiler testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually install ~100 wp.org plugins and look at the cache generation, it feels like the promises are performed concurrently.
Still don't have a way to assert it pragmatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the caching architecture by introducing a cache interface abstraction and implementing a two-tier caching strategy. The main changes include:
- Renaming
CachetoFileCacheto better reflect its file-based implementation - Introducing
CacheInterfacewithArrayCache(in-memory) andCacheProxy(two-tier) implementations - Adding a
warmCachemechanism to preload plugin status results into the in-memory cache
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/WpOrg/Api/CacheInterface.php | Defines the cache interface contract with read/write methods |
| src/WpOrg/Api/FileCache.php | Renames Cache to FileCache and implements CacheInterface |
| src/WpOrg/Api/ArrayCache.php | Implements in-memory cache using an array |
| src/WpOrg/Api/CacheProxy.php | Implements two-tier caching with fast (array) and slow (file) backends |
| src/WpOrg/Api/Client.php | Adds warmCache method to preload multiple plugin statuses |
| src/MarkClosedPluginAsAbandoned.php | Integrates CacheProxy and adds warmCache support for batch operations |
| src/Plugin.php | Registers warmCache event handlers before markClosedAsAbandoned |
| tests/Unit/WpOrg/Api/FileCacheTest.php | Updates test to reflect Cache → FileCache rename |
| tests/Unit/WpOrg/Api/ArrayCacheTest.php | Tests for the new ArrayCache implementation |
| tests/Unit/WpOrg/Api/CacheProxyTest.php | Tests for the new CacheProxy two-tier caching logic |
| tests/Feature/WpOrg/Api/ClientTest.php | Updates mocks to use CacheInterface instead of concrete Cache class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.