Improve site creation in multisite

[jeremyfelt]

wpmu_create_blog() is responsible for a lot. It uses insert_blog(), install_blog(), wp_install_defaults() and many other things to accomplish the task of creating a site.

We should go through this process and determine what kind of improvements can be made as part of wpmu_create_blog() or in a new wp_create_site()/wp_insert_site() function that is more in tune with the objectives of site creation through the REST API. See #40365.

[flixos90]

40364.diff​ introduces new functions wp_insert_site(), wp_update_site() and wp_delete_site() (in ms-blogs.php). As briefly discussed in today's office-hours, the functions follow a more modern approach parameter-wise, as all of them accept a WP_Site object and return a WP_Site object as well (or WP_Error on failure):

• wp_insert_site( $site ) accepts a WP_Site which must not have the $id property set. After having inserted the new site, it sets the $id property and returns the modified object (or WP_Error on failure).
• wp_update_site( $site ) accepts a WP_Site which must have the $id property set. After having updated the site, it returns the object (or WP_Error on failure).
• wp_delete_site( $site ) accepts a WP_Site which must have the $id property set. After having deleted the site, it sets the $id property to 0 and returns the modified object (or WP_Error on failure).

Passing objects around has several advantages:

• The properties are already defined, so we don't need as much of complex default logic that we'd have otherwise.
• It's straightforward. Instantiate a new site object, set properties and pass it to wp_insert_site().
• You can work with the same object instance, less unnecessary object instantiations from redundant get_site() calls.
• Passing objects around also opens up possibilities for more flexible filters as the objects contain much more information than just an ID. A good example is that we can now immediately pass the object to clean_blog_cache() without some weird logic such as the one in refresh_blog_details().

In addition to the above three functions, a new function is introduced for running the status transition hooks (previously located in update_blog_details()). This functionality is now part of wp_update_site(), but it should be outsourced to limit complexity of that function.

Furthermore the patch uses the three new functions: insert_blog() now wraps wp_insert_site(), update_blog_details() now wraps wp_update_site() and wpmu_delete_blog() calls wp_delete_site() internally.

