get_main_site_id ignores custom values of $current_site->blog_id

[spacedmonkey]

The function get_main_site_id was added in #29684 . This new function adds a way to get the main site id of the current network. This works on site id of the site with the same domain / path of the network. However, before the point of truth main site id was property on $current_site. Some domain mapping plugins build the $current_site and $current_blog manually. Build these globally manually means that the $current_site->blog_id will be set. Currently get_main_site_id ignores these values.

[spacedmonkey]

41936-test.diff
This is a test if blog_id is set from another source, such as a sunrise file. Test is currently failing.

41936.diff
This is my solution, the moves the logic of getting blog_id into the WP_Network. It also makes the get_main_site_id method public, so it can be called in the bootstrap process.

[flixos90]

I agree this can become a bug in some cases, thus should be fixed in 4.9.

41936.diff​ fixes it, but I still don't like the logic being part of the WP_Network method, as this is not a common practice. It may very well be the only solution to accomplish this, but I wanna try to think further about another way. This can go in post-beta, so we don't need to rush.

[spacedmonkey]

41936.4.diff

Changes

• Magic method returns int
• blog_id is now cast to string
• get_main_site_id now returns get_current_blog_id
• Don't return early, on pre_get_main_site_id filter, check if not empty, to make sure it value is not a null value.

[flixos90]

41936.5.diff​ is a minor update:

• Ensure that the $blog_id property is always a string and that the get_main_site_id() method always returns an int.
• Handle the filter separately, i.e. have it not set the $blog_id property.
• Use an internal variable for the cache key instead of "hard-coding" it twice.
• Keep the return 1 if not multisite in the get_main_site_id() function for now, as this should be a separate commit.
• Add two extra unit tests, one to ensure the property value is honored correctly, and another to ensure the filter still takes precedence.

[flixos90]

41936.6.diff​ adjusts the following:

• Fixes access of not existing $network variable by using $this instead.
• Only returns value of pre-filter or property if it is positive.
• Adjusts documentation of the pre-filter accordingly.

[jeremyfelt]

41936.6.diff​ looks good! I made one small change to the tests in 41936.7.diff​ and removed the pre filter each time it was used to avoid possible cross-test pollution.

[flixos90]

In 41861:

Multisite: Take WP_Network::$blog_id into account in get_main_site_id().

When the WP_Network::$blog_id property is set manually, for example in the multisite bootstrap process, get_main_site_id() should use that value instead of running its own logic. The main logic for the function was therefore moved into the internal WP_Network::get_main_site_id() method, which is now being accessed by the function through the magic property handling for WP_Network::$blog_id (and its equivalent WP_Network::$site_id).

Props spacedmonkey, jeremyfelt.
Fixes #41936.

[flixos90]

In 41862:

Multisite: Return get_current_blog_id() value instead of hard-coded 1 in get_main_site_id() for non-multisite environments.

See #41936.