WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#46125 closed defect (bug) (fixed)

No cache cleanup after populate_options action call

Reported by: david.binda Owned by: pento
Milestone: 5.1 Priority: high
Severity: normal Version: 5.1
Component: Networks and Sites Keywords: has-patch has-unit-tests commit
Focuses: multisite Cc:
PR Number:

Description

With the refactoring of new site creation, there seems to be a backward incompatibility being introduced.

Previously, calling get_blog_details from within populate_options action callback, created a cache, which got properly purged, which does not seem to be the case anymore.

Hooking a callback to populate_options which calls get_blog_details caches the blog's values at the state at which those are prior the options population - eg.: false for siteurl.

Seems like it can be fixed by calling clean_blog_cache right after the populate_options in the wp_initialize_site function, which was the case prior the refactor.

I'm attaching a unit test covering such behaviour and a patch for this backward compatibility breaking change.

Attachments (2)

46125.diff (1.5 KB) - added by david.binda 9 months ago.
46125.2.diff (2.0 KB) - added by spacedmonkey 8 months ago.

Download all attachments as: .zip

Change History (11)

@david.binda
9 months ago

#1 @pento
9 months ago

  • Milestone changed from Awaiting Review to 5.1

#2 @spacedmonkey
9 months ago

Thanks for the ticket here @davidbinda .

Have you got an example code where we can see the break?

As for the patch couple bit of feedback.

  1. clean_blog_cache( get_current_blog_id() ); should be clean_blog_cache( $site );. We have the site object to hand, we should use it.
  2. A factory should be used to create the test site, some should look this $factory->blog->create( $id );
  3. Instead of update_option use update_blog_option
  4. Needs doc blocks and comments on all doc.

#3 @david.binda
8 months ago

Hey Jonny, thanks for getting back!

Have you got an example code where we can see the break?

The example code was used in the unit test provided along with the patch - it's the bare minimum for reproducing the faulty behaviour. Both, the wpmu_create_blog and update_option were used in there, as it's part of the code which is in place in the implementation where I tested the patch, that's why those were used in there.

I know that wpmu_create_blog is being deprecated, but I think that the backward compatibility is being broken in here.

Hope it helps!

#4 @spacedmonkey
8 months ago

  • Keywords has-patch has-unit-tests commit added
  • Priority changed from normal to high

#5 @spacedmonkey
8 months ago

@david.binda I updated the patch and tested the solution.

I think we are good to go and this should be merged for 5.1

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


8 months ago

#7 @pento
8 months ago

  • Owner set to pento
  • Status changed from new to assigned

#8 @pento
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 44727:

Multisite: After creating a new blog, ensure the blog cache is correctly cleaned up.

Props david.binda, spacedmonkey.
Fixes #46125.

#9 @david.binda
8 months ago

Thank you @spacedmonkey for your help! Really appreciate that.

Note: See TracTickets for help on using tickets.