[wp-trac] [WordPress Trac] #53804: Bug: `wp_cache_themes_persistently` only used for setting, not getting

[WordPress Trac]

#53804: Bug: `wp_cache_themes_persistently` only used for setting, not getting
--------------------------+------------------------------
 Reporter:  mattwiebe     |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Themes        |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:  administration
--------------------------+------------------------------

Comment (by SergeyBiryukov):

 Thanks for the ticket!

 > The class checks this property throughout before setting cache values

 At a glance, this doesn't seem quite correct. As far as I can tell, the
 `$persistently_cache` property is used for three things in `WP_Theme`:
 1. To decide whether to create a [source:tags/5.8/src/wp-includes/class-
 wp-theme.php?marks=197,202#L189 persistent or non-persistent cache group]
 in `WP_Theme::__construct()`.
 2. To set the [source:tags/5.8/src/wp-includes/class-wp-
 theme.php?marks=198-200#L189 `$cache_expiration` property], if the
 `$persistently_cache` value is integer.
 3. To sanitize and [source:tags/5.8/src/wp-includes/class-wp-
 theme.php?marks=759-767#L743 cache all the headers] in `WP_Theme::get()`
 when using persistent cache.

 Other than that, the property is not checked before any other of the 11
 calls to `WP_Theme::cache_add()` in the class, they appear to be the same
 for both persistent and non-persistent cache. So it looks like the
 property is not quite used for setting cache values in the way the ticket
 title suggests, except for the headers in `WP_Theme::get()`.

 > The simple fix would be to check for `self::$persistently_cache` in the
 `cache_get` method, always returning `false` in that case.

 Since the property is not checked in `WP_Theme::cache_add()` or
 `WP_Theme::cache_delete()`, it seems like checking it in
 `WP_Theme::cache_get()` would introduce an inconsistency. Wouldn't that
 essentially break persistent caching for themes? Perhaps a more targeted
 cache invalidation might help?

 I might be misunderstanding though, would appreciate any clarification :)

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53804#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform