WP_Site_Query class

[spacedmonkey]

With WP_Site is being merged in #32450 and we need a way to get query the blogs table in a consistent way of querying for sites.

This class could be used to logic instead the following functions

get_id_from_blogname
get_blogaddress_by_id
get_blog_details
get_blog_status
get_last_updated
get_blog_id_from_url
domain_exists
wp_get_sites
get_site_by_path ( Not sure about this one )
ms_allowed_http_request_hosts

This class could also be used to create a sites endpoint in the rest api.

[spacedmonkey]

If you think of wp_get_sites is a base, the following params are accepted.

'network_id' => $wpdb->siteid,
'public' => null,
'archived' => null,
'mature' => null,
'spam' => null,
'deleted' => null,
'limit' => 100,
'offset' => 0,

To bring it into line with other functions like get_users, we should add the followings params
domain - string or array
path - string or array
includes - array of blog ids
excludes - array of blog ids
order - string - field name
orderby - string ASC|DESC
fields - all|ids|domains

[spacedmonkey]

First pass

[spacedmonkey]

Did a first pass of the wp_site_query. I started by copying and pasting the wp_comment_query the class and changing it around to fit purpose. There is a LOT in the patch so there might be to much to explain here.

I have an example of an implementation in removing the guys of wp_get_sites. I have also added _prime_site_caches and update_site_cache functions.

Look forward to hearing feedback.

[jeremyfelt]

This is an excellent first pass @spacedmonkey, thank you. I'm looking forward to diving in soon, this is definitely 4.6 material.

[spacedmonkey]

So I have gone through the code and implemented the class where I think it would be useful.
At some point this patch should be broken up into two. One for wp_site_query and another for the implementation of it in core functions.

Points of interest

• orderby CHAR_LENGTH on domain and path. Need a solution for this
• Bootstrap loader, am I loading it to late / soon.
• Should update_site_cache do more, warm other caches? Should it have a hook?
• Comments need a lot of work.
• This query object should be used to create a sites endpoint in the api

Not sure that this is secure / best way of doing this

implode( "', '", $wpdb->_escape( $this->query_vars['path__not_in'] ) ) . "' )

[jeremyfelt]

Related once we get to implementation: #21837, #24833, #33185

[flixos90]

minor adjustments

[flixos90]

@spacedmonkey
Patch looks great so far, I played around with it quite a bit!

In 35791.3.patch​ I applied two minor adjustments:

• I applied a fix in wp_get_sites() to make the code backwards-compatible (the function must return an array of associative arrays)
• I removed the count argument and replaced it by a possible value count in the fields argument, similar to the functionality of get_terms() for example - returning a count is basically also an alternative fields specification although technically not a field

[flixos90]

Directly related to this task are #31148 and #35697. We can probably close #31148 since the current patch already includes the enhancements requested there.

For #35697 we would need to think of which combinations to return are worth supporting - actually implementing this would be pretty straightforward.

[jeremyfelt]

Related: #34450 for get_id_from_blogname().

[jeremyfelt]

Related: #31377 for adding a blogname argument.

[spacedmonkey]

Updated my patch with better starting params. Also public params wasn't working :( and I fixed it.

I had a look into support getting site by blogname but couldn't think of a way of I could do it in a scalable way. If you get requesting a list of 1000 sites, I don't want to do 1000 switch to site / joins to get the blogname. It the blogname was cached in another table, such as wp_blogs this would be different.

@flixos90 good idea about making the wp_get_sites backwards-compatible. I think it should be an option to select what format the function returns. As for count, I don't agree with your change. This class is based on wp_comment_query, which does have count. I think this shouldn't change.

[flixos90]

Replying to spacedmonkey:

I think it should be an option to select what format the function returns.

Sounds good, we just need to make sure that ARRAY_A is the default.

As for count, I don't agree with your change. This class is based on wp_comment_query, which does have count. I think this shouldn't change.

Yes, I just saw it today that WP_Comment_Query has a count parameter. I was referring to the get_terms() function when proposing that change.

I personally prefer to have it as a possible value for fields since one parameter to modify the type of output should be sufficient. For example, using both the fields and count parameters in one query wouldn't make sense.

[spacedmonkey]

