Fix/utils get site null guard#4319
Conversation
…tent site IDs On PHP 8.x, calling \get_site() with an ID that does not exist returns null rather than a WP_Site object. The wrapper Utils\\get_site() was passing that null directly to array property access, causing a fatal TypeError. is_site_indexable() then received false-y data and could also propagate a null return where an array is expected. Fix: add an instanceof WP_Site guard in get_site() that returns [] when the core function returns null, and an empty() guard in is_site_indexable() that short-circuits to false for non-existent site IDs. Both changes are consistent with the existing @return array contract and the behavior the callers already expect for non-indexable sites.
…t site IDs Two tests covering the null-guard introduced in the prior commit: - testGetSiteReturnsEmptyArrayForNonexistentSite - testIsSiteIndexableReturnsFalseForNonexistentSite Both are tagged @group skip-on-single-site since the functions are multisite-only paths.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens multisite utility functions to handle non-existent site IDs safely and adds regression tests to cover the new behavior.
Changes:
- Return an empty array from
ElasticPress\Utils\get_site()when WordPress can’t resolve the site ID. - Avoid calling
get_site_meta()inis_site_indexable()when the site lookup fails. - Add multisite-only tests for both behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/php/TestUtils.php | Adds multisite-only regression tests for non-existent site IDs. |
| includes/utils.php | Adds guards for missing sites in get_site() and is_site_indexable(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function get_site( $site_id ) { | ||
| $site = \get_site( $site_id ); | ||
|
|
||
| if ( ! $site instanceof \WP_Site ) { | ||
| return []; | ||
| } |
| public function testGetSiteReturnsEmptyArrayForNonexistentSite() { | ||
| $result = ElasticPress\Utils\get_site( PHP_INT_MAX ); | ||
| $this->assertSame( [], $result ); | ||
| } |
|
Thanks for the review — addressing both points:
Returning |
Description of the Change
closes #4318
How to test the Change
Changelog Entry
Credits
Props @username, ...
Checklist: