From 34380756c8ccef7c497d281db574b755deae195d Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:05:34 +0530 Subject: [PATCH 01/11] Fix feed generator robustness at scale Addresses three critical issues causing runaway scheduling and database bloat: 1. Cursor advancement before processing: Modified get_items_for_batch() to store the last batch ID in a pending property, only committing to persistent storage after successful batch processing in handle_batch_action(). This prevents timeouts from causing the cursor to advance before the batch is processed, which would cause retries to fetch the wrong batch and skip products. 2. Duplicate timeout retries: Added deduplication check in handle_unexpected_shutdown() using as_has_scheduled_action() before calling schedule_immediate(). This prevents overlapping retries that fetch different work than originally intended. 3. Missing circuit breaker: Added MAX_BATCHES_PER_CYCLE constant (1000) and batch counter to prevent runaway scheduling. The generator now aborts with a warning if the maximum batch limit is reached during a generation cycle. These fixes prevent the issues described in customer feedback where timeout errors create fan-out of duplicate batches, leading to excessive Action Scheduler entries and database performance issues on large catalogs. Fixes: PIN4WOO-70 Co-Authored-By: Claude Sonnet 4.5 --- src/FeedGenerator.php | 79 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/src/FeedGenerator.php b/src/FeedGenerator.php index 65044af5..fde98579 100644 --- a/src/FeedGenerator.php +++ b/src/FeedGenerator.php @@ -43,6 +43,12 @@ class FeedGenerator extends AbstractChainedJob { */ const MAX_RETRIES_PER_BATCH = 2; + /** + * The max number of batches to process in a single generation cycle. + * Circuit breaker to prevent runaway scheduling and database bloat. + */ + const MAX_BATCHES_PER_CYCLE = 1000; + public const DEFAULT_PRODUCT_BATCH_SIZE = 100; /** @@ -67,6 +73,14 @@ class FeedGenerator extends AbstractChainedJob { */ private $buffers = array(); + /** + * Pending last batch ID to be committed after successful processing. + * Prevents cursor advancement before batch processing completes. + * + * @var int|null $pending_last_batch_id + */ + private $pending_last_batch_id = null; + /** * FeedGenerator initialization. * @@ -188,6 +202,21 @@ public function handle_unexpected_shutdown( int $action_id, ?array $error ) { ) ); + // Check if an action with the same hook and args is already scheduled to prevent duplicate retries. + if ( as_has_scheduled_action( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ) ) { + self::log( + sprintf( + // Translators: Action Scheduler hook name. + __( + 'Feed Generator `%s` Action retry already scheduled. Skipping duplicate.', + 'pinterest-for-woocommerce' + ), + $hook + ) + ); + return; + } + // Register retry attempt. $this->action_scheduler->schedule_immediate( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ); } @@ -267,6 +296,9 @@ protected function handle_start() { ) ); $this->feed_file_operations->prepare_temporary_files(); + + // Reset circuit breaker batch counter. + Pinterest_For_Woocommerce::save_data( 'feed_batch_count', 0 ); } catch ( Throwable $th ) { $this->handle_error( $th, $this->get_action_full_name( self::CHAIN_START ) ); throw $th; @@ -288,9 +320,15 @@ public function handle_batch_action( int $batch_number, array $args ) { /* * Action has finished successfully. + * - Commit the cursor position (only after successful processing). * - Reset number of products per batch. * - Reset action retries counter. */ + if ( null !== $this->pending_last_batch_id ) { + $this->set_last_batch_id( $this->pending_last_batch_id ); + $this->pending_last_batch_id = null; + } + Pinterest_For_Woocommerce::remove_data( 'feed_product_batch_size' ); Pinterest_For_Woocommerce::remove_data( 'feed_product_batch_attempt' ); } @@ -321,6 +359,9 @@ protected function handle_end() { } self::log( __( 'Feed generated successfully.', 'pinterest-for-woocommerce' ) ); + // Clean up circuit breaker counter. + Pinterest_For_Woocommerce::remove_data( 'feed_batch_count' ); + // Check if feed is dirty and reschedule in necessary. if ( $this->feed_is_dirty() ) { $this->mark_feed_clean(); @@ -342,6 +383,26 @@ protected function handle_end() { * @throws Exception On error. The failure will be logged by Action Scheduler and the job chain will stop. */ protected function get_items_for_batch( int $batch_number, array $args ): array { + // Circuit breaker: Check if max batches per cycle has been reached. + $batch_count = Pinterest_For_Woocommerce::get_data( 'feed_batch_count' ) ?? 0; + if ( $batch_count >= self::MAX_BATCHES_PER_CYCLE ) { + self::log( + sprintf( + // Translators: Maximum number of batches allowed. + __( + 'Feed Generator circuit breaker triggered. Maximum batch limit of %d reached. Completing generation.', + 'pinterest-for-woocommerce' + ), + self::MAX_BATCHES_PER_CYCLE + ), + \WC_Log_Levels::WARNING + ); + return array(); + } + + // Increment batch counter. + Pinterest_For_Woocommerce::save_data( 'feed_batch_count', $batch_count + 1 ); + global $wpdb; $product_ids = $wpdb->get_col( @@ -365,8 +426,10 @@ protected function get_items_for_batch( int $batch_number, array $args ): array ); $product_ids = array_map( 'intval', $product_ids ); - // We save the last product's id from the current batch to start from it next time when fetching the next batch. - $this->set_last_batch_id( $product_ids[ count( $product_ids ) - 1 ] ?? 0 ); + // Store the last product ID temporarily. It will be committed to persistent storage + // only after the batch is successfully processed (in handle_batch_action). + // This prevents timeouts from causing cursor advancement before processing completes. + $this->pending_last_batch_id = $product_ids[ count( $product_ids ) - 1 ] ?? 0; return $product_ids; } @@ -440,7 +503,7 @@ public function get_feed_products( array $ids ) { // Do not sync out of stock products which do not support backorders if woocommerce_hide_out_of_stock_items is set. if ( 'yes' === get_option( 'woocommerce_hide_out_of_stock_items' ) ) { - $products_query_args['stock_status'] = [ 'instock', 'onbackorder' ]; + $products_query_args['stock_status'] = array( 'instock', 'onbackorder' ); } return wc_get_products( $products_query_args ); @@ -502,6 +565,8 @@ public function feed_is_dirty(): bool { } /** + * Handle feed generation error by updating status and scheduling retry. + * * @param Throwable $th - An exception that was thrown. * @param string $hook_name - The name of the hook that was being executed when the exception was thrown. * @@ -760,20 +825,20 @@ protected function is_failure_rate_above_threshold( string $hook, ?array $args = * Threshold of failed actions. * phpcs:disable WooCommerce.Commenting.CommentHooks.MissingSinceComment */ - $threshold = apply_filters( 'pinterest_for_woocommerce_action_failure_threshold', 3 ); + $threshold = apply_filters( 'pinterest_for_woocommerce_action_failure_threshold', 3 ); /** * Time period of failed actions. * phpcs:disable WooCommerce.Commenting.CommentHooks.MissingSinceComment */ - $time_period = apply_filters( 'pinterest_for_woocommerce_action_failure_time_period', 30 * MINUTE_IN_SECONDS ); + $time_period = apply_filters( 'pinterest_for_woocommerce_action_failure_time_period', 30 * MINUTE_IN_SECONDS ); $failed_actions = $this->action_scheduler->search( - [ + array( 'hook' => $hook, 'args' => $args, 'status' => ActionSchedulerInterface::STATUS_FAILED, 'date' => gmdate( 'U' ) - $time_period, 'date_compare' => '>', - ], + ), 'ids' ); From 1665944091e2cf86c8f21c36ccf2adaeaa359cca Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:33:53 +0530 Subject: [PATCH 02/11] Fix unit tests for deduplication logic Update test_handle_unexpected_shutdown_does_throttle_product_number_when_rescheduling_the_action to account for new deduplication behavior: - First timeout reschedules the action (expected behavior) - Second timeout skips rescheduling due to deduplication (action already exists) - Updated expectations to reflect that schedule_immediate is called once, not twice Add new test test_handle_unexpected_shutdown_skips_rescheduling_if_action_already_scheduled to specifically verify deduplication prevents overlapping retries. Also fix phpcs violations in test file. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 86 ++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 20 deletions(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index bee4a317..4e29b097 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -135,7 +135,8 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe 'pinterest-for-woocommerce' ); - $this->action_scheduler->expects( $this->exactly( 2 ) ) + // The first call should reschedule (no duplicate exists yet). + $this->action_scheduler->expects( $this->once() ) ->method( 'schedule_immediate' ) ->with( 'pinterest/jobs/generate_feed/chain_batch', @@ -154,10 +155,55 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); $this->assertEquals( 2, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ); + // Second call should be skipped due to deduplication (action already scheduled). $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); - $this->assertEquals( 17, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); - $this->assertEquals( 3, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ); + // Batch size and attempt should remain the same since rescheduling was skipped. + $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); + $this->assertEquals( 2, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ); + } + + /** + * Tests that deduplication prevents rescheduling when action is already scheduled. + * + * @return void + */ + public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_already_scheduled() { + $action_id = as_schedule_single_action( + gmdate( 'U' ) - 1, + 'pinterest/jobs/generate_feed/chain_batch', + array( 1, array() ), + 'pinterest-for-woocommerce' + ); + + // First call schedules the action. + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( + 'pinterest/jobs/generate_feed/chain_batch', + array( 1, array() ), + 'pinterest-for-woocommerce' + ); + $this->action_scheduler->method( 'search' ) + ->willReturn( array() ); + + $error = array( + 'type' => E_ERROR, + 'message' => 'Maximum execution time', + ); + + // First timeout: should reschedule. + $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); + + // Verify batch size was decreased. + $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); + + // Second timeout with same action: should skip due to deduplication. + // The action is already scheduled from the first call. + $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); + + // Batch size should not change further since rescheduling was skipped. + $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); } /** @@ -219,10 +265,10 @@ public function test_handle_failed_execution_does_nothing_if_a_different_action( $this->feed_generator->handle_failed_execution( $action_id, new Exception( 'Some error msg.' ), '' ); $pending_actions = as_get_scheduled_actions( - [ + array( 'hook' => 'pinterest/jobs/generate_feed/chain_batch_foo', 'status' => 'pending', - ] + ) ); $this->assertCount( 0, $pending_actions ); @@ -242,7 +288,7 @@ public function test_action_scheduler_failed_execution_hook_calls_handle_failed_ // Add a callback to throw an exception when the action is processed. $callback = function () { - throw new Exception('Action `pinterest/jobs/generate_feed/chain_batch` failed to complete.' ); + throw new Exception( 'Action `pinterest/jobs/generate_feed/chain_batch` failed to complete.' ); }; add_action( 'pinterest/jobs/generate_feed/chain_batch', $callback, 10, 2 ); @@ -278,7 +324,7 @@ public function test_action_scheduler_failed_execution_hook_calls_handle_failed_ $this->assertCount( 1, $future_actions ); /** @var ActionScheduler_Action $action */ - $action = current( $future_actions ); + $action = current( $future_actions ); $delay_in_hours = (int) ceil( ( $action->get_schedule()->get_date()->getTimestamp() - time() ) / 3600 ); $this->assertEquals( 1, $delay_in_hours ); } @@ -339,7 +385,7 @@ public function test_feed_generator_start_fails_and_sets_wall_time_to_negative() try { $this->feed_generator->handle_start_action( array() ); } catch ( Exception $e ) { - $feed_generation_status = ProductFeedStatus::get()[ 'status' ]; + $feed_generation_status = ProductFeedStatus::get()['status']; $feed_generation_wall_time = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_WALL_TIME ]; $feed_generation_product_count = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_RECENT_PRODUCT_COUNT ]; @@ -367,7 +413,7 @@ public function test_feed_generator_end_sets_time_it_took_to_generate_the_feed() public function test_feed_generator_end_sets_product_count_into_persistent_state_property() { ProductFeedStatus::set( array( - 'product_count' => 13, + 'product_count' => 13, ProductFeedStatus::PROP_FEED_GENERATION_RECENT_PRODUCT_COUNT => 1, ) ); @@ -396,7 +442,7 @@ public function test_feed_generator_end_fails_and_sets_wall_time_to_negative() { try { $this->feed_generator->handle_end_action( array() ); } catch ( Exception $e ) { - $feed_generation_status = ProductFeedStatus::get()[ 'status' ]; + $feed_generation_status = ProductFeedStatus::get()['status']; $feed_generation_wall_time = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_WALL_TIME ]; $feed_generation_product_count = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_RECENT_PRODUCT_COUNT ]; @@ -416,7 +462,7 @@ public function test_while_feed_generator_is_in_progress_previous_wall_time_and_ $this->feed_generator->handle_start_action( array() ); - $status = ProductFeedStatus::get()[ 'status' ]; + $status = ProductFeedStatus::get()['status']; $wall_time = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_WALL_TIME ]; $product_count = ProductFeedStatus::get()[ ProductFeedStatus::PROP_FEED_GENERATION_RECENT_PRODUCT_COUNT ]; @@ -437,7 +483,7 @@ public function test_handle_batch_action_ends_queue_when_no_more_items() { $this->feed_generator->handle_batch_action( 1, array() ); - $this->assertEquals( 0, (int) \Pinterest_For_Woocommerce::get_data( 'feed_generation_retries' )); + $this->assertEquals( 0, (int) \Pinterest_For_Woocommerce::get_data( 'feed_generation_retries' ) ); } public function test_handle_batch_action_queues_next_batch_when_there_are_items_to_process() { @@ -454,7 +500,7 @@ public function test_handle_batch_action_queues_next_batch_when_there_are_items_ $this->feed_generator->handle_batch_action( 1, array() ); - $this->assertEquals( 0, (int) \Pinterest_For_Woocommerce::get_data( 'feed_generation_retries' )); + $this->assertEquals( 0, (int) \Pinterest_For_Woocommerce::get_data( 'feed_generation_retries' ) ); } /** @@ -466,26 +512,26 @@ public function test_get_feed_products_return_backorder_enabled_products() { update_option( 'woocommerce_hide_out_of_stock_items', 'yes' ); $product_a = \WC_Helper_Product::create_simple_product( true, - [ + array( 'name' => 'In stock product', - ] + ) ); $product_b = \WC_Helper_Product::create_simple_product( true, - [ + array( 'name' => 'Product on backorder', 'stock_status' => 'onbackorder', - ] + ) ); $product_c = \WC_Helper_Product::create_simple_product( true, - [ + array( 'name' => 'Out of stock product', 'stock_status' => 'outofstock', - ] + ) ); - $ids = [ $product_a->get_id(), $product_b->get_id(), $product_c->get_id() ]; + $ids = array( $product_a->get_id(), $product_b->get_id(), $product_c->get_id() ); $products = $this->feed_generator->get_feed_products( $ids ); From d070c596e5781fb12d48dc20a2f6e15257b58803 Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:38:10 +0530 Subject: [PATCH 03/11] Fix test deduplication checks by scheduling actual actions The deduplication logic checks for existing actions using as_has_scheduled_action(), which queries the real Action Scheduler database. Mocked schedule_immediate calls don't register in that database, so we need to actually schedule duplicate actions using as_schedule_single_action() to properly test the deduplication behavior. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 4e29b097..5ec920ea 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -155,6 +155,15 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); $this->assertEquals( 2, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ); + // Schedule a duplicate action to simulate what the first call would have done. + // This allows the deduplication logic to find it on the second call. + as_schedule_single_action( + time(), + 'pinterest/jobs/generate_feed/chain_batch', + array( 1, array() ), + 'pinterest-for-woocommerce' + ); + // Second call should be skipped due to deduplication (action already scheduled). $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); @@ -198,6 +207,15 @@ public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_alr // Verify batch size was decreased. $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); + // Schedule a duplicate action to simulate what the first call would have done. + // This allows the deduplication logic to find it on the second call. + as_schedule_single_action( + time(), + 'pinterest/jobs/generate_feed/chain_batch', + array( 1, array() ), + 'pinterest-for-woocommerce' + ); + // Second timeout with same action: should skip due to deduplication. // The action is already scheduled from the first call. $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); From 2f67c57b47ee01b9975a23880f1145050d3a9dd1 Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:40:39 +0530 Subject: [PATCH 04/11] Schedule duplicate actions in future to maintain pending status Actions scheduled with time() are immediately eligible for processing, which may cause them to not be found by as_has_scheduled_action(). Schedule them 10 seconds in the future to ensure they remain in pending status for deduplication checks. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 5ec920ea..646ed79f 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -157,8 +157,9 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe // Schedule a duplicate action to simulate what the first call would have done. // This allows the deduplication logic to find it on the second call. + // Schedule it in the future to keep it in "pending" status. as_schedule_single_action( - time(), + time() + 10, 'pinterest/jobs/generate_feed/chain_batch', array( 1, array() ), 'pinterest-for-woocommerce' @@ -209,8 +210,9 @@ public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_alr // Schedule a duplicate action to simulate what the first call would have done. // This allows the deduplication logic to find it on the second call. + // Schedule it in the future to keep it in "pending" status. as_schedule_single_action( - time(), + time() + 10, 'pinterest/jobs/generate_feed/chain_batch', array( 1, array() ), 'pinterest-for-woocommerce' From e39e390c7fbd1d6ac263beefb05cb6de1302dccb Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:43:56 +0530 Subject: [PATCH 05/11] Use real Action Scheduler in deduplication tests Replace mocked schedule_immediate with real Action Scheduler calls to properly test deduplication logic. The issue was that mocked calls don't register in the Action Scheduler database, so as_has_scheduled_action() couldn't find them. Now using anonymous class implementing ActionSchedulerInterface that wraps real as_schedule_single_action() calls, allowing deduplication checks to work correctly. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 101 +++++++++++++++---------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 646ed79f..e9bb8e07 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -128,6 +128,13 @@ public function test_handle_unexpected_shutdown_does_nothing_if_timeout_but_not_ * @return void */ public function test_handle_unexpected_shutdown_does_throttle_product_number_when_rescheduling_the_action() { + // Use real FeedGenerator without mocks for this integration test. + $real_feed_generator = new FeedGenerator( + $this->createMock( ActionSchedulerInterface::class ), + $this->feed_file_operations, + $this->local_feed_configs + ); + $action_id = as_schedule_single_action( gmdate( 'U' ) - 1, 'pinterest/jobs/generate_feed/chain_batch', @@ -135,38 +142,34 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe 'pinterest-for-woocommerce' ); - // The first call should reschedule (no duplicate exists yet). - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( - 'pinterest/jobs/generate_feed/chain_batch', - array( 1, array() ), - 'pinterest-for-woocommerce' - ); - $this->action_scheduler->method( 'search' ) - ->willReturn( array() ); - $error = array( 'type' => E_ERROR, 'message' => 'Maximum execution time', ); - $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); + // Mock search to return empty array (no failure threshold met). + $real_feed_generator = new FeedGenerator( + new class() implements ActionSchedulerInterface { + public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { + return as_schedule_single_action( time(), $hook, $args, $group ); + } + public function search( array $args = array(), string $return_format = OBJECT ): array { + return array(); + } + public function cancel( int $action_id ): void {} + public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} + }, + $this->feed_file_operations, + $this->local_feed_configs + ); + + // First call: should reschedule and throttle. + $real_feed_generator->handle_unexpected_shutdown( $action_id, $error ); $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); $this->assertEquals( 2, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ); - // Schedule a duplicate action to simulate what the first call would have done. - // This allows the deduplication logic to find it on the second call. - // Schedule it in the future to keep it in "pending" status. - as_schedule_single_action( - time() + 10, - 'pinterest/jobs/generate_feed/chain_batch', - array( 1, array() ), - 'pinterest-for-woocommerce' - ); - - // Second call should be skipped due to deduplication (action already scheduled). - $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); + // Second call: should be skipped due to deduplication (action already scheduled from first call). + $real_feed_generator->handle_unexpected_shutdown( $action_id, $error ); // Batch size and attempt should remain the same since rescheduling was skipped. $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); @@ -179,6 +182,22 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe * @return void */ public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_already_scheduled() { + // Use real FeedGenerator that actually schedules actions. + $real_feed_generator = new FeedGenerator( + new class() implements ActionSchedulerInterface { + public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { + return as_schedule_single_action( time(), $hook, $args, $group ); + } + public function search( array $args = array(), string $return_format = OBJECT ): array { + return array(); + } + public function cancel( int $action_id ): void {} + public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} + }, + $this->feed_file_operations, + $this->local_feed_configs + ); + $action_id = as_schedule_single_action( gmdate( 'U' ) - 1, 'pinterest/jobs/generate_feed/chain_batch', @@ -186,43 +205,19 @@ public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_alr 'pinterest-for-woocommerce' ); - // First call schedules the action. - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( - 'pinterest/jobs/generate_feed/chain_batch', - array( 1, array() ), - 'pinterest-for-woocommerce' - ); - $this->action_scheduler->method( 'search' ) - ->willReturn( array() ); - $error = array( 'type' => E_ERROR, 'message' => 'Maximum execution time', ); - // First timeout: should reschedule. - $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); - - // Verify batch size was decreased. + // First timeout: should reschedule and decrease batch size. + $real_feed_generator->handle_unexpected_shutdown( $action_id, $error ); $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); - // Schedule a duplicate action to simulate what the first call would have done. - // This allows the deduplication logic to find it on the second call. - // Schedule it in the future to keep it in "pending" status. - as_schedule_single_action( - time() + 10, - 'pinterest/jobs/generate_feed/chain_batch', - array( 1, array() ), - 'pinterest-for-woocommerce' - ); - - // Second timeout with same action: should skip due to deduplication. - // The action is already scheduled from the first call. - $this->feed_generator->handle_unexpected_shutdown( $action_id, $error ); + // Second timeout: deduplication should prevent rescheduling since action already exists. + $real_feed_generator->handle_unexpected_shutdown( $action_id, $error ); - // Batch size should not change further since rescheduling was skipped. + // Batch size should remain unchanged since deduplication skipped the second reschedule. $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); } From 941e3cc9aa749338417a4bb71161140f0fed4d6d Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:47:30 +0530 Subject: [PATCH 06/11] Replace anonymous class with named test helper class Anonymous classes may have compatibility issues with PHP 7.4. Created TestActionSchedulerProxy as a named class to ensure compatibility across all supported PHP versions (7.4-8.3). Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 48 +++++++++++++------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index e9bb8e07..ab870e0c 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -14,6 +14,23 @@ use Pinterest_For_Woocommerce; use WC_Helper_Product; +/** + * Test helper class that wraps real Action Scheduler functions. + */ +class TestActionSchedulerProxy implements ActionSchedulerInterface { + public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { + return as_schedule_single_action( time(), $hook, $args, $group ); + } + + public function search( array $args = array(), string $return_format = OBJECT ): array { + return array(); + } + + public function cancel( int $action_id ): void {} + + public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} +} + class FeedGeneratorTest extends \WP_UnitTestCase { /** @var ActionSchedulerInterface */ @@ -128,9 +145,9 @@ public function test_handle_unexpected_shutdown_does_nothing_if_timeout_but_not_ * @return void */ public function test_handle_unexpected_shutdown_does_throttle_product_number_when_rescheduling_the_action() { - // Use real FeedGenerator without mocks for this integration test. + // Use real FeedGenerator that schedules actual actions. $real_feed_generator = new FeedGenerator( - $this->createMock( ActionSchedulerInterface::class ), + new TestActionSchedulerProxy(), $this->feed_file_operations, $this->local_feed_configs ); @@ -147,22 +164,6 @@ public function test_handle_unexpected_shutdown_does_throttle_product_number_whe 'message' => 'Maximum execution time', ); - // Mock search to return empty array (no failure threshold met). - $real_feed_generator = new FeedGenerator( - new class() implements ActionSchedulerInterface { - public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { - return as_schedule_single_action( time(), $hook, $args, $group ); - } - public function search( array $args = array(), string $return_format = OBJECT ): array { - return array(); - } - public function cancel( int $action_id ): void {} - public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} - }, - $this->feed_file_operations, - $this->local_feed_configs - ); - // First call: should reschedule and throttle. $real_feed_generator->handle_unexpected_shutdown( $action_id, $error ); $this->assertEquals( 50, \Pinterest_For_Woocommerce::get_data( 'feed_product_batch_size' ) ); @@ -184,16 +185,7 @@ public function cancel_all( string $hook, array $args = array(), string $group = public function test_handle_unexpected_shutdown_skips_rescheduling_if_action_already_scheduled() { // Use real FeedGenerator that actually schedules actions. $real_feed_generator = new FeedGenerator( - new class() implements ActionSchedulerInterface { - public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { - return as_schedule_single_action( time(), $hook, $args, $group ); - } - public function search( array $args = array(), string $return_format = OBJECT ): array { - return array(); - } - public function cancel( int $action_id ): void {} - public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} - }, + new TestActionSchedulerProxy(), $this->feed_file_operations, $this->local_feed_configs ); From 454e6b08839c55ba5180844ec58b4c3f2ccbfe81 Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:50:33 +0530 Subject: [PATCH 07/11] Fix method signatures to match interface for PHP 7.4 compatibility Remove type hints from TestActionSchedulerProxy method signatures to match the ActionSchedulerInterface exactly. PHP 7.4 is stricter about signature compatibility when implementing interfaces. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index ab870e0c..8e8f3d30 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -18,17 +18,17 @@ * Test helper class that wraps real Action Scheduler functions. */ class TestActionSchedulerProxy implements ActionSchedulerInterface { - public function schedule_immediate( string $hook, array $args = array(), string $group = '' ): int { + public function schedule_immediate( string $hook, $args = array(), string $group = '' ) { return as_schedule_single_action( time(), $hook, $args, $group ); } - public function search( array $args = array(), string $return_format = OBJECT ): array { + public function search( $args = array(), $return_format = OBJECT ) { return array(); } - public function cancel( int $action_id ): void {} + public function cancel( $action_id ) {} - public function cancel_all( string $hook, array $args = array(), string $group = '' ): void {} + public function cancel_all( string $hook, $args = array(), string $group = '' ) {} } class FeedGeneratorTest extends \WP_UnitTestCase { From cea0552b55a2c07f5bbda1fe69131556f845c9a8 Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:54:10 +0530 Subject: [PATCH 08/11] Add docblocks to TestActionSchedulerProxy methods Add required function docblocks to satisfy WooCommerce coding standards. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 8e8f3d30..3beea3c2 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -18,16 +18,45 @@ * Test helper class that wraps real Action Scheduler functions. */ class TestActionSchedulerProxy implements ActionSchedulerInterface { + /** + * Schedule an action to run immediately. + * + * @param string $hook Action hook. + * @param mixed $args Action arguments. + * @param string $group Action group. + * @return int Action ID. + */ public function schedule_immediate( string $hook, $args = array(), string $group = '' ) { return as_schedule_single_action( time(), $hook, $args, $group ); } + /** + * Search for scheduled actions. + * + * @param mixed $args Search arguments. + * @param string $return_format Return format. + * @return array Empty array for testing. + */ public function search( $args = array(), $return_format = OBJECT ) { return array(); } + /** + * Cancel an action. + * + * @param mixed $action_id Action ID. + * @return void + */ public function cancel( $action_id ) {} + /** + * Cancel all actions matching criteria. + * + * @param string $hook Action hook. + * @param mixed $args Action arguments. + * @param string $group Action group. + * @return void + */ public function cancel_all( string $hook, $args = array(), string $group = '' ) {} } From 003c1bf2b48bc39ad570d0a39fd227c2a71d24f8 Mon Sep 17 00:00:00 2001 From: simplysaru Date: Tue, 31 Mar 2026 15:56:53 +0530 Subject: [PATCH 09/11] Add missing group parameter to search() method The ActionSchedulerInterface::search() method has 3 parameters including . Added the missing parameter to match the interface signature. Co-Authored-By: Claude Sonnet 4.5 --- tests/Unit/FeedGeneratorTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 3beea3c2..c0858846 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -35,9 +35,10 @@ public function schedule_immediate( string $hook, $args = array(), string $group * * @param mixed $args Search arguments. * @param string $return_format Return format. + * @param string $group Action group. * @return array Empty array for testing. */ - public function search( $args = array(), $return_format = OBJECT ) { + public function search( $args = array(), $return_format = OBJECT, string $group = '' ) { return array(); } From 164d3bf50a2f9762fdd79b335873bff4269647fe Mon Sep 17 00:00:00 2001 From: simplysaru Date: Wed, 1 Apr 2026 07:16:09 +0530 Subject: [PATCH 10/11] Address GitHub Copilot review feedback 1. Move deduplication check before throttling logic - Prevents unnecessary batch size adjustments when rescheduling is skipped - Only throttle when actually scheduling a retry 2. Reset pending_last_batch_id at start of handle_batch_action - Prevents stale cursor values from previous failed batches - Ensures deterministic state management across retries 3. Add test coverage for new behaviors - test_circuit_breaker_stops_processing_at_max_batches: Verifies MAX_BATCHES_PER_CYCLE limit - test_cursor_deferred_until_successful_batch_completion: Verifies cursor only advances after success Addresses Copilot review comments on PR #1166 Co-Authored-By: Claude Sonnet 4.5 --- src/FeedGenerator.php | 34 ++++++++++++----------- tests/Unit/FeedGeneratorTest.php | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/FeedGenerator.php b/src/FeedGenerator.php index fde98579..71207c07 100644 --- a/src/FeedGenerator.php +++ b/src/FeedGenerator.php @@ -184,6 +184,22 @@ public function handle_unexpected_shutdown( int $action_id, ?array $error ) { ) ); + // Check if an action with the same hook and args is already scheduled to prevent duplicate retries. + // Important: Check this BEFORE throttling to avoid unnecessary batch size adjustments. + if ( as_has_scheduled_action( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ) ) { + self::log( + sprintf( + // Translators: Action Scheduler hook name. + __( + 'Feed Generator `%s` Action retry already scheduled. Skipping duplicate.', + 'pinterest-for-woocommerce' + ), + $hook + ) + ); + return; + } + // Decrease the number of products to retry. $attempt = ( Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ?? 1 ) + 1; $limit = (int) ceil( $this->get_batch_size() / $attempt ); @@ -202,21 +218,6 @@ public function handle_unexpected_shutdown( int $action_id, ?array $error ) { ) ); - // Check if an action with the same hook and args is already scheduled to prevent duplicate retries. - if ( as_has_scheduled_action( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ) ) { - self::log( - sprintf( - // Translators: Action Scheduler hook name. - __( - 'Feed Generator `%s` Action retry already scheduled. Skipping duplicate.', - 'pinterest-for-woocommerce' - ), - $hook - ) - ); - return; - } - // Register retry attempt. $this->action_scheduler->schedule_immediate( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ); } @@ -316,6 +317,9 @@ protected function handle_start() { * @throws Throwable Related to issue possible when creating an empty feed temp file and populating the header. */ public function handle_batch_action( int $batch_number, array $args ) { + // Reset pending cursor to prevent stale values from previous failed batches. + $this->pending_last_batch_id = null; + parent::handle_batch_action( $batch_number, $args ); /* diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index c0858846..2db555b5 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -540,6 +540,52 @@ public function test_handle_batch_action_queues_next_batch_when_there_are_items_ $this->assertEquals( 0, (int) \Pinterest_For_Woocommerce::get_data( 'feed_generation_retries' ) ); } + /** + * Tests circuit breaker stops batch processing after reaching MAX_BATCHES_PER_CYCLE. + * + * @return void + */ + public function test_circuit_breaker_stops_processing_at_max_batches() { + // Set batch counter to just below the limit. + Pinterest_For_Woocommerce::save_data( 'feed_batch_count', FeedGenerator::MAX_BATCHES_PER_CYCLE - 1 ); + + // This should process normally (last allowed batch). + $items = $this->feed_generator->get_items_for_batch( 1, array() ); + $this->assertIsArray( $items ); + + // Next call should return empty array due to circuit breaker. + $items = $this->feed_generator->get_items_for_batch( 2, array() ); + $this->assertEmpty( $items, 'Circuit breaker should return empty array after max batches reached' ); + } + + /** + * Tests that cursor is only advanced after successful batch processing. + * + * @return void + */ + public function test_cursor_deferred_until_successful_batch_completion() { + // Create a product to fetch. + WC_Helper_Product::create_simple_product(); + + // Set initial cursor. + Pinterest_For_Woocommerce::save_data( 'feed_last_queued_item_id', 0 ); + + // Fetch items - this should store pending cursor but not commit it yet. + $items = $this->feed_generator->get_items_for_batch( 1, array() ); + $this->assertNotEmpty( $items ); + + // Cursor should still be at initial value (not advanced yet). + $cursor_before = Pinterest_For_Woocommerce::get_data( 'feed_last_queued_item_id' ); + $this->assertEquals( 0, $cursor_before, 'Cursor should not advance before handle_batch_action completes' ); + + // Complete batch processing. + $this->feed_generator->handle_batch_action( 1, array() ); + + // Now cursor should be advanced. + $cursor_after = Pinterest_For_Woocommerce::get_data( 'feed_last_queued_item_id' ); + $this->assertGreaterThan( $cursor_before, $cursor_after, 'Cursor should advance after successful batch completion' ); + } + /** * Tests get feed products method returns products in stock including products on backorder. * From 35513166e62fe0d0ecbcd4dd7222b7a393cf8276 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 02:14:47 +0000 Subject: [PATCH 11/11] Fix log ordering, protected method access, and AS cleanup in tests Agent-Logs-Url: https://github.com/woocommerce/pinterest-for-woocommerce/sessions/235573f9-062b-4dfb-837b-1897c7d42237 Co-authored-by: simplysaru <2734132+simplysaru@users.noreply.github.com> --- src/FeedGenerator.php | 22 +++++++++++----------- tests/Unit/FeedGeneratorTest.php | 31 ++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/FeedGenerator.php b/src/FeedGenerator.php index 71207c07..7e6439bb 100644 --- a/src/FeedGenerator.php +++ b/src/FeedGenerator.php @@ -173,17 +173,6 @@ public function handle_unexpected_shutdown( int $action_id, ?array $error ) { return; } - self::log( - sprintf( - // Translators: Action Scheduler hook name. - __( - 'Feed Generator `%s` Action timed out due to an unexpected shutdown. Rescheduling it.', - 'pinterest-for-woocommerce' - ), - $hook - ) - ); - // Check if an action with the same hook and args is already scheduled to prevent duplicate retries. // Important: Check this BEFORE throttling to avoid unnecessary batch size adjustments. if ( as_has_scheduled_action( $hook, $args, PINTEREST_FOR_WOOCOMMERCE_PREFIX ) ) { @@ -200,6 +189,17 @@ public function handle_unexpected_shutdown( int $action_id, ?array $error ) { return; } + self::log( + sprintf( + // Translators: Action Scheduler hook name. + __( + 'Feed Generator `%s` Action timed out due to an unexpected shutdown. Rescheduling it.', + 'pinterest-for-woocommerce' + ), + $hook + ) + ); + // Decrease the number of products to retry. $attempt = ( Pinterest_For_Woocommerce::get_data( 'feed_product_batch_attempt' ) ?? 1 ) + 1; $limit = (int) ceil( $this->get_batch_size() / $attempt ); diff --git a/tests/Unit/FeedGeneratorTest.php b/tests/Unit/FeedGeneratorTest.php index 2db555b5..72ee6459 100644 --- a/tests/Unit/FeedGeneratorTest.php +++ b/tests/Unit/FeedGeneratorTest.php @@ -89,6 +89,31 @@ public function setUp(): void { ProductFeedStatus::set( ProductFeedStatus::STATE_PROPS ); } + /** + * Clean up Action Scheduler entries after each test to prevent cross-test interference + * when the deduplication guard checks for pending scheduled actions. + * + * @return void + */ + public function tearDown(): void { + as_unschedule_all_actions( 'pinterest/jobs/generate_feed/chain_batch', null, 'pinterest-for-woocommerce' ); + parent::tearDown(); + } + + /** + * Helper to invoke a protected method on FeedGenerator via reflection. + * + * @param FeedGenerator $generator The FeedGenerator instance. + * @param string $method_name Name of the protected method. + * @param array $args Arguments to pass to the method. + * @return mixed Return value of the method. + */ + private function invoke_protected( FeedGenerator $generator, string $method_name, array $args = array() ) { + $reflection = new \ReflectionMethod( FeedGenerator::class, $method_name ); + $reflection->setAccessible( true ); + return $reflection->invokeArgs( $generator, $args ); + } + /** * Tests feed generator registers the action scheduler failed execution hook. * @@ -550,11 +575,11 @@ public function test_circuit_breaker_stops_processing_at_max_batches() { Pinterest_For_Woocommerce::save_data( 'feed_batch_count', FeedGenerator::MAX_BATCHES_PER_CYCLE - 1 ); // This should process normally (last allowed batch). - $items = $this->feed_generator->get_items_for_batch( 1, array() ); + $items = $this->invoke_protected( $this->feed_generator, 'get_items_for_batch', array( 1, array() ) ); $this->assertIsArray( $items ); // Next call should return empty array due to circuit breaker. - $items = $this->feed_generator->get_items_for_batch( 2, array() ); + $items = $this->invoke_protected( $this->feed_generator, 'get_items_for_batch', array( 2, array() ) ); $this->assertEmpty( $items, 'Circuit breaker should return empty array after max batches reached' ); } @@ -571,7 +596,7 @@ public function test_cursor_deferred_until_successful_batch_completion() { Pinterest_For_Woocommerce::save_data( 'feed_last_queued_item_id', 0 ); // Fetch items - this should store pending cursor but not commit it yet. - $items = $this->feed_generator->get_items_for_batch( 1, array() ); + $items = $this->invoke_protected( $this->feed_generator, 'get_items_for_batch', array( 1, array() ) ); $this->assertNotEmpty( $items ); // Cursor should still be at initial value (not advanced yet).