introduce get_current_network_id helper function

[spacedmonkey]

Similar to get_current_blog_id helper function. This function can be used to get the id of the current_site global.

[johnjamesjacoby]

The $current_blog variable is only created in multisite in ms-settings.php (and maybe sunrise.php).

• Would this benefit from an !empty() check?
• Should this function live outside of multisite, maybe in load.php next to get_current_blog_id()?

[spacedmonkey]

Did the feedback from @johnjamesjacoby . Did one better than check if empty and did a type check.

[jeremyfelt]

I've looked for this a couple times recently. We should go for it. :)

[jeremyfelt]

In single site, get_current_blog_id() will return 1 because the global $blog_id is setup in wp_initial_constants(). We don't do anything similar for a global network ID, though in get_main_network_id() we return 1 if not multisite.

So I guess my question is, does this logic make sense:

• If ! is_multisite(), return 1.
• If is_multisite() and not a WP_Network instance, return 1.
• If is_multisite() and a WP_Network instance, return ID.

What would lead to the second condition?

[johnjamesjacoby]

My thoughts:

• Yes
• Yes
• Yes

The second condition could maybe happen if sunrise.php was already in place and wasn't using core functions to populate $current_site, and honestly this seems likely for anyone that's using sunrise.php since the only thing it is really great at is pre-populating those globals.

---

Tangentially, the assumed 1 main network ID return value bothers me. A new default constant makes sense here, itself defaulted to 1 in WordPress core. And it can still be overridden in sunrise.php or wp-config.php. It's edge-case, but I've already needed to kludge this because a new network with a few sites took the place of the original one.

So for each of the 3 cases, I'd like for the 1 default return value to be configurable in some way.

[jeremyfelt]

Replying to johnjamesjacoby:

The second condition could maybe happen if sunrise.php was already in place and wasn't using core functions to populate $current_site, and honestly this seems likely for anyone that's using sunrise.php since the only thing it is really great at is pre-populating those globals.

This should be mitigated by the check and re-assignment at the end of ms-settings.php. So it seems like any real use of get_current_network_id() by an mu-plugin, plugin, or theme would be after those objects are properly available.

Tangentially, the assumed 1 main network ID return value bothers me. A new default constant makes sense here, itself defaulted to 1 in WordPress core. And it can still be overridden in sunrise.php or wp-config.php. It's edge-case, but I've already needed to kludge this because a new network with a few sites took the place of the original one.

So for each of the 3 cases, I'd like for the 1 default return value to be configurable in some way. That could be handled in it's own ticket, too, since it's a separate initiative and could cascade into the default $blog_id too?

Should the default used in get_current_network_id() be based on get_main_network_id()? We use the PRIMARY_NETWORK_ID (oops) constant and get_main_network_id filters there that could be used to override. That seems to be a safe assumption— main == default.

[flixos90]

33900.3.patch​ is an updated patch with the following changes:

• return 1 if no multisite
• use get_main_network_id() if $current_site is not a WP_Network instance
• add a similar instance check in get_main_network_id() (otherwise this change would have no point and could cause an error)
• make sure to return an integer (the id property of WP_Network is a string)

I wonder whether the instance check is really necessary (edge-case). We can probably do it since it shouldn't cause issues, but we don't do a check like this in other areas of Core (see the change in get_main_network_id() in this patch for example). I wonder whether this is worth changing in this function - I'm not opposed to it, it's just different from how we handle $current_site in the rest of Core (always assume it's a WP_Network).

Btw sorry about the weird spacing changes in wp-includes/load.php, not sure what my editor did there.

[jeremyfelt]

Replying to flixos90:

I wonder whether the instance check is really necessary (edge-case)

We may be better off with an isset() instead. If the function is used in sunrise.php for some reason, it's possible that things are not full objects yet but still have the id property. We then convert to WP_Network object in ms-settings.php. 33900.diff​ takes this route.

Thoughts?

[flixos90]

Replying to jeremyfelt:

We may be better off with an isset() instead. If the function is used in sunrise.php for some reason, it's possible that things are not full objects yet but still have the id property. We then convert to WP_Network object in ms-settings.php. 33900.diff​ takes this route.

Thoughts?

Makes sense to me, this will make it more accurate in those use-cases.

[jeremyfelt]

In 37670:

Multisite: Introduce get_current_network_id()

Similar to get_current_blog_id, this can be used to get the ID of the $current_site global. If not available, it will fallback to the main network ID. In single site, this will return 1.

Props spacedmonkey, flixos90.
Fixes #33900.