Caches: Refactored API #1060
Caches: Refactored API #1060
Conversation
For consistency, functions should always return null on non-existing data.
WordPressPluginUpdateBridge appears to have used its own cache instance in the past. Obviously not used anymore.
Since $key can be anything, the cache implementation must ensure to assign the related data reliably; most commonly by serializing and hashing the key in an appropriate way.
Even though the default path for storage is perfectly fine, some people may want to use a different location. This is an example how a cache implementation is responsible for its requirements.
There was a problem hiding this comment.
This looks good to me 👍
There are two things which I think should be worked on, however. One is mentioned below, the other is about protected functions. Maybe they should be made private to prevent confusion (because they are not supported by the interface and nobody should actually inherit these classes).
So, functions like getPath, getCacheFile and getCacheName in FileCache can be private and don't need to do additional parameter validation if this is already done by the calling function.
The same should be true for getCacheKey in SQLiteCache, right?
Co-Authored-By: fulmeek <36341513+fulmeek@users.noreply.github.com>
Done earlier, but forgotten to actually add these changes to the previous commits.
The previous code suggestions to set directory permissions to 644 had to reverted to 755 as the executable bit is required to change into that folder.
|
Since it's been a while, any news? |
Yes, I'm back 🤣 Seriously though, the changes look fine to me. Good job 👍 |
- For consistency, functions should always return null on non-existing data. - WordPressPluginUpdateBridge appears to have used its own cache instance in the past. Obviously not used anymore. - Since $key can be anything, the cache implementation must ensure to assign the related data reliably; most commonly by serializing and hashing the key in an appropriate way. - Even though the default path for storage is perfectly fine, some people may want to use a different location. This is an example how a cache implementation is responsible for its requirements.
Addressing issue #1037, this changes the Caches interface and current implementations to make it usable for different kind of caches:
loadData,saveData,getTimeandpurgeCachestay the same. You only have to ensure they consistently returnnullon non-existing data.setPath($path)andsetParameters(array $param)were designed with file based caching in mindsetScope($scope): Set scope of the current cache (replacessetPath)If
$scopeis an empty string, the cache is set to a global context. No absolute path anymore.setKey($key): Set key to assign the current data (replacessetParameters)Since
$keycan be anything, the cache implementation must ensure to assign the related data reliably; most commonly by serializing and hashing the key in an appropriate way.SQLiteCacheas an example.Feel free to add comments and suggestions :)