Opened 4 years ago
Last modified 4 years ago
#40420 new defect (bug)
key/group naming confusion in _get_non_cached_ids()
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Cache API | Keywords: | |
Focuses: | performance | Cc: |
Description
The _get_non_cached_ids()
function uses a parameter named $cache_key
that actually represents a cache group everywhere else within WordPress.
This means when looking at the block or inline documentations, a developer may accidentally be guided down the path of passing in an actual cache-key instead of the cache-group they're checking against.
My recommendation would be to clarify this bit of code to use $group
or $cache_group
rather than $cache_key
, and update any surrounding code documentation to be explicit if it needs it. It's a non-functional change, but will make the code more clear for people.
(Related: BuddyPress comes packaged with it's own similar helper function, and opted to use $cache_group
.)
40420.patch paints the bikeshed in the following ways:
$cache_group
instead of$cache_key
Notes:
$group
instead of$cache_group
everywhere else but 1 place (the customizer) which was newly introduced in 4.7.0. (See:WP_Customize_Manager->find_changeset_post_id()
)$cache_group
everywhere, but that's a lot of code-churn$group
reads too ambiguously to me, especially given the context of BuddyPress GroupsAdditional findings:
!wp_cache_get()
here, which might actually be a logic error. It's plausible this should be a strictfalse
comparison, as the intended functionality is to identify totally uncached keys, and not a value that might be an empty string or array. (This type of problem exists through-out WordPress already though.)