Something like this is what I had in mind for backwards compatibility

function wp_get_sites( $args = array(), $output = ARRAY_A ) { 
    global $wpdb; 
         
                 
    $defaults = array( 
                    'network_id' => $wpdb->siteid, 
                    'limit'      => 100, 
                    'offset'     => 0, 
                    'count'              => false, 
                        );
        $args = wp_parse_args( $args, $defaults ); 
        $query = new WP_Site_Query(); 
        $_sites = $query->query( $args ); 

        if( !$args['count'] && $output != OBJECT ){
                $sites = array();
                
                foreach ( $_sites as $site ) { 
            if ( $_site = get_site( $site, $output ) ) { 
                $sites[] = $_site; 
            } 
        } 

                return $sites;
        }
        return $_sites;
} 

[jeremyfelt]

Walking through this a bit tonight to get familiar. First of all, great stuff! Thank you for all the work and discussion so for. :100:

Replying to spacedmonkey:

At some point this patch should be broken up into two. One for wp_site_query and another for the implementation of it in core functions.

Yes, please. :) It would be beneficial to have a patch that introduces WP_Site_Query and then different patches for how it is applied throughout core. Right now, unit tests go a bit haywire and it would be easier to determine the cause if I could add/remove portions. When it comes time to commit, this will also be the approach.

• orderby CHAR_LENGTH on domain and path. Need a solution for this
• Bootstrap loader, am I loading it to late / soon.

The queries in ms-load.php / get_site_by_path() are a special beast. It's probably best to leave them be completely, or save that for a later time. This function should really only be useful during bootstrap anyway. Removing these changes helps most of the unit test errors.

• Should update_site_cache do more, warm other caches? Should it have a hook?

Hmm. There are 8 different keys that we clear in clean_blog_cache(). It looks like we could prime the short blog-details cache without requiring any more data. The rest is probably fine. This is probably something we can probably add to WP_Site::get_instance() at some point too. I think I remember @johnjamesjacoby mentioning something along those lines at some point.

• Comments need a lot of work.

The are a lot of docs. :) Pinging @DrewAPicture for a docs review at some point, though additional work from others is very welcome. :)

• This query object should be used to create a sites endpoint in the api

Absolutely. It's probably beneficial to start a PR here - ​https://github.com/WP-API/wp-api-site-endpoints - once we get things locked in.

Not sure that this is secure / best way of doing this

implode( "', '", $wpdb->_escape( $this->query_vars['path__not_in'] ) ) . "' )

At first glance, I think we're okay. I'll dig into that more soon.

Replying to flixos90:

Replying to spacedmonkey:

I think it should be an option to select what format the function returns.

Sounds good, we just need to make sure that ARRAY_A is the default.

+1 ARRAY_A should be default. We should approach adding different return formats for wp_get_sites() in a different ticket.

As for count, I don't agree with your change. This class is based on wp_comment_query, which does have count. I think this shouldn't change.

I personally prefer to have it as a possible value for fields since one parameter to modify the type of output should be sufficient. For example, using both the fields and count parameters in one query wouldn't make sense.

I lean toward the count parameter on this rather than part of fields, though it's worth thinking about.

Other notes:

• Code style: Let the code around different changes be in most cases. It's sometimes painful to see broken code standards, but with such a large change like this, it is helpful for the patch/commit history to be as clean as possible. An example here is wp-includes/http.php, where proper brackets have been added to code that is not changing.
• Unit tests: As mentioned above, some of the unit tests are failing now. We need to make sure those stay passing and we should introduce a set of tests specifically for WP_Site_Query.

[spacedmonkey]

This one was a smaller patch. Changes include.

• Fixed wp_get_sites so it now returns ARRAY_A.
• Also added new orderby params of domain_length and path_length. I am not a massive fan of the naming, so willing to change.
• Fixed formatting in http.php

When I get some more time I will generate two patches. I think that unit tests and comments are the last things we should look at once the api has been locked in and the class is unlikely to change. It would be extremely helpful if someone could make a start of the tests for this.

Last thing, was thinking we should add a new function called get_sites. This will work the same as get_posts and get_comments, as a wrapper to the query objects. We should deprecate wp_get_sites, as the naming of the function does not fit in with WP standards and the return value is ARRAY_A.

