[wp-trac] [WordPress Trac] #20004: Fix NULL and FALSE in WP_Object_Cache and make found/not-found unambiguous.

[WordPress Trac]

#20004: Fix NULL and FALSE in WP_Object_Cache and make found/not-found unambiguous.
------------------------------------+------------------------------
 Reporter:  andy                    |       Owner:
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Cache                   |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |
------------------------------------+------------------------------
Description changed by andy:

Old description:

> ```WP_Object_Cache``` mistreats ```NULL``` as a stored value by
> converting it to ```""```. To handle it correctly, the existence check
> must not use ```isset()```:
>
> {{{
> $cache[ $key ] = NULL;
> isset( $cache[ $key ] ); // false
> array_key_exists( $key, $cache ); // true
> }}}
>
> The attached patch corrects the treatment of ```NULL``` by storing it as-
> is and using ```array_key_exists()``` to test the existence of a key.
>
> As a value returned from ```wp_cache_get```, ```FALSE``` is ambiguous. It
> could mean that the value stored and retrieved was literally ```FALSE```
> or that the key was not found.
>
> The attached patch moves the found/not-found information to a pass-by-
> reference parameter, ```$found```, which is set to ```TRUE``` after a
> cache hit or ```FALSE``` after a cache miss. This allows storage and
> retrieval of ```FALSE``` without ambiguity and reduces the temptation to
> use confusing patterns such as ```!$value```, ```$value === false```, and
> ```empty( $value )```.
>
> The attached patch permits a clearer coding style:
>
> {{{
> $found = null;
> $value = wp_cache_get( $key, $group, false, $found );
> if ( ! $found ) {
>     $value = $wpdb->get_results( $query );
>     wp_cache_set( $key, $value, $group );
> }
> }}}
>
> (Assigning ```$found = null;``` prevents "Notice: Undefined variable".)
>
> However, because object cache drop-ins may lag behind core in adding
> support for the ```$found``` parameter, the following pattern may be
> preferred for backwards compatibility:
>
> {{{
> $found = null;
> $value = wp_cache_get( $key, $group, false, $found );
> if ( ! $value && ! $found ) {
>     $value = $wpdb->get_results( $query );
>     wp_cache_set( $key, $value, $group );
> }
> }}}
>
> I also added a function, ```wp_cache_found()```, to determine whether the
> previous retrieval was a hit or a miss.
>
> {{{
> $value = wp_cache_get( $key, $group );
> if ( ! wp_cache_found() ) {
>     $value = $wpdb->get_results( $query );
>     wp_cache_set( $key, $value, $group );
> }
> }}}
>
> Again, this function may not be present in drop-ins. Until such time as
> old drop-ins are forced into obsolescence, use ```function_exists```
> before calling ```wp_cache_found``` in core code.
>
> It would behoove us to devise a drop-in API versioning and update system
> to assist with this.

New description:

 ```WP_Object_Cache``` mistreats ```NULL``` as a stored value by converting
 it to ```""```. To handle it correctly, the existence check must not use
 ```isset()```:

 {{{
 $cache[ $key ] = NULL;
 isset( $cache[ $key ] ); // false
 array_key_exists( $key, $cache ); // true
 }}}

 The attached patch corrects the treatment of ```NULL``` by storing it as-
 is and using ```array_key_exists()``` to test the existence of a key.

 As a value returned from ```wp_cache_get```, ```FALSE``` is ambiguous. It
 could mean that the value stored and retrieved was literally ```FALSE```
 or that the key was not found.

 The attached patch moves the found/not-found information to a pass-by-
 reference parameter, ```$found```, which is set to ```TRUE``` after a
 cache hit or ```FALSE``` after a cache miss. This allows storage and
 retrieval of ```FALSE``` without ambiguity and reduces the temptation to
 use confusing patterns such as ```!$value```, ```$value === false```, and
 ```empty( $value )```.

 The attached patch permits a clearer coding style:

 {{{
 $value = wp_cache_get( $key, $group, false, $found );
 if ( ! $found ) {
     $value = $wpdb->get_results( $query );
     wp_cache_set( $key, $value, $group );
 }
 }}}

 However, because object cache drop-ins may lag behind core in adding
 support for the ```$found``` parameter, the following pattern may be
 preferred for backwards compatibility:

 {{{
 $value = wp_cache_get( $key, $group, false, $found );
 if ( ! $value && ! $found ) {
     $value = $wpdb->get_results( $query );
     wp_cache_set( $key, $value, $group );
 }
 }}}

 I also added a function, ```wp_cache_found()```, to determine whether the
 previous retrieval was a hit or a miss.

 {{{
 $value = wp_cache_get( $key, $group );
 if ( ! wp_cache_found() ) {
     $value = $wpdb->get_results( $query );
     wp_cache_set( $key, $value, $group );
 }
 }}}

 Again, this function may not be present in drop-ins. Until such time as
 old drop-ins are forced into obsolescence, use ```function_exists```
 before calling ```wp_cache_found``` in core code.

 It would behoove us to devise a drop-in API versioning and update system
 to assist with this.

--

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/20004#comment:5>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software