[#13452] - Add a "contains" method to cache API
- Fixed in Code Base
- 6 Jan 2017
- Medium
- Build: staging
- # 13452
- Diff
- mbabker:cache-contains
User tests: Successful: Unsuccessful:
Pull Request for Issue #12893
Summary of Changes
Creates JCache::contains() and JCacheStorage::contains() as a means to look into the cache pool to determine if a cache item with a specified key exists. Implements this method in many places where a $cache->get() === false type of check had been in use previously.
Testing Instructions
With caching enabled, things should still work as expected. Explicitly, you can store something to the cache then check for its existence with something like this:
$id = 'testCacheContainsItem';
$data = 'testData';
/** @var JCacheControllerCallback $cache */
$cache = JFactory::getCache('_testing');
if ($cache->store($data, $id) === false)
{
throw new Exception('Initial Store Failed');
}
var_dump($cache->contains($id));Documentation Changes Required
None, but it should probably be noted there may be a small (if it's even noticeable) performance hit with this.
Right now, most places just do a single $cache->get() call and if the result exists then it is used otherwise it does the operation that generates the cache data and stores it. With this, two calls to the cache store will be made. However, except for Memcache (no trailing d) and Cache_Lite, all of the stores have a way to check if a key exists in the store which is more performant. The get() methods will all read the data out of the store and do any processing required to make it usable to the PHP API (for example our filesystem handler does a string replacement to remove the PHP opening tag so the file's contents can be unserialized) while the contains() method simply checks the catalog to determine presence, a more efficient lookup operation.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_login Front End com_banners com_finder Libraries Modules Unit Tests |
IMHO For performance reason I would suggest to change it.
- usual if you use
contains()then later you will callget() - it is not atomic (
containsmay return true but later in codeget()won't return value because it expired) - for a few cache storages
contains()equivalent does not exist and we have to callget(). If we need that value then we have to callget()again which is not good for performance.
Suggestion for cache method:
- change current method
get(...)toget($id, $group = null, $raiseIfMissing = false)which may raise an exception if key not existed. - or add to
contains()additional parameter likecontains($id, $group = null, &$value = null), then if cache contains key then$valuewill be set up.
What do you thing?
No caching API I have reviewed or worked with operates in this way and I personally would find that behavior confusing and inconsistent (essentially you're saying contains() should be able to return the value through a reference and I don't think a user configurable parameter should dictate whether the API throws an Exception).
You're right that it is not atomic. Every caching API has this same issue. There isn't a good solution for that. It's also not an issue unique to caching operations, filesystem operations have this same issue (in between the time it takes one to check a file exists and to read the file it could be deleted by another process as an example).
I have tested this item
Test OK
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13452.
Another options which probably won't convince you:
get(...)with additional parameter likeget($id, $group = null, &$found = null). If key is found then$found = trueotherwisefalse.- or
get(...)with additional parameter likeget($id, $group = null, $default = false). This way we can call$cache->get('key', null, false);and checkif value === falsewithout care about what internal handler value is returned if key has not been found.
This way we can have:
contains()to simple test key exists;- and extended
get()for atomiccontains()withget()together.
It's still violating SRP. I'd rather just leave the API alone and close this over add something to a signature to create a singular atomic method. And again, it's not a "flaw" in this implementation or isolated to cache usage; any CRUD operation to a data source can (and probably will at some point in its lifetime) run into an issue because of non-atomicy.
If contains() (for some handlers that call get in contains()) can in the future cache data in a variable in the handler until get() will be call then I agree with that.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13452.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-01-06 18:32:34 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
|
||
I have tested this item✅ successfully on 492f05c
Tested the functionality with the administrator login module.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13452.