[DrewAPicture]

@jeremyfelt Happy to do an audit once the patches are separated.

[spacedmonkey]

I have split the patches up. 35791c.1.patch is the wp_site_query class (c for class), 35791i.1.patch is the implementation.

I have introduced a function called get_sites. This is bring it to like with other query objects, as comments, posts and users all have get_*s functions.

Going forward, I think we should depreciate wp_get_sites, as get_sites should replace it.

@DrewAPicture did my best with comments, but love your help.

[spacedmonkey]

Please use 35791c.3.patch to test the class. There was problems with the original patches. Sorry folks

[websupporter]

Hi,
are there plans to add a meta_query or would this be a later enhancement?

Furthermore, I've read in the doc

 * @type string|array $orderby Site status or array of statuses. To use 'meta_value'
         *     or 'meta_value_num', `$meta_key` must also be defined.
         *     To sort by a specific `$meta_query` clause, use that
         *     clause's array key. Accepts 'site_agent',
         *    'site_approved', 'site_author',
         *    'site_author_email', 'site_author_IP',
         *    'site_author_url', 'site_content', 'site_date',
         *    'site_date_gmt', 'blog_id', 'site_karma',
         *     'site_parent', 'site_id', 'site_type',
         *     'user_id', 'site__in', 'meta_value', 'meta_value_num',
         *     the value of $meta_key, and the array keys of
         *     `$meta_query`. Also accepts false, an empty array, or
         *     'none' to disable `ORDER BY` clause.
         *     Default: 'site_date_gmt'.

Line 155 (sorry, markup is messing up a bit). Is this still to come (Didn't find anything here in the discussion) or would this be a future enhancement?

[flixos90]

Replying to websupporter:

Hi,
are there plans to add a meta_query or would this be a later enhancement?

I thought about the same thing a few days ago. However adding this would be problematic since there is no global database table like blogmeta which we could use with the WP_Meta_Query class. As far as I can see, the "meta values" of a site are options and are stored in a separate DB options table for each site. It's not optimal for this approach, but there might be nothing we can do here at this point.

Supporting meta queries will be no problem for a future WP_Network_Query though since network-related meta values (network options) are located in a global sitemeta table. :)

[websupporter]

Ah, I see. This would be quite a JOIN :D

[spacedmonkey]

References to meta should be removed. It was in there by mistake. I have no plans to support joining to the options tables of all the sites for performance reasons.

[DrewAPicture]

@spacedmonkey I'm in the process of reviewing your patches. It be helpful not to iterate until I add the updated patch(es). You can expect those before today's dev chat.

[DrewAPicture]

Docs fixes

[DrewAPicture]

@spacedmonkey 35791-class.patch​ fixes a few docs syntax issues and fixes alignment on the hash notation in __construct().

Note: If you name your patches the same every time, Trac will increment the number for you. Uploading 35791-class.patch again will increment it to 35791-class.2.patch, and so on ;-)

Here are some notes not covered in 35791-class.patch​:

• If the $ID argument (__construct()) is currently unused, perhaps we shouldn't include it in arguments for the new class?
• What "back-compat" are we supporting exactly in adding a __call() magic method? It was probably added based on similar classes, but we should decide now if calling protected and private methods should be supported in this new class ;)
• Missing hook docs for parse_site_query action
• Odd to have a get_sites() method that relies entirely on query vars
• We should probably include 'join' in the $sql_clauses array even though it's not really in use by core

Next up: 35791i.1.patch​

[DrewAPicture]

@spacedmonkey 35791-implementation.patch​:

• Fixes alignment in the hash notation for get_sites()
• Adds changelog entries to the DocBlocks for ms_allowed_http_request_hosts(), wp_update_network_site_counts(), wp_get_sites(), and get_site_by_path() noting that they've been converted to use get_sites()
• Other minor spacing fixes

The same note applies as with the file naming for 35791-class.patch, by the way. If you iterate 35791-implementation.patch and name it the same the next time you upload, Trac will increment the filename for you: 35791-implementation.2.patch :-)

[DrewAPicture]

Docs fixes

[ocean90]

Replying to spacedmonkey:

