Make WordPress Core

Opened 7 years ago

Last modified 7 years ago

#40646 new enhancement

Move code from `ms-settings.php` into functions

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: needs-unit-tests has-patch ms-roadmap
Focuses: multisite Cc:

Description

As briefly discussed during yesterday's office-hours, it would be a good idea to move most of the bootstrap code from ms-settings.php into separate functions. ms_load_current_site_and_network() which was added in [37475] was a good first step in that direction. Let's introduce further functions (possibly small units) to make ms-settings.php less complex and more functionality testable.

Once ms-settings.php becomes more lightweight, we may even think about removing that file and handling all of it in wp-settings.php, as this would give a better overview of the entire core bootstrapping process.

This ticket is a subticket for #40623.

Attachments (1)

40646.diff (8.7 KB) - added by flixos90 7 years ago.

Download all attachments as: .zip

Change History (7)

@flixos90
7 years ago

#1 @flixos90
7 years ago

  • Keywords has-patch added; needs-patch removed

40646.diff introduces the following functions, all of which handle part of what is currently procedural code in ms-settings.php:

  • ms_detect_current_site_and_network(): Calls ms_load_current_site_and_network(), but gets the current domain and path before. If $current_blog and $current_site have already been populated prior (for example through a custom sunrise.php), the function ensures the two are WP_Site/WP_Network objects and simply returns true. Question: Do we need to make $domain and $path global (since they implicitly were before)?
  • ms_get_request_domain(): Utility function to get the current domain to detect the site and network from.
  • ms_get_request_path(): Utility function to get the current path to detect the site and network from.
  • ms_handle_bootstrap_result(): Accepts the result of ms_detect_current_site_and_network(), and dies if false, or redirects if it is a string (redirect URL). This is the only function that's not testable.
  • ms_setup_vars(): Accepts a WP_Site and WP_Network object (usually the current site and network) and populates further global variables accordingly. Database and cache are also initialized.

The procedural code in ms-settings.php is now a lot simpler as it only calls the above functions (plus a few that existed prior and are not testable as they're setting constants) and runs the ms_loaded hook.

While there are a few things that now happen in a slightly different order, I particularly paid attention not to change things around that one could somehow intercept (and thus run into possible BC issues). Probably the most important change is that the forced conversion to WP_Site and WP_Network happens a bit earlier, furthermore it doesn't happen when calling Core's ms_load_current_site_and_network(), since it instantiates those objects anyway.

Also some code that was previously in the if ( ! isset( $current_site ) || ! isset( $current_blog ) ) clause now runs regardless of that check (particularly setting some globals such as $blog_id), but those variables are only set if they aren't set prior, so it will work the same way. Plus it makes the job easier for custom sunrise.phps, since they don't need to manually write $blog_id = $current_blog->id anymore.

#2 @flixos90
7 years ago

Further idea: @jeremyfelt brought up yesterday that we might simply introduce ms_bootstrap() and have that as the only function call in ms-settings.php. This is probably a good idea as well, but I wanted to create smaller bits and pieces first in order to have some more testable code (ms_bootstrap() in its entirety would probably not be testable).

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


7 years ago

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


7 years ago

#5 @flixos90
7 years ago

  • Keywords ms-roadmap added
  • Milestone changed from Awaiting Review to Future Release

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


7 years ago

Note: See TracTickets for help on using tickets.