Commit aa5336428dc for woocommerce
commit aa5336428dc02a6982b2090a31c533c6af4dd051
Author: Michal Iwanow <4765119+mcliwanow@users.noreply.github.com>
Date: Wed Jul 22 11:29:00 2026 +0200
Push notifications: stop enumerating role members when fetching push tokens (#66786)
* Stop enumerating role members when fetching push tokens
get_tokens_for_roles() started with get_users( [ 'role__in' => ... ] ),
which WP_User_Query compiles to a LIKE scan over the wp_capabilities
rows of every user on the site. The scan runs on every notification
send and REST token listing, and takes ~60 seconds on sites with
multi-million-row user tables while returning only a handful of IDs.
Invert the lookup: query the push token CPT directly (the small set)
and verify each distinct token owner's role via get_userdata(), which
is an object-cached primary key lookup. Post caches are primed in one
query before the loop. Pagination totals now count stored tokens,
including any whose owner lost the eligible role; such tokens are
excluded from the returned objects (documented on the method).
Adds tests for deleted owners, pagination totals, and a guard
asserting no WP_User_Query runs on this path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Remove push tokens when owners are deleted or lose eligibility
Completes the token lifecycle for the role-scan fix: tokens previously
persisted forever unless explicitly unregistered via the REST DELETE
route.
- deleted_user now removes the user's tokens.
- set_user_role / remove_user_role remove tokens when the user no
longer holds any role eligible for push notifications.
- get_tokens_for_roles() deletes orphaned tokens (owner no longer
exists) when it encounters them, clearing pre-existing debris.
- New PushTokensDataStore::delete_tokens_for_user() backs all three
paths and invalidates the per-request role query cache.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Target 11.0.0 in since tags
The fix is intended for the 11.0.0 release (cherry-pick from trunk to
release/11.0 via the milestone flow), since #66088 ships the feature
enabled by default in 11.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Revoke tokens on delete_user to cover content reassignment
The push token post type already declares delete_with_user, so plain
user deletions clean up natively and a deleted_user hook is dead code
(it fires after posts are deleted or reassigned). Hook delete_user
instead, which runs before: this also covers deletions with content
reassignment, where the tokens would otherwise transfer to the
reassignee and the deleted user's devices would keep receiving
notifications.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Adopt review feedback: pre-filter user lookup by token owners
Per review: restrict the get_users( role__in ) lookup to the DISTINCT
post_author set of existing push token posts (one indexed query on the
small token CPT), instead of role-checking each token owner after the
fact. This keeps the original pagination semantics (totals count only
tokens owned by role-matching users) while still avoiding the full
wp_usermeta capabilities scan.
Also per review: prime post and meta caches with a single
_prime_post_caches() call, and drop the token lifecycle hooks from this
PR (tracked separately) so it stays focused on the performance fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Guard the zero-token fast path against empty-include fallback
WP_User_Query ignores an empty include argument, so passing one would
silently fall back to the unrestricted role scan. Document the
short-circuit on the ternary and add a test asserting that no user
query runs at all when no tokens exist, which is the common case on
stores without the mobile app paired.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Use pre-increment per coding standards
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* address code review: shorten comments + simplify code
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/fix-push-token-role-scan b/plugins/woocommerce/changelog/fix-push-token-role-scan
new file mode 100644
index 00000000000..d070654606b
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-push-token-role-scan
@@ -0,0 +1,4 @@
+Significance: patch
+Type: performance
+
+Push notifications: Improve performance of get_tokens_for_roles(): the role lookup is now restricted to users that actually own push tokens.
diff --git a/plugins/woocommerce/src/Internal/PushNotifications/DataStores/PushTokensDataStore.php b/plugins/woocommerce/src/Internal/PushNotifications/DataStores/PushTokensDataStore.php
index 0ad5d89ff9e..45cedabe821 100644
--- a/plugins/woocommerce/src/Internal/PushNotifications/DataStores/PushTokensDataStore.php
+++ b/plugins/woocommerce/src/Internal/PushNotifications/DataStores/PushTokensDataStore.php
@@ -318,6 +318,11 @@ class PushTokensDataStore {
* flat array (cached per-request). When $page and $per_page are
* provided, returns a paginated result with total counts.
*
+ * The eligible-user lookup is restricted to users that actually own
+ * push tokens, so the role check runs against a handful of IDs instead
+ * of scanning every user's capabilities meta, which does not scale on
+ * sites with very large user tables.
+ *
* @param string[] $roles The roles to query tokens for.
* @param int|null $page Optional page number (1-based).
* @param int|null $per_page Optional number of tokens per page.
@@ -345,10 +350,22 @@ class PushTokensDataStore {
return $this->tokens_by_roles_cache[ $cache_key ];
}
- $user_ids = get_users(
+ global $wpdb;
+
+ // Exactly this SQL to leverage the wp_posts type_status_author index; low token cardinality keeps it fast at any store size.
+ $users_with_tokens = $wpdb->get_col(
+ $wpdb->prepare(
+ "SELECT DISTINCT post_author FROM {$wpdb->posts} WHERE post_type = %s AND post_status = 'private'",
+ PushToken::POST_TYPE
+ )
+ );
+
+ // An empty include must short-circuit: WP_User_Query would ignore it and scan all users by role.
+ $user_ids = empty( $users_with_tokens ) ? array() : get_users(
array(
'role__in' => $roles,
'fields' => 'ID',
+ 'include' => $users_with_tokens,
)
);
@@ -386,7 +403,7 @@ class PushTokensDataStore {
return $this->tokens_by_roles_cache[ $cache_key ];
}
- update_meta_cache( 'post', $post_ids );
+ _prime_post_caches( $post_ids, false, true );
$tokens = array();
diff --git a/plugins/woocommerce/tests/php/src/Internal/PushNotifications/DataStores/PushTokensDataStoreTest.php b/plugins/woocommerce/tests/php/src/Internal/PushNotifications/DataStores/PushTokensDataStoreTest.php
index 9d226cd894e..2173cae36f9 100644
--- a/plugins/woocommerce/tests/php/src/Internal/PushNotifications/DataStores/PushTokensDataStoreTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/PushNotifications/DataStores/PushTokensDataStoreTest.php
@@ -853,6 +853,112 @@ class PushTokensDataStoreTest extends WC_Unit_Test_Case {
$this->assertSame( array(), $tokens );
}
+ /**
+ * @testdox Should exclude tokens whose owner no longer exists.
+ */
+ public function test_get_tokens_for_roles_excludes_tokens_of_deleted_users(): void {
+ $admin_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
+ $data_store = new PushTokensDataStore();
+
+ $this->create_push_token_for_user( $data_store, $admin_id );
+
+ wp_delete_user( $admin_id );
+
+ $tokens = $data_store->get_tokens_for_roles( array( 'administrator' ) );
+
+ $this->assertSame( array(), $tokens );
+ }
+
+ /**
+ * @testdox Should paginate tokens and report totals.
+ */
+ public function test_get_tokens_for_roles_supports_pagination(): void {
+ $admin_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
+ $data_store = new PushTokensDataStore();
+
+ for ( $i = 0; $i < 3; $i++ ) {
+ $this->create_push_token_for_user( $data_store, $admin_id );
+ }
+
+ $page_one = $data_store->get_tokens_for_roles( array( 'administrator' ), 1, 2 );
+
+ $this->assertCount( 2, $page_one['tokens'] );
+ $this->assertSame( 3, $page_one['total'] );
+ $this->assertSame( 2, $page_one['total_pages'] );
+
+ $page_two = $data_store->get_tokens_for_roles( array( 'administrator' ), 2, 2 );
+
+ $this->assertCount( 1, $page_two['tokens'] );
+ }
+
+ /**
+ * @testdox Should not run any user query when no tokens exist.
+ */
+ public function test_get_tokens_for_roles_skips_user_query_when_no_tokens_exist(): void {
+ $this->factory->user->create( array( 'role' => 'administrator' ) );
+ $data_store = new PushTokensDataStore();
+
+ $user_queries = 0;
+ $count = function () use ( &$user_queries ) {
+ ++$user_queries;
+ };
+ add_action( 'pre_get_users', $count );
+
+ $tokens = $data_store->get_tokens_for_roles( array( 'administrator' ) );
+
+ remove_action( 'pre_get_users', $count );
+
+ $this->assertSame( array(), $tokens );
+ $this->assertSame( 0, $user_queries, 'With no stored tokens there is nothing to look up: an empty include must short-circuit before get_users(), because WP_User_Query ignores an empty include argument and would fall back to the unrestricted role scan.' );
+ }
+
+ /**
+ * @testdox Should only run user queries restricted to token owners.
+ */
+ public function test_get_tokens_for_roles_only_queries_users_owning_tokens(): void {
+ $admin_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
+ $data_store = new PushTokensDataStore();
+
+ $this->create_push_token_for_user( $data_store, $admin_id );
+
+ $includes = array();
+ $capture = function ( $query ) use ( &$includes ) {
+ $includes[] = $query->query_vars['include'];
+ };
+ add_action( 'pre_get_users', $capture );
+
+ $tokens = $data_store->get_tokens_for_roles( array( 'administrator' ) );
+
+ remove_action( 'pre_get_users', $capture );
+
+ $this->assertCount( 1, $tokens );
+ $this->assertNotEmpty( $includes, 'The role lookup should run through WP_User_Query.' );
+ foreach ( $includes as $include ) {
+ $this->assertNotEmpty( $include, 'User queries on this path must be restricted to token owners: an unrestricted role__in query scans the capabilities meta of every user and does not scale on large sites.' );
+ }
+ }
+
+ /**
+ * Creates a push token owned by the given user.
+ *
+ * @param PushTokensDataStore $data_store The data store instance.
+ * @param int $user_id The owner user ID.
+ * @return PushToken The created push token object.
+ */
+ private function create_push_token_for_user( PushTokensDataStore $data_store, int $user_id ): PushToken {
+ return $data_store->create(
+ array(
+ 'user_id' => $user_id,
+ 'token' => 'test_token_' . wp_rand(),
+ 'platform' => PushToken::PLATFORM_APPLE,
+ 'device_uuid' => 'test-device-uuid-' . wp_rand(),
+ 'origin' => PushToken::ORIGIN_WOOCOMMERCE_IOS,
+ 'device_locale' => 'en_US',
+ 'metadata' => array( 'app_version' => '1.0' ),
+ )
+ );
+ }
+
/**
* Creates a test push token and saves it to the database.
*