Going forward, I think we should deprecate wp_get_sites, as get_sites should replace it.

I'm not sure if that's a good idea. get_sites() sounds to generic and could cause fatal errors if there are plugins/themes which define the same function (Yes, without a prefix).

[flixos90]

I just noticed that the class currently uses site for the cache group while WP_Site uses sites. We should probably align WP_Site_Query to use sites as well.

[flixos90]

some fixes and improvements for WP_Site_Query

[flixos90]

35791-class.2.patch​ fixes some bugs/inconsistencies and includes minor improvements.

• use sites as cache group, similar to WP_Site
• pass site IDs to _prime_site_caches() instead of network IDs
• $orderby support for site__in and network__in
• removed $found_orderby_blog_id variable (unused)
• removed $filtered_where_clause property (unused)
• simplified search clause SQL process (AND is no longer prepended, so we don't need to remove it afterwards)
• sort ascending by default (since default sort is by site ID)
• use class property $date_query to store WP_Date_Query object
• fixed docs for $orderby argument and $order argument
• added docs for parse_site_query action

[spacedmonkey]

35791-class.3.patch fixes some bugs/inconsistencies and includes minor improvements to make the unit tests pass.

$last_changed = microtime();
wp_cache_set( 'last_changed', $last_changed, 'sites' );

ms-blogs.php line 458. This was so the count were updated when sites are added / removed.

            /**
     * Converts an object to array.
         *
         * @since 4.6.0
         * @access public
         *
         * @return array Object as array.
         */
        public function to_array() {
                return get_object_vars( $this );
        }

class-wp-site.php line 200. Missing method required by get_site()

[spacedmonkey]

35791-implementation.3.patch fixes some bugs/inconsistencies and includes minor improvements to make the unit tests pass.

$args = array(
                'path__in'   => $paths,
                'domain__in' => $domains,
                'number'     => 1
        );

ms-load.php. Line 237. Fix, change path, to paths.

        if( is_array( $args['network_id'] ) ){
                $args['network__in'] = $args['network_id'];
                $args['network_id'] = null;
        }

        if( is_numeric( $args['limit'] ) ){
                $args['number'] = $args['limit'];
                $args['limit'] = null;
        }

ms-functions.php Line 2471. Backwards comp in wp_get_sites.

$result = get_sites( array(
                'path'    => $path,
                'domain'  => $domain,
                'network_id' => $site_id,
                'number'  => 1,
                'fields'  => 'ids'
        ) );

ms-functions.php Line 1264. Changed network to network_id.

[spacedmonkey]

Replying to ocean90:

Replying to spacedmonkey:

Going forward, I think we should deprecate wp_get_sites, as get_sites should replace it.

I'm not sure if that's a good idea. get_sites() sounds to generic and could cause fatal errors if there are plugins/themes which define the same function (Yes, without a prefix).

I am not sure that I agree. All of object types have a get_*s function. I know that they might be issues with adding new functions to core, but that is the risk that you take everytime a new function is added right? I don't see why we should have the design pattern for this one function. Wouldn't be do that everytime we add a new function into core going forward?

Regarding unit testing. The unit tests now pass for multisite. As in implementation we use query class, we already have some test coverage. wp_get_sites and domain_exists seem a good start for new tests, if they are required.

[jeremyfelt]

After some discussion in core chat today and a full search against the plugin repository, we're good to use get_sites() for WP_Site_Query. This is nice because it will let us return an array of objects and then work to deprecate wp_get_sites() as a wrapper for get_sites() in the future.

There is one plugin that declares its own get_sites(), but we can reach out and work with that author to resolve ahead of time.

[flixos90]

added unit tests + docs fixes

[flixos90]

In 35791-class.4.patch​ I added some unit tests for the class. All of these pass successfully. There might be some areas which we could use additional tests for, but this should be a good starting point.

In the above patch, I also fixed some invalid or missing docs regarding the $query parameter.

[jeremyfelt]

In 35791-class.5.patch​:

• Updated the naming of the individual tests to test_wp_site_query_by_{argument}_with_{descriptive}.
• Added a series of tests to continue building out coverage for more query args. Date queries should be the only thing left, though it wouldn't hurt to think of more scenarios to test.
• I've removed the __call() method as we don't have the same back-compat requirements for the protected get_search_sql method.
• This may be a future improvement, but it would be cool if we could pass wordpress.org/foo/ to search and have it split the domain and path into 2 search parts.

[flixos90]

Replying to jeremyfelt:

This may be a future improvement, but it would be cool if we could pass wordpress.org/foo/ to search and have it split the domain and path into 2 search parts.

I added this functionality in 35791-class.6.patch​ since I agree that it would be very useful and it wasn't too hard to implement.

This is a special case of search which is only run if the search string contains a / and the first / is not leading (in that case it would definitely be only a path). See the get_search_sql() method for the implementation. I also added a unit test for this functionality.

[jeremyfelt]

In 37468:

Multisite: Introduce get_site()

Given a site ID or site object, get_site() retrieves site data in the same vein as get_post() or get_comment(). This will allow for clean retrieval of sites from a primed cache when WP_Site_Query is implemented.

Adds a WP_Site::to_array() method to support multiple return types within get_site().

Props spacedmonkey.
See #35791.

[jeremyfelt]

35791-class.7.patch​ is a refreshed class patch after the introduction of get_site() in [37468]. Going to shoot for getting the rest of this in tomorrow morning and then start towards implementation.

I added this functionality in 35791-class.6.patch​ since I agree that it would be very useful and it wasn't too hard to implement.

I've removed this for now, but we should continue to explore it once we get WP_Site_Query in. We'll need some tests that cover the possibilities of single slashes, multiple slashes, leading/trailing slashes, etc...

[flixos90]

Replying to jeremyfelt:

I added this functionality in 35791-class.6.patch​ since I agree that it would be very useful and it wasn't too hard to implement.

I've removed this for now, but we should continue to explore it once we get WP_Site_Query in. We'll need some tests that cover the possibilities of single slashes, multiple slashes, leading/trailing slashes, etc...

Agreed. In 35791-class.8.patch​ I removed the related unit tests that would have failed now. Let's deal with this functionality in a future ticket after WP_Site_Query has been merged.

[jeremyfelt]

35791-class.9.patch​ makes some formatting adjustments:

• Remove some unnecessary alignment of code.
• Add periods at the end of a few doc summaries.
• Use single quotes vs double quotes when a variable is not involved.

[jeremyfelt]

The test coverage on WP_Site_Query is good. We also need to write tests to cover date_query, fields, no_found_rows, and update_site_cache before this ticket is closed. Fields is somewhat covered because we request ids everywhere in the test already, but it would be nice to see something more specific.

[jeremyfelt]

In 37477:

Multisite: Introduce WP_Site_Query

Provides a consistent way to query $wpdb->blogs for WP_Site objects based on domain, path, site ID, network ID, and more.

Introduces and uses update_site_cache() and _prime_site_caches() to maintain a cached list of WP_Site objects for use in multiple queries.

Props spacedmonkey, flixos90, DrewAPicture, jeremyfelt, ocean90.
See #35791.

[spacedmonkey]

@jeremyfelt  is there any chance that any of the implementation go in? At very least get_sites and wp_get_sites should go in 4.6..

[jeremyfelt]

In 37616:

Multisite: Introduce get_sites()

get_sites() is a wrapper for WP_Site_Query.

Props spacedmonkey, DrewAPicture.
See #35791.

[jeremyfelt]

In 37617:

Multisite: Replace wp_get_sites() internals with get_sites()

get_sites() should be considered a replacement for wp_get_sites(). Backward compatibility is maintained in the meantime by using get_site() to populate the return data with associative arrays rather than WP_Site objects.

Props spacedmonkey, flixos90.
See #35791.

[jeremyfelt]

See #36994 for the deprecation of wp_get_sites().

[dd32]

Just wanted to quickly weigh in on the usage of ARRAY_A here.. is it really needed? and when would you ever want to do ARRAY_N?

It looks like it's being used for BC, but, it feels like that if the calling function is needing BC-data, it should be the one to massage the new API's return format into something it understands. (ie. in r37617, it should just be $results[] = (array) get_site( $_site );)

A potentially better option would even to just implement ArrayAccess for WP_Site instead (Although that'll fail any is_array() checks that people have added for some reason) but has the potential to reduce issues and give a consistent object for the future.. unlike this, where legacy code will still be passing arrays around, when it could just be using a WP_Site object, reducing the amount of changes needed to "upgrade".

[jeremyfelt]

In 37618:

Multisite: Bump last_changed cache on site update and creation

When a site is added, updated, or deleted, the site_ids cache for a query will no longer be reliable. Bumping last_changed will force a new query for site IDs.

See #35791.

[jeremyfelt]

Replying to dd32:

Just wanted to quickly weigh in on the usage of ARRAY_A here.. is it really needed? and when would you ever want to do ARRAY_N?

It looks like it's being used for BC, but, it feels like that if the calling function is needing BC-data, it should be the one to massage the new API's return format into something it understands. (ie. in r37617, it should just be $results[] = (array) get_site( $_site );)

I think the biggest reason right now is alignment with get_post(), get_comment(), and get_term(). I wouldn't be opposed to removing that argument and providing a proper WP_Site object.

Because ARRAY_A is available, it's used to help wp_get_sites() maintain back-compat.

A potentially better option would even to just implement ArrayAccess for WP_Site instead (Although that'll fail any is_array() checks that people have added for some reason) but has the potential to reduce issues and give a consistent object for the future.. unlike this, where legacy code will still be passing arrays around, when it could just be using a WP_Site object, reducing the amount of changes needed to "upgrade".

Good point, this could be a good way of encouraging those with legacy code to switch to get_sites().

[dd32]

Replying to jeremyfelt:

Replying to dd32:

Just wanted to quickly weigh in on the usage of ARRAY_A here.. is it really needed? and when would you ever want to do ARRAY_N?

It looks like it's being used for BC, but, it feels like that if the calling function is needing BC-data, it should be the one to massage the new API's return format into something it understands. (ie. in r37617, it should just be $results[] = (array) get_site( $_site );)

I think the biggest reason right now is alignment with get_post(), get_comment(), and get_term(). I wouldn't be opposed to removing that argument and providing a proper WP_Site object.

Yeah, I figured that was the main reason, but I don't think it's something we need/should carry forward to newer API's with the move towards proper objects.

[jeremyfelt]

In 37620:

Multisite: Replace $wpdb->blog queries in ms-functions.php with get_sites()

get_sites() is now used in:

• domain_exists()
• wp_update_network_site_counts()
• get_blog_id_from_url()

Props spacedmonkey, jeremyfelt.
See #35791.

[jeremyfelt]

In 37628:

Multisite: Replace $wpdb->blog queries in get_site_by_path() with get_sites()

Props spacedmonkey, DrewAPicture.
See #35791.

[jeremyfelt]

Remove $object parameter from get_site()

[jeremyfelt]

35791-get-site.diff​ removes the output parameter from get_site() and instead casts (array) on individual sites inside wp_get_sites() to maintain back-compat.

I'm not sure we need to implement ArrayAccess for WP_Site. Most developers using sites in this way should be aware of either WP_Site or the stdClass objects that preceded it. wp_get_sites() is an outlier because it queried the database directly and requested ARRAY_A to solve a specific problem.

[flixos90]

Replying to jeremyfelt:

I'm not sure we need to implement ArrayAccess for WP_Site. Most developers using sites in this way should be aware of either WP_Site or the stdClass objects that preceded it. wp_get_sites() is an outlier because it queried the database directly and requested ARRAY_A to solve a specific problem.

I agree that we rather shouldn't do it. Keeping it a regular object only will be a more structured long-term solution even if that leaves us with a little higher amount of functions that we need to adjust after wp_get_sites() has been deprecated (use objects instead of arrays).

[jeremyfelt]

In 37652:

Multisite: Remove the output parameter from get_site()

Full WP_Site objects should be expected from get_site() rather than arrays.

In the single (soon to be deprecated) use of arrays for this in core, we can cast the result to (array) for back-compat.

See #35791.

[jeremyfelt]

35791.diff​ adds handling for a search_columns query var, similar to WP_User_Query, which allows us to separate searches between domain and path when needed. It also adds the wildcard handling (via @flixos90) from the patches on #36675.

This allows us to better handle searches in WP_MS_Sites_List_Table when in a subdirectory configuration as all domains will usually be the same and should not be searched. A site_search_columns filter is added for anyone wishing to change the columns on a search.

Tests for domain/path searching and wildcard searching included.

[flixos90]

Replying to jeremyfelt:

35791.diff​ adds handling for a search_columns query var, similar to WP_User_Query, which allows us to separate searches between domain and path when needed. It also adds the wildcard handling (via @flixos90) from the patches on #36675.

This allows us to better handle searches in WP_MS_Sites_List_Table when in a subdirectory configuration as all domains will usually be the same and should not be searched. A site_search_columns filter is added for anyone wishing to change the columns on a search.

It might be useful to add an additional filter site_search_available_columns to filter the array( 'domain', 'path' ) bit which is used to intersect the array. I would think that most of the time you would wanna adjust what's available instead of forcing the query var to have a specific value. This could be achived with the existing site_search_columns filter as well, but with a little overhead.

[flixos90]

I just noticed that the docs for get_site() were not adjusted after removing the $output parameter. 35791-docfix.diff​ fixes that (and also fixes some type docs for the function).

[DrewAPicture]

In 37699:

Docs: Update the documentation for get_site() and the get_site filter following the removal of the $output parameter in [37652].

Props flixos90.
See #35791.

[jeremyfelt]

In 37735:

Multisite: Add search column support to WP_Site_Query.

domain and/or path can be used to specify which column(s) should be searched.

See #35791.

[spacedmonkey]

Added a fix for an issue that @ocean90 spotted. Basically if the site query is coming from cache the 'SELECT FOUND_ROWS()' query still runs. In 35791-cachefound.diff I have written a patch, where the result of the found rows query is not run.

Caching this result is similar to ​advanced-post-cache plugin saves running the found rows query.

Looping @jeremyfelt as this should be fixed before 4.6.

[spacedmonkey]

35791-cachefound-2.diff is a massive refactor of the found sites code. It adds a new private method called set_found_sites, which brings it into line with the WP_Query class. The other big change is that the found_sites and max_num_pages variables are also saved in the same cache as the site ids. This means that only the query variables are stored in one cache. This will save multi calls to cache.

If no_found_rows is true, the cache will save a 0 value, this seems a little pointless, but it does make the cache consistent.

[jeremyfelt]

In 37868:

Multisite: Cache found_sites and max_num_pages in WP_Site_Query.

This avoids a second uncached query used to determine found rows. Instead, we can cache the number of found sites and the max number of pages for reuse when the same query is requested in the future.

Props spacedmonkey.
See #35791.

[flixos90]

35791.2.diff​ gets rid of an unnecessary property being set. It probably got there accidentally.

[jeremyfelt]

In 37875:

Multisite: Remove unused site_count property from WP_Site_Query.

Related: [37837].

Props flixos90.
See #35791.

[jeremyfelt]

Closing this as fixed. Great progress everyone!

Let's open new issues for anything that comes up.

[spacedmonkey]

Added 35791-dontcache-max_num_pages.diff . This bring site query inline with 38001 and comment query

[jeremyfelt]

Re-opening to address concerns from fixing the found rows issue in comment queries. See https://core.trac.wordpress.org/ticket/37184#comment:7

[jeremyfelt]

In 38002:

Multisite: Don't store max_num_pages in WP_Site_Query query cache.

This value can be easily calculated with available data.

Props spacedmonkey.
Fixes #35791.

[jeremyfelt]

In 38008:

Docs: Correct the description of the $network_id in WP_Site_Query.

Passing 0 for network_id results in a query across all networks.

See #35791.

[SergeyBiryukov]

In 38085:

Multisite: Correct default values for orderby and order in WP_Site_Query::__construct().

Add a unit test.

Props ramiy, SergeyBiryukov.
See #35791.

[SergeyBiryukov]

In 38103:

Docs: Clarify the fields argument description in WP_Site_Query::__construct().

Props ramiy.
See #35791.

[flixos90]

In 40372:

Multisite: Fix wp_get_sites() to return an unlimited amount of sites when passing a falsy limit argument.

Props iandunn for the original patch.
Fixes #39879. See #35791.