Implement `wp_initialize_site()` and `wp_uninitialize_site()`

[flixos90]

As summarized in https://core.trac.wordpress.org/ticket/40364#comment:14, we'd like to have solid functions for installing a site in a multisite network as well as uninstalling it.

These functions should only take care of the "site-level" part of things, like:

• creating/dropping database tables
• populating options
• setting up initial content

The idea is to only be required to call wp_insert_site() and then wp_install_site() to get a new site set up.

Before we start working on the two functions, we should discuss what exactly it needs to do. This will have to match what currently happens spread out between wpmu_create_blog() and another function it calls, install_blog(). We should aim to make this consistent and also think about ways to enhance these functions and the functions they use, for example by providing filters to adjust some of these defaults.

This will be a rather comprehensive task, so it does not necessarily need to go in at a similar time as the changes from #40364. We should probably even wait to start it until those changes have been completed. This ticket should just be on the horizon for now, I'd say.

[flixos90]

These tickets belong to our planned roadmap (a few of them not per final decision), so flagging with a keyword for better overview.

[flixos90]

This should be a 5.0 priority, together with #40364.

[flixos90]

41333.diff​ is a first take on this:

• A wp_install_site action is introduced and fired after the site's DB row has been inserted and after any other related functionality has been executed (via the wp_insert_site hook).
• A wp_uninstall_site action is introduced and fired before the site's DB row will be deleted and before any other related functionality will be executed (via the wp_delete_site hook).
• A new wp_install_site() function is hooked into wp_install_site. It receives the site object and any arguments passed to wp_insert_site() that are not part of the site's DB row fields.
• A new wp_uninstall_site() function is hooked into wp_uninstall_site. It receives the site object.
• The temporary removal of the action wp_update_blog_public_option_on_site_update() from the update_blog_public hook has been moved into wp_insert_site(). This is necessary since, when the wp_insert_site hook fires, the database tables won't exist yet.
• The removal of site metadata (blogmeta) is now part of wp_delete_site(). This data is in network scope, so it should be covered by this function, while wp_uninstall_site() covers the site scope data.
• wp_install_site() is arguably the most complex part of the patch. It contains logic that was previously part of wpmu_create_blog(). Moreover, install_blog() has been deprecated, and its logic is now also part of wp_install_site(), in a more flexible way. populate_options() now accepts an optional $options array parameter that allows to override some of the initial options with custom values. This parameter is used in wp_install_site(), which allows to get rid of some unnecessary update_option() calls later on.
• wp_uninstall_site() contains logic that was previously part of wpmu_delete_blog(). It removes users from the site, drops the DB tables and removes the uploads directory. It explicitly does not support the $drop parameter, as uninstalling a site is equal to dropping everything. It's thus not a direct replacement for wpmu_delete_blog() because it makes more sense to deal with setting the deleted flag and actually deleting separately.
• wpmu_create_blog() now simply calls wp_insert_site(), passing the $title, $user_id, and $meta arguments to it as necessary.
• wpmu_delete_blog() now simply calls wp_delete_site() if $drop is true. Since wpmu_delete_blog() removes all users from the site even if it only sets the deleted flag, that code may be executed twice (wp_uninstall_site() needs to remove users as well). This is not an issue though, since after the first run those users will already have been removed.
• Any default option that relies on some network value now uses the network that is specified for the new site. It previously would look up the current network, which may not always be the right one (usually it would, but not always).
• The unrelated function install_blog_defaults() has been moved to ms-deprecated.php. It has been deprecated for a long time, but for some reason was still in ms-functions.php.

Future follow-up stuff (that should not be part of this ticket):

• wpmu_create_blog() and wpmu_delete_blog() should no longer be used by core. However I think it's best to make these changes in a release after 5.0, because we wanna make sure the code we have is really backward-compatible first.
• The two functions should then probably be deprecated too.

Open questions from my end:

• In wp_install_site(), passing an empty user ID currently results in a WP_Error. I wonder whether we should use a default, for example the network administrator of the site's network. That would be a good fallback IMO, since that user has access anyway.
• Is the order of the actions the right choice?
  • For inserting a site: DB transaction -> wp_insert_site hook -> wp_install_site hook
  • For deleting a site: wp_uninstall_site hook -> DB transaction -> wp_delete_site hook
• How do we deal with the wpmu_new_blog and delete_blog actions? If we need to maintain those, they need to be moved into wp_insert_site() / wp_delete_site() respectively. We should definitely deprecate them IMO, in favor of wp_insert_site / wp_delete_site.

This whole change needs quite a bit of testing, so please apply the patch and play around with it. It's not that big actually, a lot has been copy-paste only. The most important thing we need to ensure is that site installation and uninstallation (DB tables, correct initial options and content, uploads directory) work correctly.

Unit tests will be provided after some initial feedback and discussion on whether this is on the right track.

[flixos90]

Results from today's office hours (​https://wordpress.slack.com/archives/C03BVB47S/p1533658006000406) and next changes to implement:

