Automattic / wp-cache-memcached

Memcached Object Cache for WordPress
6 stars 0 forks source link

Improve Non-Persistent Group Handling in WP_Object_Cache (Sentinel Value) #10

Open mreishus opened 2 months ago

mreishus commented 2 months ago

Improve Non-Persistent Group Handling in WP_Object_Cache

This PR addresses a critical bug in the handling of non-persistent cache groups, particularly affecting the 'themes' group. The issue prevented the add() method from working correctly after a failed get() operation on a non-existent key.

Key Changes:

  1. Introduced a NOT_SET_SENTINEL constant to distinguish between truly non-existent keys and keys that have been checked but not found.
  2. Modified add(), replace(), get(), and delete() methods to properly handle the NOT_SET_SENTINEL for non-persistent groups.
  3. Test coverage added to ensure correct behavior in various scenarios.

Bug Fix:

Previously, when attempting to get() a non-existent key in a non-persistent group (like 'themes'), the cache would mark this key as existing but empty. This prevented subsequent add() operations from succeeding, as the key was incorrectly considered to already exist.

The new implementation allows add() to succeed after a failed get(), which is the expected behavior for a cache system.

Benefits:

Testing:

Tests have been added to cover individual operations, multiple operations, edge cases, and the specific scenario that triggered the original bug.

This change is crucial for ensuring correct behavior in WordPress installations using non-persistent cache groups, particularly for theme-related operations.

To test, run bin/test.sh.

mreishus commented 2 months ago

Alternate implementation w/o a sentinel value is here: #9. But it has the disadvantage of wp_cache_delete('key_in_nonpersistent_group') acting differently than than unset( $this->cache['key_in_nonpersistent_group'] );.