Commit 879eda5bf21 for woocommerce
commit 879eda5bf21f114da624b15cfa61b885d606da87
Author: Chris Lilitsas <1105590+xristos3490@users.noreply.github.com>
Date: Wed Sep 16 14:35:14 2026 +0300
Harden Back in Stock Notifications feature gate after alpha constant move (#68761)
* fix: harden Back in Stock Notifications feature gate after alpha constant move
* fix: gate services via FeaturesUtil, keep raw option read for data store
* fix: honor BIS alpha constant only until the 11.2.0 DB migration runs
* test: cover the fail-open sign-up path when the rate limit cannot be claimed
* fix: fail soft in every NotificationQuery method while the feature is off
diff --git a/plugins/woocommerce/changelog/fix-harden-back-in-stock-notifications-feature-gate b/plugins/woocommerce/changelog/fix-harden-back-in-stock-notifications-feature-gate
new file mode 100644
index 00000000000..f09ae29948d
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-harden-back-in-stock-notifications-feature-gate
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Harden the Back in Stock Notifications feature gate: honor the WOOCOMMERCE_BIS_ALPHA_ENABLED constant until the option migration runs, fail soft in NotificationQuery when the feature is off, make SignupRateLimiter static so SignupService::init() keeps its three-argument signature, and deprecate StockNotifications::init_hooks() in favor of maybe_init_services().
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupRateLimiter.php b/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupRateLimiter.php
index 11d5f6c9039..eb9ad747b50 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupRateLimiter.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupRateLimiter.php
@@ -64,8 +64,8 @@ class SignupRateLimiter {
* @param string $user_email The e-mail address used to sign up.
* @return bool True if the attempt must be rejected.
*/
- public function is_rate_limited( string $user_email ): bool {
- foreach ( array_keys( $this->get_rate_limits( $user_email ) ) as $rate_limit_id ) {
+ public static function is_rate_limited( string $user_email ): bool {
+ foreach ( array_keys( self::get_rate_limits( $user_email ) ) as $rate_limit_id ) {
if ( ! WC_Rate_Limiter::retried_too_soon( $rate_limit_id ) ) {
continue;
}
@@ -99,10 +99,10 @@ class SignupRateLimiter {
* @param string $user_email The e-mail address used to sign up.
* @return bool True if every rate limit was applied.
*/
- public function apply( string $user_email ): bool {
+ public static function apply( string $user_email ): bool {
$applied_rate_limit_ids = array();
- foreach ( $this->get_rate_limits( $user_email ) as $rate_limit_id => $delay ) {
+ foreach ( self::get_rate_limits( $user_email ) as $rate_limit_id => $delay ) {
if ( ! WC_Rate_Limiter::set_rate_limit( $rate_limit_id, $delay ) ) {
// Leave no partial window behind: a half-applied limit would block the
// customer on an attempt that never went through. The failed limit is
@@ -129,8 +129,8 @@ class SignupRateLimiter {
* @param string $user_email The e-mail address used to sign up.
* @return array<string, int>
*/
- private function get_rate_limits( string $user_email ): array {
- $options = $this->get_options();
+ private static function get_rate_limits( string $user_email ): array {
+ $options = self::get_options();
$rate_limits = array();
if ( ! $options['enabled'] ) {
@@ -141,7 +141,7 @@ class SignupRateLimiter {
if ( is_user_logged_in() ) {
$rate_limits[ self::RATE_LIMIT_USER_PREFIX . get_current_user_id() ] = $options['client_delay'];
} else {
- $ip_address = $this->get_ip_address( $options['proxy_support'] );
+ $ip_address = self::get_ip_address( $options['proxy_support'] );
if ( '' !== $ip_address ) {
$rate_limits[ self::RATE_LIMIT_IP_PREFIX . hash( 'sha256', $ip_address ) ] = $options['client_delay'];
}
@@ -166,7 +166,7 @@ class SignupRateLimiter {
* @param bool $proxy_support Whether to read the client address from forwarding headers.
* @return string The IP address, or an empty string if it could not be resolved.
*/
- private function get_ip_address( bool $proxy_support ): string {
+ private static function get_ip_address( bool $proxy_support ): string {
if ( $proxy_support ) {
return WC_Geolocation::get_ip_address();
}
@@ -181,7 +181,7 @@ class SignupRateLimiter {
*
* @return array{enabled: bool, proxy_support: bool, client_delay: int, email_delay: int}
*/
- private function get_options(): array {
+ private static function get_options(): array {
$defaults = array(
'enabled' => self::ENABLED,
'proxy_support' => self::PROXY_SUPPORT,
@@ -220,8 +220,8 @@ class SignupRateLimiter {
// default, and a negative delay would clear the limit instead of setting one, so it
// is clamped to zero.
return array(
- 'enabled' => $this->to_bool( $options['enabled'] ?? $defaults['enabled'], $defaults['enabled'] ),
- 'proxy_support' => $this->to_bool( $options['proxy_support'] ?? $defaults['proxy_support'], $defaults['proxy_support'] ),
+ 'enabled' => self::to_bool( $options['enabled'] ?? $defaults['enabled'], $defaults['enabled'] ),
+ 'proxy_support' => self::to_bool( $options['proxy_support'] ?? $defaults['proxy_support'], $defaults['proxy_support'] ),
'client_delay' => is_numeric( $options['client_delay'] ?? null ) ? max( 0, (int) $options['client_delay'] ) : $defaults['client_delay'],
'email_delay' => is_numeric( $options['email_delay'] ?? null ) ? max( 0, (int) $options['email_delay'] ) : $defaults['email_delay'],
);
@@ -233,7 +233,7 @@ class SignupRateLimiter {
* @param mixed $value The filtered value.
* @param bool $fallback The value to use when the filtered value spells no boolean.
*/
- private function to_bool( $value, bool $fallback ): bool {
+ private static function to_bool( $value, bool $fallback ): bool {
if ( is_bool( $value ) ) {
return $value;
}
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupService.php b/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupService.php
index 2e04ce0897b..03a7b7a6eb9 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupService.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/Frontend/SignupService.php
@@ -71,13 +71,6 @@ class SignupService {
*/
private EmailManager $email_manager;
- /**
- * Signup rate limiter.
- *
- * @var SignupRateLimiter
- */
- private SignupRateLimiter $rate_limiter;
-
/**
* The logger.
*
@@ -93,18 +86,15 @@ class SignupService {
* @param EligibilityService $eligibility_service The eligibility service.
* @param NotificationManagementService $notification_management_service The notification management service.
* @param EmailManager $email_manager The email manager.
- * @param SignupRateLimiter $rate_limiter The signup rate limiter.
*/
final public function init(
EligibilityService $eligibility_service,
NotificationManagementService $notification_management_service,
- EmailManager $email_manager,
- SignupRateLimiter $rate_limiter
+ EmailManager $email_manager
) {
$this->eligibility_service = $eligibility_service;
$this->notification_management_service = $notification_management_service;
$this->email_manager = $email_manager;
- $this->rate_limiter = $rate_limiter;
$this->logger = \wc_get_logger();
}
@@ -184,7 +174,7 @@ class SignupService {
}
}
- if ( $this->rate_limiter->is_rate_limited( $user_email ) ) {
+ if ( SignupRateLimiter::is_rate_limited( $user_email ) ) {
return new \WP_Error( self::ERROR_RATE_LIMITED );
}
@@ -195,7 +185,7 @@ class SignupService {
// A claim only fails when the rate limit table cannot be written to, which a shopper
// can neither cause nor resolve. Let the sign-up through rather than turn a broken
// limiter into a store-wide sign-up outage.
- if ( ! $this->rate_limiter->apply( $user_email ) ) {
+ if ( ! SignupRateLimiter::apply( $user_email ) ) {
$this->logger->warning(
'Could not claim the stock notification sign-up rate limit window. Allowing the sign-up to proceed.',
array( 'source' => 'stock-notifications-signup-errors' )
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/NotificationQuery.php b/plugins/woocommerce/src/Internal/StockNotifications/NotificationQuery.php
index b5d557c41c1..6309ad134e9 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/NotificationQuery.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/NotificationQuery.php
@@ -35,6 +35,24 @@ class NotificationQuery {
return (int) self::run_query( $args );
}
+ /**
+ * Load the stock notification data store, or null while the feature is off.
+ *
+ * The data store is only registered while the feature is enabled, so
+ * `WC_Data_Store::load()` throws when it is off. Every query in this class
+ * goes through here so all of them fail soft instead of fataling.
+ *
+ * @return \WC_Data_Store|null
+ */
+ private static function load_data_store(): ?\WC_Data_Store {
+ try {
+ return \WC_Data_Store::load( 'stock_notification' );
+ } catch ( \Exception $e ) {
+ \wc_caught_exception( $e, __METHOD__ );
+ return null;
+ }
+ }
+
/**
* Single dispatch site to the underlying data store's `query()` method.
*
@@ -43,10 +61,12 @@ class NotificationQuery {
*
* @param array $args Query args.
* @return mixed Whatever the data store returns for the requested `return` mode
- * (array of objects/ids, int for `count`).
+ * (array of objects/ids, int for `count`), or null while the feature is off.
*/
private static function run_query( array $args ) {
- return \WC_Data_Store::load( 'stock_notification' )->query( $args );
+ $data_store = self::load_data_store();
+
+ return $data_store ? $data_store->query( $args ) : null;
}
/**
@@ -56,7 +76,9 @@ class NotificationQuery {
* @return bool True if the product has active notifications, false otherwise.
*/
public static function product_has_active_notifications( array $product_ids ): bool {
- return \WC_Data_Store::load( 'stock_notification' )->product_has_active_notifications( $product_ids );
+ $data_store = self::load_data_store();
+
+ return $data_store ? $data_store->product_has_active_notifications( $product_ids ) : false;
}
/**
@@ -67,7 +89,9 @@ class NotificationQuery {
* @return bool True if the notification exists, false otherwise.
*/
public static function notification_exists_by_email( int $product_id, string $email ): bool {
- return \WC_Data_Store::load( 'stock_notification' )->notification_exists_by_email( $product_id, $email );
+ $data_store = self::load_data_store();
+
+ return $data_store ? $data_store->notification_exists_by_email( $product_id, $email ) : false;
}
/**
@@ -78,6 +102,8 @@ class NotificationQuery {
* @return bool True if the notification exists, false otherwise.
*/
public static function notification_exists_by_user_id( int $product_id, int $user_id ): bool {
- return \WC_Data_Store::load( 'stock_notification' )->notification_exists_by_user_id( $product_id, $user_id );
+ $data_store = self::load_data_store();
+
+ return $data_store ? $data_store->notification_exists_by_user_id( $product_id, $user_id ) : false;
}
}
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/StockNotifications.php b/plugins/woocommerce/src/Internal/StockNotifications/StockNotifications.php
index aba1fd9d521..b5640de6796 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/StockNotifications.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/StockNotifications.php
@@ -4,6 +4,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\Internal\StockNotifications;
+use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Internal\DataStores\StockNotifications\StockNotificationsDataStore;
use Automattic\WooCommerce\Internal\Features\FeaturesController;
use Automattic\WooCommerce\Internal\RegisterHooksInterface;
@@ -57,7 +58,7 @@ class StockNotifications implements RegisterHooksInterface {
* Set up the data retention tasks when WooCommerce is installed or updated.
*/
public function on_install_or_update() {
- if ( ! FeaturesUtil::feature_is_enabled( self::FEATURE_NAME ) ) {
+ if ( ! self::is_enabled() ) {
return;
}
@@ -106,7 +107,7 @@ class StockNotifications implements RegisterHooksInterface {
* @internal
*/
public function maybe_init_services(): void {
- if ( ! FeaturesUtil::feature_is_enabled( self::FEATURE_NAME ) ) {
+ if ( ! self::is_enabled() ) {
return;
}
@@ -134,6 +135,32 @@ class StockNotifications implements RegisterHooksInterface {
}
}
+ /**
+ * Register hooks and services.
+ *
+ * Declared without a native return type on purpose: the method it restores had
+ * none, and adding one would fatal any subclass that overrides it.
+ *
+ * @deprecated 11.2.0 Replaced by maybe_init_services().
+ *
+ * @internal
+ *
+ * @return void
+ */
+ public function init_hooks() {
+ wc_deprecated_function( __METHOD__, '11.2.0', __CLASS__ . '::maybe_init_services()' );
+
+ if ( did_action( 'init' ) ) {
+ $this->maybe_init_services();
+ return;
+ }
+
+ // Alpha-era callers ran this on plugins_loaded. Defer so the feature check
+ // runs after the textdomain is loaded, or the feature definitions would be
+ // built untranslated and WordPress would flag the early translation call.
+ add_action( 'init', array( $this, 'maybe_init_services' ), 1 );
+ }
+
/**
* Register the data stores, unless the feature is disabled.
*
@@ -142,10 +169,11 @@ class StockNotifications implements RegisterHooksInterface {
*/
public function register_data_stores( $data_stores ) {
// WC_Data_Store::__construct() re-applies this filter on every data store load,
- // so re-check the option: the feature can be switched off after this callback was
+ // so re-check the feature: it can be switched off after this callback was
// attached at `init`. Read the option directly rather than through
- // feature_is_enabled(), which builds translated feature definitions.
- if ( 'yes' !== get_option( self::ENABLE_OPTION_NAME, 'no' ) ) {
+ // feature_is_enabled(): a data store can be loaded before `init`, and building
+ // the translated feature definitions that early is not safe.
+ if ( 'yes' !== get_option( self::ENABLE_OPTION_NAME, 'no' ) && ! self::is_alpha_enabled() ) {
return $data_stores;
}
@@ -156,4 +184,34 @@ class StockNotifications implements RegisterHooksInterface {
$data_stores['stock_notification'] = wc_get_container()->get( StockNotificationsDataStore::class );
return $data_stores;
}
+
+ /**
+ * Check whether the feature is enabled.
+ *
+ * Must not run before `init`: feature_is_enabled() builds the translated feature
+ * definitions on first use. Every caller is hooked at `init` or later.
+ *
+ * @return bool
+ */
+ private static function is_enabled(): bool {
+ return FeaturesUtil::feature_is_enabled( self::FEATURE_NAME ) || self::is_alpha_enabled();
+ }
+
+ /**
+ * Check whether the site opted in through the alpha constant.
+ *
+ * Only bridges the gap until wc_update_1120_migrate_stock_notifications_alpha_constant()
+ * has copied the constant into the feature option. The database version is bumped
+ * after every 11.2.0 callback has run, so once it reaches 11.2.0 the Features screen
+ * toggle is the only switch.
+ *
+ * @return bool
+ */
+ private static function is_alpha_enabled(): bool {
+ if ( ! Constants::is_true( 'WOOCOMMERCE_BIS_ALPHA_ENABLED' ) ) {
+ return false;
+ }
+
+ return version_compare( (string) get_option( 'woocommerce_db_version', '0' ), '11.2.0', '<' );
+ }
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupRateLimiterTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupRateLimiterTests.php
index 7c985b9c745..fad7013f19c 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupRateLimiterTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupRateLimiterTests.php
@@ -12,13 +12,6 @@ use WC_Unit_Test_Case;
*/
class SignupRateLimiterTests extends WC_Unit_Test_Case {
- /**
- * The System Under Test.
- *
- * @var SignupRateLimiter
- */
- private $sut;
-
/**
* The server values seen before the test replaced them.
*
@@ -38,8 +31,6 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$_SERVER['REMOTE_ADDR'] = '192.0.2.10';
unset( $_SERVER['HTTP_X_FORWARDED_FOR'] );
-
- $this->sut = new SignupRateLimiter();
}
/**
@@ -65,45 +56,45 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
* @testdox Should not rate limit the first attempt.
*/
public function test_first_attempt_is_not_rate_limited(): void {
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A first sign-up attempt should go through' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A first sign-up attempt should go through' );
}
/**
* @testdox Should rate limit a repeated attempt from the same e-mail address.
*/
public function test_repeated_attempt_is_rate_limited(): void {
- $this->assertTrue( $this->sut->apply( 'shopper@example.com' ), 'The rate limit should be applied' );
+ $this->assertTrue( SignupRateLimiter::apply( 'shopper@example.com' ), 'The rate limit should be applied' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A repeated sign-up attempt should be rate limited' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A repeated sign-up attempt should be rate limited' );
}
/**
* @testdox Should treat e-mail addresses that differ only in case and whitespace as the same.
*/
public function test_email_is_normalized(): void {
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( ' SHOPPER@Example.com ' ), 'The e-mail address should be normalized before hashing' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( ' SHOPPER@Example.com ' ), 'The e-mail address should be normalized before hashing' );
}
/**
* @testdox Should rate limit another e-mail address coming from the same IP address.
*/
public function test_other_email_from_same_ip_is_rate_limited(): void {
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'other@example.com' ), 'Sign-ups should also be rate limited per IP address' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'Sign-ups should also be rate limited per IP address' );
}
/**
* @testdox Should not rate limit another e-mail address coming from another IP address.
*/
public function test_other_email_from_other_ip_is_not_rate_limited(): void {
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'A different client signing up with a different e-mail address should not be held back' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'A different client signing up with a different e-mail address should not be held back' );
}
/**
@@ -112,12 +103,12 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_email_limit_is_independent_of_the_client_limit(): void {
$this->set_delays( 0, 600 );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'The same e-mail address should be rate limited from any IP address' );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'With the per-IP limit off, another e-mail address should go through' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'The same e-mail address should be rate limited from any IP address' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'With the per-IP limit off, another e-mail address should go through' );
}
/**
@@ -126,13 +117,13 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_client_limit_is_independent_of_the_email_limit(): void {
$this->set_delays( 30, 0 );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'other@example.com' ), 'The same client should be rate limited whichever e-mail address it uses' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'The same client should be rate limited whichever e-mail address it uses' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'With the per-e-mail limit off, another client should go through' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'With the per-e-mail limit off, another client should go through' );
}
/**
@@ -143,8 +134,8 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$queries = $this->record_queries(
function () {
- $this->assertTrue( $this->sut->apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A zero delay should let every attempt through' );
+ $this->assertTrue( SignupRateLimiter::apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A zero delay should let every attempt through' );
}
);
@@ -159,9 +150,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_numeric_string_delays_are_applied(): void {
$this->set_delays( '90', '90' );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A numeric string delay should be coerced to an integer and applied' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A numeric string delay should be coerced to an integer and applied' );
}
/**
@@ -170,11 +161,11 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_forwarded_header_is_ignored_by_default(): void {
$_SERVER['HTTP_X_FORWARDED_FOR'] = '198.51.100.5';
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'A spoofable header should not be used to key the rate limit' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'A spoofable header should not be used to key the rate limit' );
}
/**
@@ -185,15 +176,15 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$this->set_options( array( 'proxy_support' => true ) );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertTrue( $this->sut->is_rate_limited( 'other@example.com' ), 'A store behind a proxy should key the rate limit on the forwarded address' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'A store behind a proxy should key the rate limit on the forwarded address' );
$_SERVER['HTTP_X_FORWARDED_FOR'] = '198.51.100.6';
- $this->assertFalse( $this->sut->is_rate_limited( 'guest@example.com' ), 'Another forwarded address should not be held back' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'guest@example.com' ), 'Another forwarded address should not be held back' );
}
/**
@@ -204,10 +195,10 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$this->set_options( array( 'proxy_support' => true ) );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'An unresolved IP address should not be rate limited' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'The per-e-mail limit should still apply' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'An unresolved IP address should not be rate limited' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'The per-e-mail limit should still apply' );
}
/**
@@ -218,8 +209,8 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$queries = $this->record_queries(
function () {
- $this->assertTrue( $this->sut->apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A disabled limiter should let every attempt through' );
+ $this->assertTrue( SignupRateLimiter::apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A disabled limiter should let every attempt through' );
}
);
@@ -234,9 +225,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_unrecognized_enabled_option_falls_back_to_the_default(): void {
$this->set_options( array( 'enabled' => 'ture' ) );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A value that spells no boolean should fall back to the default rather than switch the limiter off' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A value that spells no boolean should fall back to the default rather than switch the limiter off' );
}
/**
@@ -255,12 +246,12 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
2
);
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ) );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ) );
$this->assertSame( array(), $fired, 'The action should not fire for an attempt that goes through' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ) );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ) );
$this->assertCount( 1, $fired, 'The action should fire once for a refused attempt' );
$this->assertStringStartsWith( 'stock_notifications_signup_email_', $fired[0][0], 'The action should receive the ID of the limit that was hit' );
$this->assertSame( 'shopper@example.com', $fired[0][1], 'The action should receive the e-mail address' );
@@ -272,15 +263,15 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_logged_in_user_is_keyed_on_user_id_not_ip(): void {
wp_set_current_user( $this->factory->user->create() );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
$_SERVER['REMOTE_ADDR'] = '192.0.2.20';
- $this->assertTrue( $this->sut->is_rate_limited( 'other@example.com' ), 'The same logged-in user should be rate limited even from another IP address' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'The same logged-in user should be rate limited even from another IP address' );
wp_set_current_user( 0 );
- $this->assertFalse( $this->sut->is_rate_limited( 'guest@example.com' ), 'A guest on that IP address should not be held back by the logged-in user limit' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'guest@example.com' ), 'A guest on that IP address should not be held back by the logged-in user limit' );
}
/**
@@ -289,11 +280,11 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_other_logged_in_user_from_same_ip_is_not_rate_limited(): void {
wp_set_current_user( $this->factory->user->create() );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
wp_set_current_user( $this->factory->user->create() );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'A different logged-in user on the same IP address should not be held back' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'A different logged-in user on the same IP address should not be held back' );
}
/**
@@ -306,9 +297,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
wp_set_current_user( $this->factory->user->create() );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'other@example.com' ), 'A logged-in user should be rate limited regardless of whether the IP address resolves' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'A logged-in user should be rate limited regardless of whether the IP address resolves' );
}
/**
@@ -329,7 +320,7 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
add_filter( 'query', $filter );
try {
- $applied = $this->sut->apply( 'shopper@example.com' );
+ $applied = SignupRateLimiter::apply( 'shopper@example.com' );
} finally {
$wpdb->suppress_errors( $suppress );
}
@@ -337,8 +328,8 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
remove_filter( 'query', $filter );
$this->assertFalse( $applied, 'A sign-up attempt whose limits cannot be stored should not be let through' );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'The per-IP limit should have been rolled back' );
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'The limit that could not be stored should have been cleared' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'The per-IP limit should have been rolled back' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'The limit that could not be stored should have been cleared' );
}
/**
@@ -347,9 +338,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_non_array_options_fall_back_to_defaults(): void {
$this->set_options( 'nope' );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'Non-array options should fall back to the defaults, which still rate limit' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'Non-array options should fall back to the defaults, which still rate limit' );
}
/**
@@ -360,8 +351,8 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$queries = $this->record_queries(
function () {
- $this->assertTrue( $this->sut->apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
- $this->assertFalse( $this->sut->is_rate_limited( 'shopper@example.com' ), 'A negative delay should let every attempt through' );
+ $this->assertTrue( SignupRateLimiter::apply( 'shopper@example.com' ), 'A disabled limiter should report success' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'A negative delay should let every attempt through' );
}
);
@@ -376,9 +367,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_non_numeric_delays_fall_back_to_defaults(): void {
$this->set_delays( false, array( 30 ) );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'Non-numeric delays should fall back to the defaults, which still rate limit' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'Non-numeric delays should fall back to the defaults, which still rate limit' );
}
/**
@@ -387,7 +378,7 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
public function test_object_delay_does_not_fatal(): void {
$this->set_delays( new \stdClass(), new \stdClass() );
- $this->assertTrue( $this->sut->apply( 'shopper@example.com' ), 'An object delay should not cause a fatal error' );
+ $this->assertTrue( SignupRateLimiter::apply( 'shopper@example.com' ), 'An object delay should not cause a fatal error' );
}
/**
@@ -399,9 +390,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$this->set_delays( 30, 0 );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertFalse( $this->sut->is_rate_limited( 'other@example.com' ), 'With no resolvable client address, the per-client limit should not apply' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'other@example.com' ), 'With no resolvable client address, the per-client limit should not apply' );
}
/**
@@ -413,9 +404,9 @@ class SignupRateLimiterTests extends WC_Unit_Test_Case {
$this->set_delays( 30, 180 );
- $this->sut->apply( 'shopper@example.com' );
+ SignupRateLimiter::apply( 'shopper@example.com' );
- $this->assertTrue( $this->sut->is_rate_limited( 'shopper@example.com' ), 'The per-e-mail limit should still apply when the client address cannot be resolved' );
+ $this->assertTrue( SignupRateLimiter::is_rate_limited( 'shopper@example.com' ), 'The per-e-mail limit should still apply when the client address cannot be resolved' );
}
/**
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupServiceTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupServiceTests.php
index 4b7ec32d5a4..4036810ba8a 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupServiceTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Frontend/SignupServiceTests.php
@@ -63,7 +63,7 @@ class SignupServiceTests extends \WC_Unit_Test_Case {
$notification_management_service->init( $this->email_manager );
$this->sut = new SignupService();
- $this->sut->init( $eligibility_service, $notification_management_service, $this->email_manager, new SignupRateLimiter() );
+ $this->sut->init( $eligibility_service, $notification_management_service, $this->email_manager );
}
/**
@@ -412,26 +412,35 @@ class SignupServiceTests extends \WC_Unit_Test_Case {
/**
* @testdox Should let the signup through when the rate limit window cannot be claimed.
*/
- public function test_signup_proceeds_when_the_rate_limit_cannot_be_claimed() {
- $rate_limiter = $this->createMock( SignupRateLimiter::class );
- $rate_limiter->method( 'is_rate_limited' )->willReturn( false );
- $rate_limiter->method( 'apply' )->willReturn( false );
+ public function test_signup_proceeds_when_the_rate_limit_cannot_be_claimed(): void {
+ global $wpdb;
- $eligibility_service = new EligibilityService();
- $eligibility_service->init( new StockManagementHelper() );
+ $product = $this->create_out_of_stock_product();
- $notification_management_service = new NotificationManagementService();
- $notification_management_service->init( $this->email_manager );
+ // Break the rate limit write the same way SignupRateLimiterTests does, so apply() fails
+ // for a reason the shopper cannot fix and signup() has to decide whether to fail open.
+ $suppress = $wpdb->suppress_errors( true );
+ $filter = static function ( $query ) {
+ if ( false !== strpos( $query, 'stock_notifications_signup_email_' ) ) {
+ return 'SELECT 1 FROM a_table_that_does_not_exist';
+ }
- $sut = new SignupService();
- $sut->init( $eligibility_service, $notification_management_service, $this->email_manager, $rate_limiter );
+ return $query;
+ };
- $product = $this->create_out_of_stock_product();
- $result = $sut->signup( $product->get_id(), 0, 'guest@example.com' );
+ add_filter( 'query', $filter );
+
+ try {
+ $result = $this->sut->signup( $product->get_id(), 0, 'guest@example.com' );
+ } finally {
+ remove_filter( 'query', $filter );
+ $wpdb->suppress_errors( $suppress );
+ }
$this->assertNotWPError( $result, 'A signup should not fail because the rate limit window could not be claimed' );
$this->assertEquals( SignupService::SIGNUP_SUCCESS, $result->get_code(), 'The signup should report success' );
- $this->assertInstanceOf( Notification::class, $sut->is_already_signed_up( $product->get_id(), 0, 'guest@example.com' ), 'The notification should have been created' );
+ $this->assertInstanceOf( Notification::class, $this->sut->is_already_signed_up( $product->get_id(), 0, 'guest@example.com' ), 'The notification should have been created' );
+ $this->assertFalse( SignupRateLimiter::is_rate_limited( 'guest@example.com' ), 'No partial rate limit window should be left behind' );
}
/**
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/StockNotificationsTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/StockNotificationsTests.php
index 23fc3562725..fa0623680f1 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/StockNotificationsTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/StockNotificationsTests.php
@@ -3,8 +3,10 @@
declare( strict_types = 1 );
namespace Automattic\WooCommerce\Tests\Internal\StockNotifications;
+use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Internal\Features\FeaturesController;
use Automattic\WooCommerce\Internal\StockNotifications\DataRetentionController;
+use Automattic\WooCommerce\Internal\StockNotifications\NotificationQuery;
use Automattic\WooCommerce\Internal\StockNotifications\StockNotifications;
use WC_Admin_Settings;
use WC_Settings_Products;
@@ -15,6 +17,13 @@ use WC_Settings_Products;
class StockNotificationsTests extends \WC_Unit_Test_Case {
use StockNotificationsFeatureTrait;
+ /**
+ * Database version to restore after a test that changes it.
+ *
+ * @var string|null
+ */
+ private ?string $original_db_version = null;
+
/**
* Set up before each test.
*/
@@ -27,6 +36,11 @@ class StockNotificationsTests extends \WC_Unit_Test_Case {
* Clean up after tests.
*/
public function tearDown(): void {
+ Constants::clear_single_constant( 'WOOCOMMERCE_BIS_ALPHA_ENABLED' );
+ if ( null !== $this->original_db_version ) {
+ update_option( 'woocommerce_db_version', $this->original_db_version );
+ $this->original_db_version = null;
+ }
wc_get_container()->get( DataRetentionController::class )->clear_daily_task();
delete_option( 'woocommerce_customer_stock_notifications_unverified_deletions_days_threshold' );
delete_option( 'woocommerce_queue_flush_rewrite_rules' );
@@ -152,6 +166,22 @@ class StockNotificationsTests extends \WC_Unit_Test_Case {
new \WC_Data_Store( 'stock_notification' );
}
+ /**
+ * @testdox NotificationQuery returns empty results instead of throwing while the feature is disabled.
+ */
+ public function test_notification_query_fails_soft_when_the_feature_is_disabled(): void {
+ $controller = wc_get_container()->get( StockNotifications::class );
+
+ remove_filter( 'woocommerce_data_stores', array( $controller, 'register_data_stores' ) );
+ update_option( StockNotifications::ENABLE_OPTION_NAME, 'no' );
+
+ $this->assertSame( array(), NotificationQuery::get_notifications( array() ) );
+ $this->assertSame( 0, NotificationQuery::count_notifications( array() ) );
+ $this->assertFalse( NotificationQuery::product_has_active_notifications( array( 1 ) ) );
+ $this->assertFalse( NotificationQuery::notification_exists_by_email( 1, 'shopper@example.com' ) );
+ $this->assertFalse( NotificationQuery::notification_exists_by_user_id( 1, 1 ) );
+ }
+
/**
* @testdox register_data_stores re-checks the option, so a hooked callback stays inert once the feature is turned off.
*/
@@ -166,6 +196,105 @@ class StockNotificationsTests extends \WC_Unit_Test_Case {
$this->assertSame( $stores, $controller->register_data_stores( $stores ) );
}
+ /**
+ * @testdox The alpha constant keeps the data store registered while the option is still unset.
+ */
+ public function test_register_data_stores_falls_back_to_the_alpha_constant(): void {
+ $controller = wc_get_container()->get( StockNotifications::class );
+
+ delete_option( StockNotifications::ENABLE_OPTION_NAME );
+ Constants::set_constant( 'WOOCOMMERCE_BIS_ALPHA_ENABLED', true );
+ $this->set_db_version( '11.1.0' );
+
+ $this->assertArrayHasKey( 'stock_notification', $controller->register_data_stores( array() ) );
+ }
+
+ /**
+ * @testdox The alpha constant is ignored once the database has been migrated to 11.2.0.
+ */
+ public function test_alpha_constant_is_ignored_once_the_database_is_migrated(): void {
+ $controller = wc_get_container()->get( StockNotifications::class );
+
+ update_option( StockNotifications::ENABLE_OPTION_NAME, 'no' );
+ Constants::set_constant( 'WOOCOMMERCE_BIS_ALPHA_ENABLED', true );
+ $this->set_db_version( '11.2.0' );
+
+ $this->assertArrayNotHasKey( 'stock_notification', $controller->register_data_stores( array() ) );
+
+ wc_get_container()->reset_all_resolved();
+ wc_get_container()->get( StockNotifications::class )->maybe_init_services();
+
+ $this->assertArrayNotHasKey( 'stock-notifications', wc_get_account_menu_items() );
+ }
+
+ /**
+ * @testdox maybe_init_services() wires the services for alpha sites until the database is migrated.
+ */
+ public function test_maybe_init_services_honors_the_alpha_constant_until_the_database_is_migrated(): void {
+ update_option( StockNotifications::ENABLE_OPTION_NAME, 'no' );
+ Constants::set_constant( 'WOOCOMMERCE_BIS_ALPHA_ENABLED', true );
+ $this->set_db_version( '11.1.0' );
+
+ wc_get_container()->reset_all_resolved();
+ wc_get_container()->get( StockNotifications::class )->maybe_init_services();
+
+ $this->assertArrayHasKey( 'stock-notifications', wc_get_account_menu_items() );
+ }
+
+ /**
+ * Set the stored database version for the current test, remembering the original.
+ *
+ * @param string $version Database version to store.
+ */
+ private function set_db_version( string $version ): void {
+ if ( null === $this->original_db_version ) {
+ $this->original_db_version = (string) get_option( 'woocommerce_db_version', '' );
+ }
+ update_option( 'woocommerce_db_version', $version );
+ }
+
+ /**
+ * @testdox init_hooks() is deprecated and forwards to maybe_init_services().
+ */
+ public function test_init_hooks_is_deprecated_and_forwards_to_maybe_init_services(): void {
+ $this->setExpectedDeprecated( StockNotifications::class . '::init_hooks' );
+
+ wc_get_container()->reset_all_resolved();
+ wc_get_container()->get( StockNotifications::class )->init_hooks();
+
+ $this->assertArrayHasKey( 'stock-notifications', wc_get_account_menu_items() );
+ }
+
+ /**
+ * @testdox init_hooks() defers to init when called before it, instead of building the feature definitions early.
+ */
+ public function test_init_hooks_defers_to_init_when_called_early(): void {
+ global $wp_actions;
+
+ $this->setExpectedDeprecated( StockNotifications::class . '::init_hooks' );
+
+ $controller = wc_get_container()->get( StockNotifications::class );
+ $init_count = $wp_actions['init'] ?? 0;
+ $was_hooked = has_action( 'init', array( $controller, 'maybe_init_services' ) );
+
+ remove_filter( 'woocommerce_data_stores', array( $controller, 'register_data_stores' ) );
+ remove_action( 'init', array( $controller, 'maybe_init_services' ), 1 );
+ // Pretend init has not fired yet; restored in the finally block.
+ unset( $wp_actions['init'] ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+
+ try {
+ $controller->init_hooks();
+
+ $this->assertSame( 1, has_action( 'init', array( $controller, 'maybe_init_services' ) ), 'maybe_init_services() should be queued on init at priority 1' );
+ $this->assertFalse( has_filter( 'woocommerce_data_stores', array( $controller, 'register_data_stores' ) ), 'Nothing should be wired up before init' );
+ } finally {
+ $wp_actions['init'] = $init_count; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ if ( false === $was_hooked ) {
+ remove_action( 'init', array( $controller, 'maybe_init_services' ), 1 );
+ }
+ }
+ }
+
/**
* @testdox The stock notification data store becomes available once the feature is enabled.
*/