• wp_install_site should be renamed to wp_initialize_site.
• wp_uninstall_site should be renamed to wp_uninitialize_site.
• A wp_is_site_initialized() function should be introduced that looks up the site's DB tables, with a short-circuit hook.
• wpmu_new_blog should be deprecated and moved into wp_insert_site().
• delete_blog should be deprecated and moved into wp_delete_site().

[flixos90]

41333.2.diff​ makes the following changes (as described above):

• Use wp_initialize_site and wp_uninitialize_site instead of wp_install_site and wp_uninstall_site.
• Update documentation to reflect the above wording too.
• Introduce wp_is_site_initialized() and a pre_wp_is_site_initialized hook inside it. Use that function in wp_update_blog_public_option_on_site_update() so that that function can always be hooked in regardless of whether a site is initialized or not.
• Deprecate wpmu_new_blog and move it into wp_insert_site().
• Deprecate delete_blog and deleted_blog and move them into wp_delete_site(). Since the hooks still need to be executed when the site is not deleted, but only its deleted flag is set, wpmu_delete_blog() flow has been adjusted and it conditionally runs those actions as well. The parts where $drop is possibly changed due to restrictions has been moved to the top of the function, since otherwise it causes unexpected issues: It does that at the moment, when you call wpmu_delete_blog() for the main site with $drop set to true, the delete_blog filter will receive true although the value will actually be set to false afterwards. The change I made here fixes this. Other than that, the flow in wpmu_delete_blog() remains completely the same as before.

I'd like to discuss whether we should introduce pre-filters for inserting, updating and deleting sites. Those would allow both short-circuiting, but also allow to perform actions before anything is changed. Furthermore, I deprecated delete_blog referring to wp_delete_site as replacement, however a pre_wp_delete_site filter would be a more accurate one.

Another thing I'm asking myself is whether the wp_insert_site hook or wp_initialize_site hook should be run first.

[jeremyfelt]

This is looking really good, @flixos90 - thank you!

I'd like to discuss whether we should introduce pre-filters for inserting, updating and deleting sites. Those would allow both short-circuiting, but also allow to perform actions before anything is changed. Furthermore, I deprecated delete_blog referring to wp_delete_site as replacement, however a pre_wp_delete_site filter would be a more accurate one.

I changed my mind a bit on pre-filters during today's Slack conversation. I think as long as we can define the ways that people can be flexible with the site creation process, then we're good. It does sound like a short-circuit filter for site deletion is a good idea. Site insertion can already be overridden.

Another thing I'm asking myself is whether the wp_insert_site hook or wp_initialize_site hook should be run first.

We just chatted through this in Slack and it seems that we're all on the same page with wp_insert_site being fired first.

And one additional note from a review of the patch:

• It seems like the addition of the $options parameter to populate_options() might deserve a ticket of its own.

[flixos90]

Next steps, based on feedback and multisite meeting discussion:

• open extra ticket for adding an optional $options = array() parameter to populate_options()
• open extra ticket for adding an optional $roles = array() parameter to populate_roles()
• open extra ticket for adding a populate_site_meta( $meta = array() ) function in wp-admin/includes/schema.php
• open extra ticket for adding a populate_network_meta( $meta = array() ) function in wp-admin/includes/schema.php (code for that is currently part of populate_network())
• in wp_initialize_site(), merge options from options argument with default options and pass them to populate_options()
• add support for extra roles on a new site to wp_initialize_site(), via roles argument
• add support for site meta on a new site to wp_initialize_site(), via meta argument
• make site initialization arguments filterable, with a new wp_initialize_site_args filter
• add wp_validate_site_deletion action, where folks can amend a WP_Error in order to prevent site deletion under certain circumstances

[flixos90]

In 43627:

Upgrade/Install: Add support for an optional $options parameter to populate_options().

Fixes #44893. See #41333.

[flixos90]

In 43628:

Upgrade/Install: Introduce populate_network_meta(), moving logic out of populate_network().

Fixes #44895. See #41333.

[flixos90]

In 43629:

Upgrade/Install: Introduce populate_site_meta().

Fixes #44896. See #41333.

[flixos90]

41333.3.diff​ includes the following improvements:

• In wp_initialize_site(), passed options are directly passed to populate_options() for efficient insertion.
• In wp_initialize_site(), a meta argument is now supported, as key => value pairs, which are then passed to populate_site_meta().
• Introduce a wp_initialize_site_args filter to adjust arguments passed to site initialization on the fly.
• Introduce a wp_validate_site_deletion action, that receives a WP_Error object and allows to cancel site deletion.
• Fix a critical bug where sites with an empty user ID would not be initialized. This was previously possible and needs to remain that way.
• Adjust the two utility functions hooked into wpmu_new_blog (which is deprecated) to be hooked into wp_initialize_site instead. A tiny bit of code has been adjusted in both functions to support the different types of parameters from the new action.

As of this patch, all existing unit tests pass without issues.

There are only a few things left to do:

