Skip to content

Commit d903fc5

Browse files
marcusquinnmarcusquinn
authored
fix: guard cron scheduling to main site only in multisite (#1137) (#1149)
* fix: guard cron scheduling to main site only in multisite networks * test: add PHPUnit tests for multisite cron guard logic - Add test-multisite-cron-guard.php covering both code changes: - schedule_access_token_cleanup(): verifies cron event is scheduled on single-site and main site, skipped on subsites - delete_all_cached_data(): verifies wp_cron() fires on single-site and main site, skipped on subsites - Tests use markTestSkipped() to self-select based on WP_MULTISITE env - Add multisite PHPUnit step to CI workflow (WP_MULTISITE=1) so both single-site and multisite code paths are exercised in CI * fix: use cron_request filter to test wp_cron() guard in PHPUnit wp_cron() spawns an async HTTP loopback via spawn_cron() — it does not execute cron callbacks synchronously. In the PHPUnit test environment there is no HTTP server, so the loopback silently fails and scheduled events never fire. Replace the 'schedule event → check if callback fired' pattern with a 'cron_request' filter hook that detects whether spawn_cron() was reached. This correctly tests the multisite guard logic (the if-condition) without depending on HTTP infrastructure. Also clear the doing_cron transient before each test to prevent spawn_cron() from bailing due to a stale lock. * fix: detect wp_cron() via shutdown hook, not cron_request filter wp_cron() in WP 6.7+ defers to _wp_cron() via add_action('shutdown'), so the cron_request filter never fires during the test. Instead, verify the guard logic by checking whether _wp_cron was hooked to shutdown (or wp_loaded for ALTERNATE_WP_CRON). Tested locally against wp-env (PHP 8.1, WP 7.1-alpha, PHPUnit 9.6): - Single-site: 16 tests, 19 assertions, 0 failures - Multisite: 16 tests, 24 assertions, 0 failures
1 parent b538d18 commit d903fc5

4 files changed

Lines changed: 243 additions & 2 deletions

File tree

‎.github/workflows/wp-phpunit.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ jobs:
5858
- name: Install WP Tests
5959
run: bash bin/install-wp-tests.sh wordpress_test root root 127.0.0.1 latest
6060

61-
- name: PHPUnit tests
61+
- name: PHPUnit tests (single site)
6262
run: |
6363
echo "define('WP_TESTS_PHPUNIT_POLYFILLS_PATH', '$HOME/.composer/vendor/yoast/phpunit-polyfills');" >> /tmp/wordpress-tests-lib/wp-tests-config.php
6464
phpunit --config=phpunit.xml
65+
66+
- name: PHPUnit tests (multisite)
67+
run: |
68+
WP_MULTISITE=1 phpunit --config=phpunit.xml

‎src/Git_Updater/GU_Upgrade.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ static function ( $value, $key ) use ( &$options, $base_options ) {
151151
* @return void
152152
*/
153153
private function schedule_access_token_cleanup() {
154+
if ( is_multisite() && ! is_main_site() ) {
155+
return;
156+
}
157+
154158
if ( false === wp_next_scheduled( 'gu_delete_access_tokens' ) ) {
155159
wp_schedule_event( time() + \MONTH_IN_SECONDS, 'twicedaily', 'gu_delete_access_tokens' );
156160
}

‎src/Git_Updater/Traits/GU_Trait.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,9 @@ final public function delete_all_cached_data() {
293293

294294
$wpdb->query( $wpdb->prepare( $delete_string, [ '%ghu-%' ] ) ); // phpcs:ignore
295295

296-
wp_cron();
296+
if ( ! is_multisite() || is_main_site() ) {
297+
wp_cron();
298+
}
297299

298300
return true;
299301
}

‎tests/test-multisite-cron-guard.php‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
<?php
2+
/**
3+
* Test multisite cron guard logic.
4+
*
5+
* Tests for:
6+
* - GU_Upgrade::schedule_access_token_cleanup() — early return on subsites.
7+
* - GU_Trait::delete_all_cached_data() — wp_cron() skipped on subsites.
8+
*
9+
* Run with WP_TESTS_MULTISITE=true to exercise the multisite code paths.
10+
* In single-site mode, the guards pass through and behavior is unchanged.
11+
*
12+
* @package Git_Updater
13+
*/
14+
15+
/**
16+
* Class Test_Multisite_Cron_Guard
17+
*
18+
* Uses the GU_Trait directly and invokes GU_Upgrade's private method
19+
* via reflection (GU_Upgrade is final, so we cannot extend it).
20+
*/
21+
class Test_Multisite_Cron_Guard extends \WP_UnitTestCase {
22+
23+
use Fragen\Git_Updater\Traits\GU_Trait;
24+
25+
/**
26+
* Clean up cron events and hooks after each test.
27+
*/
28+
public function tear_down() {
29+
wp_clear_scheduled_hook( 'gu_delete_access_tokens' );
30+
remove_action( 'shutdown', '_wp_cron' );
31+
remove_action( 'wp_loaded', '_wp_cron', 20 );
32+
parent::tear_down();
33+
}
34+
35+
/*
36+
|--------------------------------------------------------------------------
37+
| GU_Trait::delete_all_cached_data() — wp_cron() guard
38+
|--------------------------------------------------------------------------
39+
*/
40+
41+
/**
42+
* On a single site, delete_all_cached_data() should call wp_cron().
43+
*
44+
* wp_cron() defers actual work to _wp_cron() via the 'shutdown' action
45+
* (or 'wp_loaded' when ALTERNATE_WP_CRON is set). We verify the guard
46+
* allowed wp_cron() to run by checking that _wp_cron was hooked.
47+
*/
48+
public function test_delete_all_cached_data_calls_wp_cron_on_single_site() {
49+
if ( is_multisite() ) {
50+
$this->markTestSkipped( 'Single-site test — skipped under multisite.' );
51+
}
52+
53+
// Remove any pre-existing _wp_cron hook so we can detect a fresh add.
54+
remove_action( 'shutdown', '_wp_cron' );
55+
remove_action( 'wp_loaded', '_wp_cron', 20 );
56+
57+
$result = $this->delete_all_cached_data();
58+
59+
$this->assertTrue( $result, 'delete_all_cached_data() should return true.' );
60+
$this->assertTrue(
61+
has_action( 'shutdown', '_wp_cron' ) !== false
62+
|| has_action( 'wp_loaded', '_wp_cron' ) !== false,
63+
'wp_cron() should hook _wp_cron on single-site.'
64+
);
65+
66+
// Clean up.
67+
remove_action( 'shutdown', '_wp_cron' );
68+
remove_action( 'wp_loaded', '_wp_cron', 20 );
69+
}
70+
71+
/**
72+
* On the main site of a multisite network, delete_all_cached_data()
73+
* should call wp_cron().
74+
*
75+
* @group multisite
76+
*/
77+
public function test_delete_all_cached_data_calls_wp_cron_on_main_site() {
78+
if ( ! is_multisite() ) {
79+
$this->markTestSkipped( 'Multisite test — skipped under single-site.' );
80+
}
81+
82+
// Ensure we're on the main site.
83+
switch_to_blog( get_main_site_id() );
84+
85+
// Remove any pre-existing _wp_cron hook so we can detect a fresh add.
86+
remove_action( 'shutdown', '_wp_cron' );
87+
remove_action( 'wp_loaded', '_wp_cron', 20 );
88+
89+
$result = $this->delete_all_cached_data();
90+
91+
$this->assertTrue( $result, 'delete_all_cached_data() should return true.' );
92+
$this->assertTrue(
93+
has_action( 'shutdown', '_wp_cron' ) !== false
94+
|| has_action( 'wp_loaded', '_wp_cron' ) !== false,
95+
'wp_cron() should hook _wp_cron on the main site.'
96+
);
97+
98+
remove_action( 'shutdown', '_wp_cron' );
99+
remove_action( 'wp_loaded', '_wp_cron', 20 );
100+
restore_current_blog();
101+
}
102+
103+
/**
104+
* On a subsite of a multisite network, delete_all_cached_data()
105+
* should NOT call wp_cron().
106+
*
107+
* @group multisite
108+
*/
109+
public function test_delete_all_cached_data_skips_wp_cron_on_subsite() {
110+
if ( ! is_multisite() ) {
111+
$this->markTestSkipped( 'Multisite test — skipped under single-site.' );
112+
}
113+
114+
// Create a subsite and switch to it.
115+
$blog_id = self::factory()->blog->create();
116+
switch_to_blog( $blog_id );
117+
118+
$this->assertFalse( is_main_site(), 'Should be on a subsite.' );
119+
120+
// Remove any pre-existing _wp_cron hook so we can detect a fresh add.
121+
remove_action( 'shutdown', '_wp_cron' );
122+
remove_action( 'wp_loaded', '_wp_cron', 20 );
123+
124+
$result = $this->delete_all_cached_data();
125+
126+
$this->assertTrue( $result, 'delete_all_cached_data() should still return true.' );
127+
$this->assertFalse(
128+
has_action( 'shutdown', '_wp_cron' ) !== false
129+
|| has_action( 'wp_loaded', '_wp_cron' ) !== false,
130+
'wp_cron() should NOT hook _wp_cron on a subsite.'
131+
);
132+
133+
restore_current_blog();
134+
}
135+
136+
/*
137+
|--------------------------------------------------------------------------
138+
| GU_Upgrade::schedule_access_token_cleanup() — multisite guard
139+
|--------------------------------------------------------------------------
140+
|
141+
| schedule_access_token_cleanup() is private, so we test it indirectly
142+
| by checking whether the cron event gets scheduled after construction
143+
| scenarios. We use reflection to invoke the private method.
144+
*/
145+
146+
/**
147+
* Get a GU_Upgrade instance and invoke schedule_access_token_cleanup().
148+
*
149+
* @return void
150+
*/
151+
private function invoke_schedule_access_token_cleanup() {
152+
$upgrade = new Fragen\Git_Updater\GU_Upgrade();
153+
$reflection = new ReflectionMethod( $upgrade, 'schedule_access_token_cleanup' );
154+
$reflection->setAccessible( true );
155+
$reflection->invoke( $upgrade );
156+
}
157+
158+
/**
159+
* On a single site, schedule_access_token_cleanup() should schedule
160+
* the gu_delete_access_tokens cron event.
161+
*/
162+
public function test_schedule_access_token_cleanup_schedules_on_single_site() {
163+
if ( is_multisite() ) {
164+
$this->markTestSkipped( 'Single-site test — skipped under multisite.' );
165+
}
166+
167+
// Ensure no pre-existing event.
168+
wp_clear_scheduled_hook( 'gu_delete_access_tokens' );
169+
$this->assertFalse( wp_next_scheduled( 'gu_delete_access_tokens' ), 'Precondition: no event scheduled.' );
170+
171+
$this->invoke_schedule_access_token_cleanup();
172+
173+
$this->assertNotFalse(
174+
wp_next_scheduled( 'gu_delete_access_tokens' ),
175+
'gu_delete_access_tokens should be scheduled on single-site.'
176+
);
177+
}
178+
179+
/**
180+
* On the main site of a multisite network, schedule_access_token_cleanup()
181+
* should schedule the cron event.
182+
*
183+
* @group multisite
184+
*/
185+
public function test_schedule_access_token_cleanup_schedules_on_main_site() {
186+
if ( ! is_multisite() ) {
187+
$this->markTestSkipped( 'Multisite test — skipped under single-site.' );
188+
}
189+
190+
switch_to_blog( get_main_site_id() );
191+
wp_clear_scheduled_hook( 'gu_delete_access_tokens' );
192+
193+
$this->invoke_schedule_access_token_cleanup();
194+
195+
$this->assertNotFalse(
196+
wp_next_scheduled( 'gu_delete_access_tokens' ),
197+
'gu_delete_access_tokens should be scheduled on the main site.'
198+
);
199+
200+
restore_current_blog();
201+
}
202+
203+
/**
204+
* On a subsite of a multisite network, schedule_access_token_cleanup()
205+
* should NOT schedule the cron event.
206+
*
207+
* @group multisite
208+
*/
209+
public function test_schedule_access_token_cleanup_skips_on_subsite() {
210+
if ( ! is_multisite() ) {
211+
$this->markTestSkipped( 'Multisite test — skipped under single-site.' );
212+
}
213+
214+
$blog_id = self::factory()->blog->create();
215+
switch_to_blog( $blog_id );
216+
217+
$this->assertFalse( is_main_site(), 'Should be on a subsite.' );
218+
219+
wp_clear_scheduled_hook( 'gu_delete_access_tokens' );
220+
$this->assertFalse( wp_next_scheduled( 'gu_delete_access_tokens' ), 'Precondition: no event scheduled.' );
221+
222+
$this->invoke_schedule_access_token_cleanup();
223+
224+
$this->assertFalse(
225+
wp_next_scheduled( 'gu_delete_access_tokens' ),
226+
'gu_delete_access_tokens should NOT be scheduled on a subsite.'
227+
);
228+
229+
restore_current_blog();
230+
}
231+
}

0 commit comments

Comments
 (0)