Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#46125 closed defect (bug) (fixed)

No cache cleanup after populate_options action call

Reported by: davidbinda's profile david.binda Owned by: pento's profile 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:

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 6 years ago.
46125.2.diff (2.0 KB) - added by spacedmonkey 6 years ago.

Download all attachments as: .zip

Change History (11)

@david.binda
6 years ago

#1 @pento
6 years ago

  • Milestone changed from Awaiting Review to 5.1

#2 @spacedmonkey
6 years 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
6 years 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!

@spacedmonkey
6 years ago

#4 @spacedmonkey
6 years ago

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

#5 @spacedmonkey
6 years 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.


6 years ago

#7 @pento
6 years ago

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

#8 @pento
6 years 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
6 years ago

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

Note: See TracTickets for help on using tickets.