• Complete #44894, and then add support for custom roles to wp_initialize_site().
• Add tests for wp_initialize_site().
• Add tests for wp_uninitialize_site().
• Add tests for wp_is_site_initialized().
• Add tests to ensure wp_insert_site() triggers wp_initialize_site() with proper arguments correctly.
• Add tests to ensure wp_delete_site() triggers wp_uninitialize_site() correctly.
• Remove all actions from wp_initialize_site for all wp_insert_site() tests that don't interact with the site's scope at all, to significantly improve tests performance.
• Add tests for the new wp_validate_site_deletion action.

[spacedmonkey]

I reviewed the latest patch, here is my feedback.

if ( 'https' === parse_url( get_network_option( $network->id, 'siteurl' ), PHP_URL_SCHEME ) ) {

should become

if ( 'https' === parse_url( $network->siteurl, PHP_URL_SCHEME ) ) {

The function install_blog_defaults needs this

_deprecated_function( __FUNCTION__, '5.0.0' );

Breaks this into multiple lines to make it more readable.

 $blog_id = wp_insert_site( 
                                array_merge(
                                        $site_data,
                                        array(
                                                'title'   => $title,
                                                'user_id' => $user_id,
                                                'options' => array_diff_key( $options, array_flip( $site_data_whitelist ) ),
                                        )
                                )
                        );

becomes

 $site_data_default = array(
                            'title'   => $title,
                            'user_id' => $user_id,
                            'options' => array_diff_key( $options, array_flip( $site_data_whitelist ) ),
 );
 $blog_options = array_merge( $site_data, $site_data_default );
 $blog_id = wp_insert_site( $blog_options);

I would change the following as well

$result   = false;
$suppress = $wpdb->suppress_errors();
if ( $wpdb->get_results( "DESCRIBE {$wpdb->posts}" ) ) {
        $result = true;
}
$wpdb->suppress_errors( $suppress );

becomes

$suppress = $wpdb->suppress_errors();
$result =  $wpdb->get_results( "DESCRIBE {$wpdb->posts}" );
$wpdb->suppress_errors( $suppress );

[spacedmonkey]

I would also have a look into change how the unit test factories, so you can pass an param that would disable the tables being created.

The new param create the table and is defaults to true.

$id = $factory->blog->create( $id, true );

where as this

$id = $factory->blog->create( $id, false );

would not create the extra table.

This should really speed up some of the unit tests running.

[flixos90]

Quick summary of yesterday's multisite chat, we decided to not pursue adding specific support for $roles to wp_initialize_site() for now (related: #44894). There are a few complex issues related to that, and we don't wanna expose a specific interface to interact with it that would make developers assume that everything easily works as expected.

As a result, the only remaining part here is solid test coverage for the new functions.

[flixos90]

41333.4.diff​ introduces comprehensive tests for the new functions. It also addresses the issues that @spacedmonkey outlined earlier.

[flixos90]

In 43654:

Multisite: Introduce a site initialization and uninitialization API.

This changeset makes the new CRUD API for sites introduced in [43548] usable for real-world sites. A new function wp_initialize_site(), which takes care of creating a site's database tables and populating them with initial values, is hooked into the site insertion process that is initiated when calling wp_insert_site(). Similarly, a new function wp_uninitialize_site(), which takes care of dropping a site's database tables, is hooked into the site deletion process that is initiated when calling wp_delete_site().

A new function wp_is_site_initialized() completes the API, allowing to check whether a site is initialized. Since this function always makes a database request in its default behavior, it should be called with caution. Plugins that would like to use site initialization in special ways can leverage a pre_wp_is_site_initialized filter to alter that default behavior.

The separate handling of the site's row in the wp_blogs database table and the actual site setup allows for more flexibility in controlling whether or how a site's data is set up. For example, a unit test that only checks data from the site's database table row can unhook the site initialization process to improve performance. At the same time, developers consuming the new sites API only need to know about the CRUD functions, since the initialization and uninitialization processes happen internally.

With this changeset, the foundation for a sites REST API endpoint is fully available. The previously recommended functions wpmu_create_blog() and wpmu_delete_blog() now call the new respective function internally. Further follow-up work to this includes replacing calls to wpmu_create_blog() with wp_insert_site(), update_blog_details() with wp_update_site() and wpmu_delete_blog() with wp_delete_blog() throughout the codebase.

As a side-effect of this work, the wpmu_new_blog, delete_blog, and deleted_blog actions and the install_blog() function have been deprecated.

Fixes #41333. See #40364.

[flixos90]

In 43655:

Multisite: Fix coding standard errors after [43654].

See #41333.

[TimothyBlynJacobs]

Minor, but should this be WP_Error::has_errors() instead of ! empty( WP_Error::$errors ). in ms-blogs line 598?

[flixos90]

@TimothyBlynJacobs Good catch! Same in line 881. Will fix this in a later commit, as it's not critical.

[flixos90]

Version numbers need to be adjusted to 5.1.0.

[jrf]

Version numbers need to be adjusted to 5.1.0.

See #45657 ;-)

[flixos90]

In 44469:

Multisite: Update @since tags for site management APIs.

Fixes #40364. Fixes #41333.