Use get_site in clean_blog_cache

[spacedmonkey]

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.

[johnjamesjacoby]

This seems sound.

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

Should be: if ( is_object( $blog )

[spacedmonkey]

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

[flixos90]

+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.

[flixos90]

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

[flixos90]

40201.diff​ builds upon the previous patch with the goal to become a 1-to-1-replacement for refresh_blog_details(). The function should be able to also deal with an ID for a site that has just been deleted (like refresh_blog_details() handles it). The patch deprecates refresh_blog_details() and its filter of the same name. The filter has been moved into clean_blog_cache() so that it still fires for code that now calls that function instead of refresh_blog_details().

Note that this is rather experimental: I definitely think we should aim for clean_blog_cache() to become a 1-to-1-replacement. Whether we deprecate the old function and filter immediately is another question - I just did it in this patch, but it can be easily reversed. If the function is deprecated, we also need to replace all calls to it in core with clean_blog_cache().

Putting this into 4.9 to give it some more thought. It aligns well with approaches like #40364 and #40180 / #40228. Also adding back the needs-unit-tests keyword, since you probably forgot to upload the tests previously @spacedmonkey :)

[flixos90]

I think we can very well do this change in 4.9, but I'd rather hold off with the deprecation part. We can even replace all core calls to refresh_blog_details() with calls to clean_blog_cache(), but let's discuss deprecation in a broader scope, together with get_blog_details() and update_blog_details() both of which will be able to be deprecated after #40228 and #40364. This would furthermore allow us to deprecate early in a cycle vs a week before Beta now.

[spacedmonkey]

I updated the patch. 40201.2.diff

Refactor the logic a bit here.

• Set default param to 0 on clean_blog_cache. This means you can call clean_blog_cache() and it will just clear the current site- which is useful.
• Pass all input through get_site, which will cast, null, int, object and wp_site to wp_site. Making sure we are dealing with wp_site object going forward.

[flixos90]

40201.3.diff​ makes the following changes:

• Do not deprecate the refresh_blog_details() function, only the action.
• Bail if an empty parameter is given to clean_blog_cache().
• Bail if no site could be found in clean_blog_cache() and the parameter is not numeric.

[flixos90]

Tests to verify original behavior

[flixos90]

In 41715:

Multisite: Add specific tests for clean_blog_cache() and refresh_blog_details().

See #40201.

[flixos90]

In 41716:

Multisite: Establish clean_blog_cache() as a replacement for refresh_blog_details().

Going forward, clean_blog_cache() is recommended to be used instead of refresh_blog_details(). It has been adjusted to match the functionality of the latter, with the exception that it always requires a site ID or object to be passed. The refresh_blog_details action has been deprecated in favor of the clean_site_cache action. The function itself is not formally deprecated at this point, but will likely be in the near future.

Props spacedmonkey.
Fixes #40201.

[flixos90]

In 41717:

Multisite: Replace calls to refresh_blog_details() with clean_blog_cache().

Fixes #42077. See #40201.