#56605 closed defect (bug) (fixed)
Reconsider wp_cache_supports_group_flush implementation
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | 6.1 |
Component: | Cache API | Keywords: | 2nd-opinion changes-requested has-patch has-unit-tests add-to-field-guide |
Focuses: | Cc: |
Description
r53767 introduced the ability to flush cache groups via #4476, but I believe it may include a non-trivial oversight.
The new wp_cache_supports_group_flush()
function simply returns true
or false
, and will require a drop-in plugin to redefine this function in a pluggable way to override its return value.
This approach does not empower the $wp_object_cache
global Class to define for itself whether or not its backend supports flushing by cache group. Instead, it relies on the internal exceptions of wp_start_object_cache()
to skip loading wp-includes/cache.php
(which returns unpluggable true
) and load wp-includes/cache-compat.php
(which returns pluggable false
).
This makes this new "flag" or "setting" unique relative to the other Cache API functions located in wp-includes/cache.php
, which my intuition tells me is likely not the ideal DUX, or an intentional implementation detail for this new feature in WordPress 6.1.
I would like to politely suggest that before 6.1 ships without it, that the WP_Object_Cache
be considered to have added to it some method of checking and reporting if the cache backend it interfaces with supports flushing groups (or potentially any future thing?)
Perhaps that means a new WP_Object_Cache::supports( $feature = '' ) {}
method and a new wp_cache_supports( $feature = '' ) {}
function?
Perhaps that just means a new WP_Object_Cache::can_flush_groups()
method specifically (though, I'd hate to add it now with the idea of deprecating it later for supports()
anyways.)
Thoughts?
Change History (13)
#1
@
3 years ago
- Summary changed from Move supports_group_flush to Reconsider wp_cache_supports_group_flush implementation
#2
in reply to:
↑ description
@
3 years ago
#3
@
2 years ago
@tillkruess What do you think?
@johnjamesjacoby Any chance of a POC PR to better show what you mean?
#5
@
2 years ago
The wp_cache_supports()
sounds like a good idea, because WP_Object_Cache
may not be present.
@johnjamesjacoby Would each plugin supply it as usual and a fallback in core?
This ticket was mentioned in PR #3347 on WordPress/wordpress-develop by SergeyBiryukov.
2 years ago
#6
- Keywords has-patch has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/56605
#8
@
2 years ago
Are there any other features like the wp_cache_*_multiple()
methods we want to allow checking support for?
tillkruss commented on PR #3347:
2 years ago
#9
Are there any other features like the wp_cache_*_multiple()
methods we want to allow checking support for? It would be handy to have for various diagnostics.
#10
follow-up:
↓ 11
@
2 years ago
@SergeyBiryukov You can commit if you wish. However, dev notes will have to be updated. CC @milana_cap
#11
in reply to:
↑ 10
@
2 years ago
- Keywords add-to-field-guide added
Replying to spacedmonkey:
@SergeyBiryukov You can commit if you wish. However, dev notes will have to be updated. CC @milana_cap
Ticket added to my watch list. Thank you @spacedmonkey
SergeyBiryukov commented on PR #3347:
2 years ago
#13
Merged in r54448.
Thanks for the ticket!
Replying to johnjamesjacoby:
That makes sense to me and seems more future-proof.
It looks like the current implementation is the result of the discussion on PR #2368 Delete Cache by Group.