WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 2 weeks ago

#40201 new enhancement

Use get_site in clean_blog_cache

Reported by: spacedmonkey Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch has-unit-tests dev-feedback
Focuses: multisite Cc:

Description

The clean_blog_cache takes a generic object as a param. This is a little strange as other clean_*_cache functions either take a list of ids or an ID. Now that the WP_Site object is in core, we should make sure we use it in this function. If we pass the $blog param through the get_site function it would have the following benefits.

  • All objects passed to function will be cast to WP_Site object. This would mean that the object would always domain and path properties.
  • Ability to pass a site id, as get_site allows for id lookup.
  • Better error handling. This function doesn't handle not passing an object very well.

Attachments (3)

40201.patch (716 bytes) - added by spacedmonkey 5 months ago.
40201.2.patch (954 bytes) - added by spacedmonkey 5 months ago.
40201.3.patch (1018 bytes) - added by spacedmonkey 5 months ago.

Download all attachments as: .zip

Change History (13)

#1 @spacedmonkey
5 months ago

  • Keywords needs-patch needs-unit-tests 2nd-opinion added

@spacedmonkey
5 months ago

#2 @spacedmonkey
5 months ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


5 months ago

#4 @johnjamesjacoby
5 months ago

This seems sound.

if( is_object( $blog ) needs a whitespace fix before it's 100% ready for core.

Should be: if ( is_object( $blog )

#5 @spacedmonkey
5 months ago

  • Keywords has-unit-tests dev-feedback added; needs-unit-tests 2nd-opinion removed

Updated the patch. Now does better type checking. Patch also includes tests.

#6 @flixos90
5 months ago

+1 for this enhancement.

I just reviewed the latest patch and noticed that it doesn't handle the case where a site has just been deleted (get_site() will return null when passing an ID to it then). To fix that, we can use the logic that is currently part of refresh_blog_details(). It could probably be removed from that function afterwards.
Also I don't think we need to check is_object( $blog ) || is_int( $blog ) - due to the documented parameter we can simply pass $blog through get_site(). If the result is empty and $blog is numeric, the logic I mentioned above can be used to create a fake object.

There's something else to consider though: Maybe we should hold off with that enhancement until the changes for get_blog_details() have been merged (see #40180 and #40228 for background). With that, and with removing the blog-id-cache lookup in get_blog_id_from_url() (which doesn't make sense to have in there anymore, now that WP_Site_Query results are cached anyway; there's no ticket for that yet though) we don't need to use any of those weird caches anymore that consist of domain and path. That means that the only part of clean_blog_cache() that needs a full object would be the action hook. Not sure if we can work around it completely, but we could definitely optimize the clean_blog_cache() function even more once those changes are in place.

#7 @flixos90
5 months ago

I opened #40362 and #40363 as small related enhancements for clean_blog_cache().

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


2 weeks ago

Note: See TracTickets for help on using tickets.