This patch of course needs thorough review and testing (I haven't tested it at all tbh, will do that soon). Things to consider in particular:

• Is the approach clear enough / flexible / stable enough?
• Since objects are passed by reference, the input objects are directly modified. Is that acceptable, or should we clone them in all of the three functions to require the caller to use the returned object?
• There are some //TODO annotations in the patch with further things to think about.

[flixos90]

See #38828 for things we should focus on improving with our new wp_update_site() function compared to the current way update_blog_details() works.

[boonebgorges]

#38828 was marked as a duplicate.

[flixos90]

Relevant summary from last week's multisite meeting:

• The patch needs to be changed to use the bad old $args pattern for now. A new pattern like passing around WP_Site objects should be discussed in a broader scope, affecting all similar core functions. It's not suitable here as it would just introduce a new pattern others might disagree with and thus cause inconsistency. I'll update the patch soon.
• wp_insert_site(), wp_update_site() and wp_delete_site() deal with the wp_blogs table row only (a site in the network context). This ticket is only about implementing these three functions.
• We'll also implement wp_install_site() and wp_uninstall_site(). The first will be responsible for setting up a site's DB tables, populating default options, initial content etc. The latter will be its counterpart, responsible for dropping everything from a site. This will be part of a separate ticket.
• The last step will be to implement wp_create_site() (wraps wp_insert_site() and wp_install_site()) and wp_remove_site() (wraps wp_uninstall_site() and wp_delete_site()). The name for the latter could probably be more clear, so let's think about a more suitable name. Having those two functions available makes it easy for developers, as they then only need to call one function instead of two when creating/removing a site.

[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]

40364.2.diff​ is a patch for the new functions following the details that we recently discussed:

• It introduces wp_insert_site( $data ), wp_update_site( $site_id, $data ) and wp_delete_site( $site_id ). All of these have the sole purpose of interacting with the wp_blogs database table.
• Each of the three functions introduces its individual action hook where the site object (or in the case of wp_update_site() the current and previous site object) is passed to the hook.
• Everything that goes beyond handling the database row and the related cache is not part of the respective function, but instead hooked in via the aforementioned action hooks. This includes the updating network counts process (via a new wp_maybe_update_network_site_counts_on_update() function) and the site status transition triggers (via a new wp_maybe_transition_site_statuses_on_update() function).
• The new wp_maybe_transition_site_statuses_on_update() fires the hooks that were previously part of update_blog_details(). Two new make_public_blog and make_private_blog hooks have been introduced to complete the rest of them.
• It has always been the case that when the public status changes, the respective site's blog_public option is changed accordingly. Since this is an action that happens in site scope and not in network scope (like all the rest), it must not be tied in with the functionality. Therefore the new make_public_blog and make_private_blog actions are used to hook in the new functions wp_make_blog_public_on_site_update() and wp_make_blog_private_on_site_update in order to take care of that functionality. In cases where we only wanna test interaction with wp_blogs without actually creating the site's individual DB tables (in unit tests for example), those hooks can be temporarily removed to not cause any issues.
• insert_blog() now wraps wp_insert_site() and has been deprecated. wpmu_create_blog() has been adjusted to use wp_insert_site().
• update_blog_details() now wraps wp_update_site().
• wpmu_delete_blog() now uses wp_delete_site() to delete the wp_blogs row.
• I ran all existing related unit tests to verify the changes work in a backward-compatible fashion. In addition I introduced several tests for the three new main functions.

Open questions (particularly important for the review):

• Should wp_insert_site() and wp_update_site() contain any additional restrictions? At the moment, they do not check for example whether a domain and path combination doesn't already exist. I personally think that's fine as those are low-level functions and the wp_create_site() that we're going to introduce later will take care of these checks (that are currently handled in wpmu_create_blog() too and were not present in insert_blog()). The only check that is made in these functions is that a domain is always present. Furthermore if the path is empty, a simple slash is used.
• When no site_id (network ID) is passed, the main network ID is used. Is that desired or should it rather use the current network ID?
• update_blog_details() has now just become a simple wrapper for wp_update_site(). I generally would say that it should be deprecated, but since it is so widely used, I'm not sure that's a good step. What are the thoughts here? This is different to insert_blog() which is very rarely used.

[jeremyfelt]

This is looking good, thanks @flixos90! I added a slightly tweaked patch. Some comments/thoughts:

Should wp_insert_site() and wp_update_site() contain any additional restrictions?

• It's tempting, but it's never happened at this low level before (insert_blog(), update_blog_details()), and I think we can avoid it. It's probably worth thinking about some more before we implement. Domain and path validation would specifically be interesting if it fit.
• I did update the patch to trim the domain before checking emptiness.

When no site_id (network ID) is passed, the main network ID is used. Is that desired or should it rather use the current network ID?

• I think we should use the current network ID if one is not specified. I would probably specify it every time myself, but I can imagine being confused in a multi network environment if the site ended up on another network when I didn't.
• Related: In wp_update_site(), what case would lead to site_id being empty? It should already have one from the old site object. Are we protecting against false or 0 being passed to wp_update_site()? If so, maybe it should fall back to the old site's value instead of the main/current network?

update_blog_details() has now just become a simple wrapper for wp_update_site(). I generally would say that it should be deprecated, but since it is so widely used, I'm not sure that's a good step. What are the thoughts here?

• How widely used is it outside of core? My gut says that we can probably deprecate it, but I'm not completely sure.

And the remaining:

• This isn't a new problem, and could be another ticket, but should we think about adding a check for the new site's options table in WP_Site's get_details()? If I do wp_insert_site( $args ) and then immediately look for get_site( 2 )->home, I'll get an error because wp_2_options doesn't exist. Should I know enough as a dev to avoid that myself without a kinder warning?
• Updated patch to remove a now unused $wpdb in update_blog_details()
• The action names wp_insert_site, wp_update_site, and wp_delete_site don't necessarily clarify on their own (to me) that the action is fired after the function has performed its main task. What about wp_site_inserted, wp_site_updated, and wp_site_deleted? Or something else?

[flixos90]

Replying to jeremyfelt:

Related: In wp_update_site(), what case would lead to site_id being empty? It should already have one from the old site object. Are we protecting against false or 0 being passed to wp_update_site()? If so, maybe it should fall back to the old site's value instead of the main/current network?

In 40364.4.diff​ the value falls back to the one previously set. This is indeed only useful when someone passes a falsy network ID value to wp_update_site().

My gut says that we can probably deprecate it, but I'm not completely sure.

Regarding deprecation, I'd like to add to this ticket what we discussed a few weeks ago: Let's only deprecate insert_blog() for now. The *_blog_details() functions can be considered at a later point, when this ticket (for update_blog_details(), #40180 and #40228 (for get_blog_details()) and #40201 (for refresh_blog_details()) have all been merged.

This isn't a new problem, and could be another ticket, but should we think about adding a check for the new site's options table in WP_Site's get_details()? If I do wp_insert_site( $args ) and then immediately look for get_site( 2 )->home, I'll get an error because wp_2_options doesn't exist. Should I know enough as a dev to avoid that myself without a kinder warning?

I think this all ends up being dev-education. As discussed, the functions part of this ticket are very low-level. The actual function to create a new site will be wp_create_site(), which will wrap wp_insert_site() and a future wp_install_site() and will be the de-facto replacement for wpmu_create_blog().

The action names wp_insert_site, wp_update_site, and wp_delete_site don't necessarily clarify on their own (to me) that the action is fired after the function has performed its main task. What about wp_site_inserted, wp_site_updated, and wp_site_deleted? Or something else?

I was following the examples in wp_insert_post() or wp_insert_comment() which use present tense, but I'm not opposed to another name. I don't like having only the past tense versions there - if they're the only filter, I'd rather use the function name. For other examples of how it's been done before, we might wanna look at wp_insert_term(): That one has to hooks, if we were going with that way, it would be insert_site and inserted_site - not sure how much sense that makes. I think I prefer the function name for the filter name, alongside with the docblock, which states that it is run after the process.

[jeremyfelt]

Replying to flixos90:

The action names wp_insert_site, wp_update_site, and wp_delete_site don't necessarily clarify on their own (to me) that the action is fired after the function has performed its main task. What about wp_site_inserted, wp_site_updated, and wp_site_deleted? Or something else?

I was following the examples in wp_insert_post() or wp_insert_comment() which use present tense, but I'm not opposed to another name. I don't like having only the past tense versions there - if they're the only filter, I'd rather use the function name.

Per today's Slack conversation - let's stick with simple actions that match the function names. wp_insert_site, wp_update_site, wp_delete_site (like @flixos90 already had). :)

[flixos90]

This is almost ready, but needs to wait for #41333 to be merged. Let's make it a 5.0 priority.

[flixos90]

40364.6.diff​ applies cleanly against the latest changes in trunk and makes the following adjustments:

• Use wp_update_site() internally in update_blog_status().
• Use the existing update_blog_public action hook in wp_maybe_transition_site_statuses_on_update().
• Update version numbers to use 5.0.0.

Something we need to carefully examine is how the hooks when changing one of the site status fields are fired. In trunk, the hooks fire only if the value has changed when calling update_blog_details(), but they fire always when calling update_blog_status(), which I think doesn't make sense. Furthermore the update_blog_public hook only fires in update_blog_status() at all.
I think we should make this more consistent, which the above patch is a first take at. The hooks are now fired as part of wp_maybe_transition_site_statuses_on_update(), and they are only fired if their value has changed. While this may theoretically be a breaking change for very unusual setups (I don't know who would wanna do something when a status field is updated, but not actually changed), I think we should be good here - but it's something to discuss further of course. Note that for example I needed to adjust a few tests for these, since those would expect the hooks to run in update_blog_status() even when the value wasn't changed.

Note that in the current patch there are two tests that fail to pass, however I haven't been able to figure out the reason yet.

[flixos90]

40364.7.diff​ is an update that applies cleanly against the latest coding standards commit.

It furthermore adds a filter function that ensures the caches for new domain or path are cleaned as well when one of those values is updated on a site. Two tests for that have been added.

[jeremyfelt]

I reviewed this and then chatted with @flixos90 at WCUS contributor day. A few notes from that discussion:

• If we provide compatibility for the network_id parameter, then we should use that in the documentation instead of site_id to encourage its use.
• We should be able to remove the $compat_keys loop as there should only really be one compat key.
• wp_update_blog_public_option_on_site_update() makes sense as a hooked function because it's data that is managed outside of the core site data in the DB.
• The patch changes some behavior of the site status hooks, which is an okay (small back-compat breaking) change. The previous behavior was inconsistent in two places. This results in the change to test assertions.
• We should use domain_exists() in wp_insert_site()
• We should use domain_exists() in wp_update_site() whenever one of domain/path/network_id changes.
• Add validations for domain and path. This gives us an opportunity to consolidate the existing workflows used in site-new.php, wp-signup.php, and wpmu_activate_signup(). These all validate a site before adding it in their slightly different, but mostly the same way. Related: #41443

We'll really want to dig into the existing new site flows to be sure that validation is happening as expected.

[flixos90]

40364.8.diff​ implements the points mentioned above. Most improvements are trivial, with the exception of validating/sanitizing site data:

• The domain and path sanitization bits from wpmu_create_blog() have been removed and are now part of wp_insert_site() which is called by it. They are also enforced on updating a site with this information. This keeps them consistent at a centralized point. The parameters for wpmu_new_blog are now taken from the new site object, to ensure the passed values are sanitized.
• Domain, path and network ID only need to be sanitized in wp_insert_site() / wp_update_site() when they're actually passed as arguments. Re-sanitizing the data when it comes from the original site object is not necessary.
• As discussed at WordCamp US as well, there is no an action validate_site_data. It is passed an empty WP_Error object to add validation errors to. Since this is passed by reference anyway, it is better off as an action than as a filter, as the latter would allow tweaking the filter value in unexpected ways. All validation is part of a function wp_validate_site_data() which is hooked in by default. Validation ensures the domain is not empty and that the domain-path-network combination doesn't already exist. By having this in a centralized logic, these checks now happen correctly both when inserting and updating a site.

I remember we considered going further with validation there, however I no longer think this is a good idea. The domain_exists() check certainly belongs here, because it ensures uniqueness (and that would normally be part of the database schema). But more finegrained validation should not be part of the database functions, but remain in pre-made checks like wpmu_validate_blog_signup(). This becomes particularly obvious since that function mostly checks the site name, which is not even part of the arguments (although it's technically somewhere in the domain or path).
Let's keep the responsibilities of the functions as they are now and not overcomplicate this. wp_insert_site() will eventually replace wpmu_create_blog() (with a future wp_install_site() hooked into the wp_insert_site action), so we are also on the safe side by not integrating too specific validation into here.

I will soon do another review of the unit tests and add further ones. I'll particularly pay attention to the hooked-in functions that were previously hard-coded somewhere. Once this has sufficient coverage, I think it can go in.

[mnelson4]

In today's discussion @spacedmonkey (correct me if I'm wrong!) was pointed out there will be multiple ways to create a site (via WP CLI, REST API, admin page, signup form, etc) so it would be nice if each of those functions didn't need to have repeated logic. So as clarification, each of those different controller-type codes should use wp_validate_site_data() and if the data is considered valid, use the soon-to-exist wp_install_site() (which, internally, will use wp_insert_site() etc) but for now they'll use

What would be a use-case for using wp_insert_site() directly?

[flixos90]

40364.9.diff​ has a few improvements over the previous patch:

• Introduce a wp_normalize_site_data filter to normalize the data passed to wp_insert_site() and wp_update_site(). This reduces complexity of the functions significantly and reduces duplicate code. A function wp_normalize_site_data() is automatically hooked in for default normalization (stripping invalid characters from domain, ensuring the path has a trailing slash, maintaining BC when someone uses site_id instead of network_id).
• Rename validate_site_data action to wp_validate_site_data.
• In wp_validate_site_data(), ensure that the path is not empty.
• In wpmu_create_blog(), pass the data in the $meta array that is relevant for the wp_blogs row to wp_insert_site() immediately. insert_blog() didn't support those, but our new function does, so we can reduce database transactions by quite a few here. Afterwards these values can be removed from the $meta array, since it should only contain regular options at this point.
• Ensure wp_maybe_transition_site_statuses_on_update() is hooked into wp_insert_site as well. In that case, the default flag values for a site are used to compare against. This ensures that the site status hooks fire also when inserting a site as necessary, which would previously happen because update_blog_status() was called for each flag. This ensures that the change in wpmu_create_blog() described above is fully backward-compatible.
• In wpmu_create_blog(), temporarily unhook the action to change the blog_public option. This needs to happen because at this point the database tables are not available yet. We will be able to remove this hack once #41333 is done, since that will hook in the site installation process before that hook.

With just a few more unit tests, this should be ready to go in.

[flixos90]

40364.10.diff​ does the following:

• Introduce additional unit tests to test the changes comprehensively. In particular:
  • Test normalizing site data works as expected.
  • Test validating site data works as expected.
  • Ensure that site status hooks are executed as expected, both on insert and update.
• In wp_validate_site_data(), ensure a network ID is passed. For wp_insert_site() and wp_update_site() this will never fail, as the network ID will be auto-populated if not given, but since the function stands on its own, it should be checked.
• Fix a bug in wp_validate_site_data() that prevents the domain_exists() check if one of the required values is missing (that will already cause a failure anyway).
• Reinstate the domain_exists() check into wpmu_create_blog(). This allows us to change the error code for the same issue in wp_validate_site_data() to a more appropriate site_taken (currently blog_taken), and also reduces the chance for unexpected issues by bailing as early as it currently happens in such cases.

I think this is ready to be committed. @jeremyfelt @spacedmonkey If you could give it a final review, that would be great!

Afterwards, I'll focus on #41333 and #42252, which should also make it into 5.0.

[spacedmonkey]

So I spent the evening on this patch and I the following feedback.

• Change this action to a filter do_action( 'wp_validate_site_data', $validity, $data, null ); to a filter. It the conversion in WordPress, if you are going change a value, developers use filters. They wouldn't think to use an action. I think developers will find it weird.
• If you pass an id into wp_insert_site, it should pass to wp_update_site to bring it into line with other crud functions. If it doesn't do this, at least handle this as error.
• The following lines need to be changed as they are filter not actions.

add_action( 'wp_normalize_site_data', 'wp_normalize_site_data', 10, 1 ); -> apply_filters( 'wp_normalize_site_data', 'wp_normalize_site_data', 10, 1 );
add_action( 'wp_validate_site_data', 'wp_validate_site_data', 10, 3 ); -> apply_filters( 'wp_validate_site_data', 'wp_validate_site_data', 10, 3 );

• Should the actions do_action( 'delete_blog', $blog_id, $drop ); and do_action( 'deleted_blog', $blog_id, $drop ); be moved to the wp_delete_site? I know that the drop tables param doesn't really map, but I am wondering if they will be breakage without these actions.

Otherwise, this patch is extremely solid. We are nearly there

[flixos90]

Replying to spacedmonkey:

Thanks a lot for the review!

Change this action to a filter do_action( 'wp_validate_site_data', $validity, $data, null ); to a filter. It the conversion in WordPress, if you are going change a value, developers use filters. They wouldn't think to use an action. I think developers will find it weird.

Technically, it doesn't make sense for this to be a filter since objects (i.e. $validity) are passed by reference in PHP. If though, the general opinion is that this should be a filter for how WordPress developers are used to do it, I'm not entirely opposed to changing it. I'd like to have a third opinion here.

• If you pass an id into wp_insert_site, it should pass to wp_update_site to bring it into line with other crud functions. If it doesn't do this, at least handle this as error.

That wp_insert_post() and wp_update_post() behave like this is very unexpected as it is not explicit at all. Because these here are new APIs for which we try to make things better, I vote for not doing this solely for the sake of compatibility with those two functions (especially since not all WordPress object CRUD functions behave like the latter either).

• The following lines need to be changed as they are filter not actions.

add_action( 'wp_normalize_site_data', 'wp_normalize_site_data', 10, 1 ); -> apply_filters( 'wp_normalize_site_data', 'wp_normalize_site_data', 10, 1 );
add_action( 'wp_validate_site_data', 'wp_validate_site_data', 10, 3 ); -> apply_filters( 'wp_validate_site_data', 'wp_validate_site_data', 10, 3 );

Good catch, I'll implement this soon!

• Should the actions do_action( 'delete_blog', $blog_id, $drop ); and do_action( 'deleted_blog', $blog_id, $drop ); be moved to the wp_delete_site? I know that the drop tables param doesn't really map, but I am wondering if they will be breakage without these actions.

It's helpful that you bring this up, I haven't considered it before. I think we should deal with this in #41333 as it goes more into the flow of uninstalling the site and in the current state of the patch works fully backward-compatible.

[spacedmonkey]

Technically, it doesn't make sense for this to be a filter since objects (i.e. $validity) are passed by reference in PHP. If though, the general opinion is that this should be a filter for how WordPress developers are used to do it, I'm not entirely opposed to changing it. I'd like to have a third opinion here.

I am telling you, that as a developer, I was confused seeing it was action and not a filter. Even if you can change a value in action, it being an action not a filter sends a message that maybe you shouldn't change it. I don't see why it should be an action, it is not an action, it a variable that is designed to be change, that is a filter in my mind.

That wp_insert_post() and wp_update_post() behave like this is very unexpected as it is not explicit at all. Because these here are new APIs for which we try to make things better, I vote for not doing this solely for the sake of compatibility with those two functions (especially since not all WordPress object CRUD functions behave like the latter either).

So both wp_insert_post() and wp_insert_user() do this. Honestly it is a pattern that I don't love. As I said before, it set the blog_id on wp_insert_site, it should error out otherwise, it might have unexpected consequences.

Looking again at the patch wp_validate_site_data should have some validation for registered and last_updated to make sure they are in the right date format. The following code can be found in wp_insert_post and something similar can be used.

        // Validate the date.
        $mm         = substr( $post_date, 5, 2 );
        $jj         = substr( $post_date, 8, 2 );
        $aa         = substr( $post_date, 0, 4 );
        $valid_date = wp_checkdate( $mm, $jj, $aa, $post_date );
        if ( ! $valid_date ) {
                if ( $wp_error ) {
                        return new WP_Error( 'invalid_date', __( 'Invalid date.' ) );
                } else {
                        return 0;
                }
        }

[flixos90]

40364.11.diff​ implements recently suggested changes, plus a bit more:

• wp_normalize_site_data() is now correctly added as a hook via add_filter().
• wp_normalize_site_data() now casts site status fields to integer values.
• wp_normalize_site_data() now strips empty-ish date fields (registered and last_updated), as a date should always be present. Stripping the values out at this early point will ensure the default values (current time / previous time values) are used.
• wp_validate_site_data() now ensures that registered and last_updated are always provided and valid (via wp_checkdate()).
• current_time( 'mysql', true ) is now used for generating MySQL date values, since we should always use GMT here, as otherwise the value would depend "randomly" on the current site (usually the main site) and its timezone settings, which is unexpected. A general format (i.e. GMT) should be used instead. Up to this point, only last_updated used GMT while registered used a localized time which doesn't make any sense. They need to have the same format.
• New tests have been added for all of the above.

[spacedmonkey]

Looking good!

I think we are nearly there.
I noticed in wp_update_site that there are the following lines

$whitelist = array( 'domain', 'path', 'network_id', 'registered', 'last_updated', 'public', 'archived', 'mature', 'spam', 'deleted', 'lang_id' );
$data = array_intersect_key( wp_parse_args( $data, $defaults ), array_flip( $whitelist ) );

I believe the same whitelisting should be applied in wp_insert_site. This would fix my issue with an id being passed to the wp_insert_site function.

As an idea, we could do this. Change the filter to this

$data = apply_filters( 'wp_normalize_site_data', $data, $defaults );

and then move this

$whitelist = array( 'domain', 'path', 'network_id', 'registered', 'last_updated', 'public', 'archived', 'mature', 'spam', 'deleted', 'lang_id' );
$data = array_intersect_key( wp_parse_args( $data, $defaults ), array_flip( $whitelist ) );

into wp_normalize_site_data to stop repeated code.

[spacedmonkey]

In 40364.12.diff I moved a lot repeated logic to wp_normalize_site_data filter.

[jeremyfelt]

Some notes after going through 40364.11.diff​. (I tried 40364.12.diff, but the patch broke unit tests and I was on a plane, so I went back one rev)

• A domain is required, but only by default through the wp_validate_site_data() function. If somebody unhooked this default validation, then it would be possible to enter domain/path as empty strings. This is also possible in the existing insert_blog(), so it’s probably not a thing to fix, but a thing to remember. Because of this, we may want to remove the note in the docs that domain is required or should be provided and leave that up to the validation documentation.
• Why $validity instead of $errors? The latter seems to be the pattern we’ve followed in the past except for the Customizer.
• I believe it’s possible to insert a site with network ID 0 now. I’m not sure if that’s something that somebody is or should be relying on. In the current patch, anything passed as 0 will be overwritten with the current network ID, even if somebody removed the default site validation and added their own.
• Should we provide protection for anyone inserting a site with a network ID that does not exist? I lean toward no, and we may have talked about it before. This is probably something to document in the dev note.
• Should it be possible to delete the main site with wp_delete_site() by default? I know this is purely transactional, but this can have some serious consequences and may be worth some sort of validation that can be disabled.
• It’s a little strange that I can delete sites that still have other database tables. It’s also a little strange that I can delete all sites and leave myself in a weird place. That does seem like developer education, but could there be room for a check that says you can’t delete the last site?
• If a site already exists in the database with a zero date—possibly entered through another source (?), I’m not able to update through wp_update_site() without also including registered and last_updated parameters. I think we need to account for that.
• Should there be an error when you attempt to update the blog_id of a site through args passed to wp_update_site()? Right now everything updates except for the blog_id, which may (?) be somewhat confusing.
• If I use wp_delete_site( 0 ), the current site is deleted, which will probably lead to trouble somewhere someday. We should have an empty check on the ID so that the current site is not grabbed via get_site() in that scenario. The same thing should probably be done on wp_update_site() as well to avoid accidental site updating. I don’t think it’s safe to fallback in any empty scenario—a site ID should always be provided.

Thanks for all of the work on this!

[flixos90]

40364.13.diff​ takes the feedback from before into account.

@spacedmonkey

• The defaults parsing and the whitelist intersection are duplicate code in wp_insert_site() and wp_update_site(), but they need to remain in those functions as those two are the only absolutely critical parts that should not be unhooked. wp_normalize_site_data is a filter, so these crucial parts must happen outside.
• The same applies to renaming network_id back to site_id prior to the DB transaction. If that happened in wp_normalize_site_data(), it would affect wp_validate_site_data(), and it would be unexpected having to deal with network_id in one function and with site_id in the other.
• However, I agree the duplicate code there is ugly and unnecessary. Therefore I introduced a wp_prepare_site_data() function that contains all of it. That function now executes the wp_normalize_site_data filter, parses against defaults and whitelist, and executes the wp_validate_site_data action. This way we don't have duplicate code, while it is still obligatory.

@jeremyfelt

• I changed the $domain argument description to Site domain. Default empty string..
• $validity is now called $errors.
• I agree it makes sense to make the network ID assurance optional, as technically 0 is an allowed value. I adjusted the respective code (now part of wp_prepare_site_data()): The current network ID (or previous network ID in case of an update) is now just a default against which the data is parsed. There is still a clause in the validation function that prevents a network ID of 0, but that again can theoretically be unhooked.
• I don't think we need to validate a network exists for the given ID. Let's keep this for the dev note.
• I didn't introduce any checks for deleting the main site. wpmu_delete_blog() currently does not have a check like that, I think the functions here are too low-level in order to account for such things. _If_ we need truly need this, it can also be an enhancement in the future.
• Regarding the database tables for a site, those will be covered by #41333. That is why I'd like to have this ticket here merged asap so that we can start with the other ticket. wp_install_site() and wp_uninstall_site() will be hooked into wp_insert_site and wp_delete_site respectively, so that then the DB tables are handled correctly. Regarding deletion of sites in general, that is a matter of developer education.
• I'm not sure we need to deal with someone passing $blog_id in the array of arguments. It is not a valid argument for inserting or updating a site. For updates, the $site_id parameter is there for that.
• I added a check that ensures wp_update_site( 0 ) and wp_delete_site( 0 ) return an error. We shouldn't fall back to the current site in either case here.

[flixos90]

40364.14.diff​ implements the final change that was agreed on in yesterday's office hours meeting: When 0000-00-00 00:00:00 is stored as one of the site date values for whatever reason (the new functions catch that, but for whatever reason old code might not have), we wanna ensure this validation doesn't fail. Therefore wp_validate_site_data() now allows that "empty" date to be passed. Note that it is stripped out before though, if it is actually passed to wp_insert_site() or wp_update_site(), so this is only an extra BC measure to account for old data, as throwing an error then would be unexpected. Two test datasets to verify this behavior have been added.

In yesterday's discussion we determined this is ready. I'll commit shortly.

[flixos90]

In 43548:

Multisite: Complete the new CRUD API for managing sites.

New functions wp_insert_site( $data ), wp_update_site( $id, $data ) and wp_delete_site( $id ) are introduced to manage site rows in the wp_blogs table, forming the new CRUD API together with the existing get_site() / get_sites(). The new API provides various benefits over the previously existing API, fixing several cache invalidation issues and being hook-driven so that normalization and validation of the passed data can be fully customized.

New hooks introduced as part of this are the actions wp_insert_site, wp_update_site, wp_delete_site, wp_validate_site_data and the filter wp_normalize_site_data.

At this point, wp_insert_site() does not handle setting up the site's database tables, and wp_delete_site() does not handle dropping the site's database tables, so the two can not yet be used directly as full replacements of wpmu_create_blog() and wpmu_delete_blog(). Managing the site's database tables will be added via hooks as part of the follow-up ticket #41333.

The existing functions wpmu_create_blog(), update_blog_details(), and wpmu_delete_blog() make use of the respective new counterpart and will be obsolete once #41333 has been completed.

Props flixos90, jeremyfelt, spacedmonkey.
Fixes #40364.

[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]

Version numbers need to be adjusted to 5.1.0.

[flixos90]

In 44469:

Multisite: Update @since tags for site management APIs.

Fixes #40364. Fixes #41333.

[pento]

#41064 was marked as a duplicate.

[pento]

#40035 was marked as a duplicate.