Conversation
Codecov Report
@@ Coverage Diff @@
## master #88 +/- ##
============================================
+ Coverage 77.75% 79.38% +1.62%
- Complexity 145 158 +13
============================================
Files 1 1
Lines 454 485 +31
============================================
+ Hits 353 385 +32
+ Misses 101 100 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| $this->group_ops_stats( 'get_multiple', $uncached_keys_list, $group, null, $elapsed ); | ||
|
|
||
| foreach ( $uncached_keys as $id => $key ) { | ||
| $found = array_key_exists( $key, $values ); |
There was a problem hiding this comment.
Memcache::get() will return the stored value which may be null. isset() returns false for null values.
| $return[ $id ] = $value; | ||
| $return_cache[ $key ] = [ | ||
| 'value' => $value, | ||
| 'found' => $found, |
There was a problem hiding this comment.
false !== $value will produce an incorrect result if we have stored false for the given key.
| $key = $this->key( $id, $group ); | ||
|
|
||
| if ( isset( $this->cache[ $key ] ) && ( ! $force || $no_mc ) ) { | ||
| $value = $this->cache[ $key ]['found'] ? $this->cache[ $key ]['value'] : false; |
There was a problem hiding this comment.
There is a nice feature: when set() fails, we may end up with some value and found set to false.
I chose to return false because the value is not in Memcached. Returning something else may break add/increment scenarios.
rinatkhaziev
requested review from
WPprodigy,
brandonpayton,
jmdodd and
nickdaugherty
|
I just tagged some folks to whom this may be relevant. |
|
Wanted to mention that last time I played around with the "send an array of keys to get()" functionality, it turned out it handled some edge cases differently - particularly So in |
|
My tests show that we have issues with |
rinatkhaziev
left a comment
rinatkhaziev
left a comment
There was a problem hiding this comment.
Left a few comments of not too much substance. The only part that makes me apprehensive is the $flags handling, but maybe I'm just not fully understanding the implications.
Another thing is is tests we only use small values which is not indicative of what users actually do. Maybe it can be a subject of a separate PR though.
sjinks
force-pushed
the
GH-63
branch
3 times, most recently
from
6f2488d to
0e8645c
Compare
Co-authored-by: Rinat K <rinat@automattic.com>
|
Kudos, SonarCloud Quality Gate passed!
|
4 participants
This PR adds support for the
wp_cache_get_multiple()function introduced in WordPress 5.5.Fixes #63