From 1e4c94e7f2cd94f13d9d43ae6aaf170be833c079 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:54:05 +0200 Subject: [PATCH 01/19] Refactor `MapiProductInputsService` to use `run_in_batches` for concurrent sub-requests, improving readability and batch handling logic. --- .../Services/MapiProductInputsService.php | 132 +++++++++++++----- 1 file changed, 94 insertions(+), 38 deletions(-) diff --git a/src/API/Google/Mapi/Services/MapiProductInputsService.php b/src/API/Google/Mapi/Services/MapiProductInputsService.php index f4daaa0bb5..11b75135a9 100644 --- a/src/API/Google/Mapi/Services/MapiProductInputsService.php +++ b/src/API/Google/Mapi/Services/MapiProductInputsService.php @@ -89,34 +89,21 @@ public function insert_many( array $inputs, int $concurrency = 10 ): array { $paths_by_index[ $index ] = $this->build_path( $data_source ); } - $client = $this->client; - - $requests = function () use ( $inputs, $client, $paths_by_index ) { - foreach ( $inputs as $index => $input ) { - yield $index => $client->request_async( 'POST', $paths_by_index[ $index ], $input->to_array() ); + return $this->run_in_batches( + $inputs, + $concurrency, + __METHOD__, + function ( int $index, ProductInput $input ) use ( $paths_by_index ): array { + return [ + 'method' => 'POST', + 'path' => $paths_by_index[ $index ], + 'body' => $input->to_array(), + ]; + }, + function ( array $body ): ProductInput { + return ProductInput::from_array( $body ); } - }; - - $successes = []; - $failures = []; - - ( new EachPromise( - $requests(), - [ - 'concurrency' => $concurrency, - 'fulfilled' => function ( array $body, int $index ) use ( &$successes ) { - $successes[ $index ] = ProductInput::from_array( $body ); - }, - 'rejected' => function ( $reason, int $index ) use ( &$failures ) { - $failures[ $index ] = $reason; - }, - ] - ) )->promise()->wait(); - - return [ - 'successes' => $successes, - 'failures' => $failures, - ]; + ); } /** @@ -245,26 +232,86 @@ public function delete_many( array $inputs, int $concurrency = 10 ): array { $paths_by_index[ $index ] = $this->build_delete_path( $input, $data_source ); } - $client = $this->client; - - $requests = function () use ( $inputs, $client, $paths_by_index ) { - foreach ( $inputs as $index => $input ) { - yield $index => $client->request_async( 'DELETE', $paths_by_index[ $index ] ); + return $this->run_in_batches( + $inputs, + $concurrency, + __METHOD__, + function ( int $index ) use ( $paths_by_index ): array { + return [ + 'method' => 'DELETE', + 'path' => $paths_by_index[ $index ], + ]; + }, + function ( array $body, ProductInput $input ): ProductInput { + return $input; } - }; + ); + } + + /** + * Run product sub-requests in concurrent multipart batches, demuxing each + * sub-response back to a per-input success (ProductInput) or failure + * (MerchantApiException). A whole-batch failure marks every input in it as failed. + * + * @param ProductInput[] $inputs + * @param int $concurrency Concurrent batch requests. + * @param string $operation Method label used on sub-request failures. + * @param callable $build_request fn(int $index, ProductInput $input): array{method: string, path: string, body?: array} + * @param callable $on_success fn(array $body, ProductInput $input): ProductInput + * + * @return array{successes: array, failures: array} + */ + private function run_in_batches( array $inputs, int $concurrency, string $operation, callable $build_request, callable $on_success ): array { + $client = $this->client; + $index_batches = array_chunk( array_keys( $inputs ), $this->get_batch_size() ); $successes = []; $failures = []; + $promises = function () use ( $index_batches, $inputs, $client, $build_request ) { + foreach ( $index_batches as $batch_num => $indices ) { + $requests = []; + foreach ( $indices as $index ) { + $requests[ $index ] = $build_request( $index, $inputs[ $index ] ); + } + + yield $batch_num => $client->batch_async( $requests ); + } + }; + ( new EachPromise( - $requests(), + $promises(), [ 'concurrency' => $concurrency, - 'fulfilled' => function ( array $body, int $index ) use ( &$successes, $inputs ) { - $successes[ $index ] = $inputs[ $index ]; + 'fulfilled' => function ( array $batch_results, $batch_num ) use ( &$successes, &$failures, $inputs, $index_batches, $on_success, $operation ) { + foreach ( $batch_results as $index => $result ) { + // Ignore a sub-response for an id that was not in this batch (guards + // against a malformed or duplicate Content-ID in the response). + if ( ! isset( $inputs[ $index ] ) ) { + continue; + } + + if ( $result['status'] >= 200 && $result['status'] < 300 ) { + $successes[ $index ] = $on_success( $result['body'], $inputs[ $index ] ); + } else { + $failures[ $index ] = new MerchantApiException( $result['status'], $result['body'], $operation ); + } + } + + // A requested sub-request missing from the parsed response is a retryable + // failure, never a silent no-op. + foreach ( array_diff( $index_batches[ $batch_num ], array_keys( $batch_results ) ) as $index ) { + $failures[ $index ] = new MerchantApiException( + 500, + [ 'error' => [ 'message' => 'No sub-response returned for this product in the batch response.' ] ], + $operation + ); + } }, - 'rejected' => function ( $reason, int $index ) use ( &$failures ) { - $failures[ $index ] = $reason; + 'rejected' => function ( $reason, $batch_num ) use ( &$failures, $index_batches ) { + foreach ( $index_batches[ $batch_num ] as $index ) { + $failures[ $index ] = $reason; + } }, ] ) )->promise()->wait(); @@ -275,6 +322,15 @@ public function delete_many( array $inputs, int $concurrency = 10 ): array { ]; } + /** + * Number of product sub-requests to pack into a single batch HTTP request. + * + * @return int + */ + protected function get_batch_size(): int { + return max( 1, (int) apply_filters( 'woocommerce_gla_mapi_batch_size', 50 ) ); + } + /** * Build the productInputs.insert path with the resolved data source. * From f4da1688e1b2a7ee355e0fe1e91d81979c9065ea Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:54:25 +0200 Subject: [PATCH 02/19] Add batch request handling to `MerchantApiClient` for efficient grouped sub-requests --- src/API/Google/Mapi/MerchantApiClient.php | 125 ++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/src/API/Google/Mapi/MerchantApiClient.php b/src/API/Google/Mapi/MerchantApiClient.php index 62aa941567..d0059053c1 100644 --- a/src/API/Google/Mapi/MerchantApiClient.php +++ b/src/API/Google/Mapi/MerchantApiClient.php @@ -22,6 +22,8 @@ */ class MerchantApiClient { + private const BATCH_BOUNDARY = 'gla_mapi_batch_boundary'; + /** @var ClientInterface */ protected $http; @@ -146,6 +148,129 @@ public function delete( string $path ): array { return $this->request( 'DELETE', $path ); } + /** + * Send a batch of sub-requests as a single multipart/mixed HTTP request and return + * the per-sub-request results keyed by the input key. + * + * @param array $requests + * + * @return array + * @throws MerchantApiException On a non-2xx response for the batch request itself. + */ + public function batch( array $requests ): array { + return $this->batch_async( $requests )->wait(); + } + + /** + * Asynchronous variant of {@see batch()}. + * + * @param array $requests + * + * @return PromiseInterface Resolves to array. + */ + public function batch_async( array $requests ): PromiseInterface { + $request = $this->build_batch_request( $requests ); + $method_label = __METHOD__; + + return $this->http->sendAsync( $request )->then( + function ( ResponseInterface $response ): array { + return $this->parse_batch_response( $response ); + }, + function ( $reason ) use ( $method_label ) { + if ( $reason instanceof RequestException && $reason->hasResponse() ) { + throw new MerchantApiException( + $reason->getResponse()->getStatusCode(), + $this->decode_response( $reason->getResponse() ), + $method_label, + $reason + ); + } + + throw $reason; + } + ); + } + + /** + * Build the multipart/mixed batch request. Each sub-request is an application/http + * part tagged with a Content-ID so response parts can be mapped back by key. + * + * @param array $requests + * + * @return Request + */ + protected function build_batch_request( array $requests ): Request { + $boundary = self::BATCH_BOUNDARY . '_' . wp_generate_password( 16, false ); + $body = ''; + + foreach ( $requests as $key => $sub ) { + $body .= "--{$boundary}\r\n"; + $body .= "Content-Type: application/http\r\n"; + $body .= "Content-ID: \r\n\r\n"; + $body .= sprintf( "%s /%s\r\n", $sub['method'], ltrim( $sub['path'], '/' ) ); + + if ( isset( $sub['body'] ) ) { + $body .= "Content-Type: application/json\r\n\r\n"; + $body .= (string) wp_json_encode( $sub['body'] ) . "\r\n"; + } else { + $body .= "\r\n"; + } + } + + $body .= "--{$boundary}--\r\n"; + + return new Request( + 'POST', + $this->base_url . 'batch', + [ 'Content-Type' => "multipart/mixed; boundary={$boundary}" ], + $body + ); + } + + /** + * Parse a multipart/mixed batch response into per-sub-request results keyed by the + * original request key, extracted from each part's Content-ID. + * + * @param ResponseInterface $response + * + * @return array + */ + protected function parse_batch_response( ResponseInterface $response ): array { + if ( ! preg_match( '/boundary=(?:"([^"]+)"|([^;\s]+))/', $response->getHeaderLine( 'Content-Type' ), $matches ) ) { + return []; + } + + $boundary = '' !== ( $matches[1] ?? '' ) ? $matches[1] : $matches[2]; + $results = []; + + foreach ( explode( '--' . $boundary, (string) $response->getBody() ) as $part ) { + $part = trim( $part ); + if ( '' === $part || '--' === $part ) { + continue; + } + + if ( ! preg_match( '/Content-ID:\s*]+)>/i', $part, $id_match ) + || ! preg_match( '#HTTP/\d(?:\.\d)?\s+(\d+)#', $part, $status_match ) ) { + continue; + } + + $http_offset = strpos( $part, 'HTTP/' ); + $inner_body = ''; + if ( false !== $http_offset && preg_match( '/\r?\n\r?\n(.*)$/s', substr( $part, $http_offset ), $body_match ) ) { + $inner_body = trim( $body_match[1] ); + } + + $decoded = '' === $inner_body ? [] : json_decode( $inner_body, true ); + + $results[ $id_match[1] ] = [ + 'status' => (int) $status_match[1], + 'body' => is_array( $decoded ) ? $decoded : [], + ]; + } + + return $results; + } + /** * Decode a JSON response body to an array. * From 0519e217ea69a0bc3240c92c21c648f1b74bde8e Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:54:55 +0200 Subject: [PATCH 03/19] Add retry middleware to `GoogleServiceProvider` for handling transient Merchant API errors with capped exponential backoff and jitter --- .../GoogleServiceProvider.php | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/src/Internal/DependencyManagement/GoogleServiceProvider.php b/src/Internal/DependencyManagement/GoogleServiceProvider.php index 9d51fa9b2f..27d2f1ac39 100644 --- a/src/Internal/DependencyManagement/GoogleServiceProvider.php +++ b/src/Internal/DependencyManagement/GoogleServiceProvider.php @@ -59,8 +59,10 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Service\SiteVerification as SiteVerificationService; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Client as GuzzleClient; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\ClientInterface; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\ConnectException; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\RequestException; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\HandlerStack; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Middleware as GuzzleMiddleware; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Psr7\Utils; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\League\Container\Definition\Definition; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Psr\Http\Message\RequestInterface; @@ -189,6 +191,9 @@ protected function register_guzzle() { $handler_stack->push( $this->override_http_url(), 'override_http_url' ); } + // Innermost: retry transient failures before the error handler turns them into exceptions. + $handler_stack->push( $this->retry_on_transient_error(), 'retry_on_transient_error' ); + return new GuzzleClient( [ 'handler' => $handler_stack ] ); }; @@ -196,6 +201,95 @@ protected function register_guzzle() { $this->share_concrete( ClientInterface::class, new Definition( GuzzleClient::class, $callback ) ); } + /** + * Retry middleware for transient Merchant API failures: HTTP 429, 5xx, and + * connection errors. Honors a Retry-After header when present, otherwise uses + * capped exponential backoff with jitter. + * + * @return callable + */ + protected function retry_on_transient_error(): callable { + $limit = (int) apply_filters( 'woocommerce_gla_mapi_retry_limit', 3 ); + + return GuzzleMiddleware::retry( + function ( int $retries, RequestInterface $request, ?ResponseInterface $response = null, ?\Throwable $reason = null ) use ( $limit ): bool { + if ( $retries >= $limit ) { + return false; + } + + // No response means the request did not complete. + if ( ! $response instanceof ResponseInterface ) { + // A connection error never reached the server, so any method is safe. + if ( $reason instanceof ConnectException ) { + return true; + } + + // Other transport failures (e.g. a reset mid-response) may have been + // applied, so only retry idempotent requests or the product/batch paths. + return $reason instanceof RequestException && $this->is_retryable_request( $request ); + } + + $code = $response->getStatusCode(); + + // 429 (rate limited) is not applied server-side, so any method is safe to retry. + if ( 429 === $code ) { + return true; + } + + // A 5xx may have been applied, so only retry idempotent requests (or the + // product upsert/batch paths) to avoid duplicating non-idempotent writes. + return $code >= 500 && $this->is_retryable_request( $request ); + }, + function ( int $retries, ?ResponseInterface $response = null ): int { + return $this->retry_delay( $retries, $response ); + } + ); + } + + /** + * The delay in milliseconds before a retry: the Retry-After header when present, + * otherwise capped exponential backoff with jitter. Both paths are capped so a + * large value cannot stall the background sync job. + * + * @param int $retries + * @param ResponseInterface|null $response + * + * @return int + */ + protected function retry_delay( int $retries, ?ResponseInterface $response = null ): int { + $max_delay = 30000; + + if ( $response instanceof ResponseInterface && $response->hasHeader( 'Retry-After' ) ) { + $retry_after = (int) $response->getHeaderLine( 'Retry-After' ); + + if ( $retry_after > 0 ) { + return (int) min( $retry_after * 1000, $max_delay ); + } + } + + $backoff = ( 2 ** max( 0, $retries - 1 ) ) * 1000; + + return (int) min( $backoff, $max_delay ) + wp_rand( 0, 1000 ); + } + + /** + * Whether a request is safe to retry on a 5xx: idempotent methods, or the product + * upsert/batch endpoints where a POST is an upsert rather than a create. + * + * @param RequestInterface $request + * + * @return bool + */ + protected function is_retryable_request( RequestInterface $request ): bool { + if ( in_array( strtoupper( $request->getMethod() ), [ 'GET', 'HEAD', 'PUT', 'DELETE', 'PATCH' ], true ) ) { + return true; + } + + $path = $request->getUri()->getPath(); + + return false !== strpos( $path, 'productInputs' ) || false !== strpos( $path, 'batch' ); + } + /** * Register ads client. */ From dfc00c251fdb159afe5da64acad0d123aad09eb2 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:55:08 +0200 Subject: [PATCH 04/19] Add hash-based skipping for unchanged product syncs in `BatchProductHelper` to optimize re-sync logic --- src/Product/BatchProductHelper.php | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/Product/BatchProductHelper.php b/src/Product/BatchProductHelper.php index aac3ce3aeb..3aefb655cc 100644 --- a/src/Product/BatchProductHelper.php +++ b/src/Product/BatchProductHelper.php @@ -220,10 +220,18 @@ public function generate_mapi_update_entries( array $products ): array { $attributes = array_merge( $this->attribute_manager->get_all_values( $parent ), $attributes ); } + $input = ( new WCProductInputAdapter( $product, $country, $parent, $target_countries, $attributes, $mapping_rules ) )->get_product_input(); + $hash = $this->product_input_hash( $input ); + + if ( $this->can_skip_unchanged_product( $product, $hash ) ) { + continue; + } + $entries[] = [ 'product' => $product, 'country' => $country, - 'input' => ( new WCProductInputAdapter( $product, $country, $parent, $target_countries, $attributes, $mapping_rules ) )->get_product_input(), + 'input' => $input, + 'hash' => $hash, ]; } catch ( GoogleListingsAndAdsException $exception ) { do_action( @@ -239,6 +247,46 @@ public function generate_mapi_update_entries( array $products ): array { return $entries; } + /** + * A stable hash of the ProductInput payload, used to skip re-syncing products + * whose Merchant API payload is unchanged since the last successful sync. + * + * @param ProductInput $input + * + * @return string + */ + protected function product_input_hash( ProductInput $input ): string { + return md5( (string) wp_json_encode( $input->to_array() ) ); + } + + /** + * Whether a product can be skipped because its payload is unchanged since the last + * successful sync. Products old enough to be due for expiry resubmission are never + * skipped, and woocommerce_gla_force_product_resync forces a full re-sync. + * + * @param WC_Product $product + * @param string $hash The current ProductInput hash. + * + * @return bool + */ + protected function can_skip_unchanged_product( WC_Product $product, string $hash ): bool { + if ( apply_filters( 'woocommerce_gla_force_product_resync', false, $product ) ) { + return false; + } + + if ( $this->meta_handler->get_sync_hash( $product ) !== $hash ) { + return false; + } + + // Clamp to the expiry-resubmission window so a filtered freshness can never let an + // unchanged product be skipped past the point it is due for resubmission. + $max_freshness = ProductRepository::RESUBMIT_EXPIRY_DAYS * DAY_IN_SECONDS; + $freshness = min( (int) apply_filters( 'woocommerce_gla_sync_hash_freshness', $max_freshness ), $max_freshness ); + $synced_at = (int) $this->meta_handler->get_synced_at( $product ); + + return $synced_at > ( time() - $freshness ); + } + /** * Generate MAPI delete entries for the given products. * From 1125a5d38cda24457f2015b4106f0971fb44aab7 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:55:17 +0200 Subject: [PATCH 05/19] Add `update_sync_hash` method to `ProductHelper` to support hash-based product sync logic --- src/Product/ProductHelper.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php index c0f3922b9f..06dbaea48c 100644 --- a/src/Product/ProductHelper.php +++ b/src/Product/ProductHelper.php @@ -124,6 +124,17 @@ public function mark_as_synced( WC_Product $product, GoogleProduct $google_produ } } + /** + * Store the hash of the ProductInput payload last successfully synced, used to + * skip re-syncing unchanged products. + * + * @param WC_Product $product + * @param string $hash + */ + public function update_sync_hash( WC_Product $product, string $hash ): void { + $this->meta_handler->update_sync_hash( $product, $hash ); + } + /** * @param WC_Product $product */ From 7ad5fb1727499a32ce943958afc2898654eadcd7 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:55:26 +0200 Subject: [PATCH 06/19] Add `sync_hash` metadata to `ProductMetaHandler` to support hash-based product sync logic --- src/Product/ProductMetaHandler.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Product/ProductMetaHandler.php b/src/Product/ProductMetaHandler.php index 581cbdcd6f..1ae649d097 100644 --- a/src/Product/ProductMetaHandler.php +++ b/src/Product/ProductMetaHandler.php @@ -44,6 +44,9 @@ * @method update_mc_status( WC_Product $product, string $value ) * @method delete_mc_status( WC_Product $product ) * @method get_mc_status( WC_Product $product ): string|null + * @method update_sync_hash( WC_Product $product, string $value ) + * @method delete_sync_hash( WC_Product $product ) + * @method get_sync_hash( WC_Product $product ): string|null */ class ProductMetaHandler implements Service, Registerable { @@ -58,6 +61,7 @@ class ProductMetaHandler implements Service, Registerable { public const KEY_SYNC_FAILED_AT = 'sync_failed_at'; public const KEY_SYNC_STATUS = 'sync_status'; public const KEY_MC_STATUS = 'mc_status'; + public const KEY_SYNC_HASH = 'sync_hash'; protected const TYPES = [ self::KEY_SYNCED_AT => 'int', @@ -69,6 +73,7 @@ class ProductMetaHandler implements Service, Registerable { self::KEY_SYNC_FAILED_AT => 'int', self::KEY_SYNC_STATUS => 'string', self::KEY_MC_STATUS => 'string', + self::KEY_SYNC_HASH => 'string', ]; /** From a4b0a97dc7fdd1a7e00172be064bfe5031f61ae9 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:55:38 +0200 Subject: [PATCH 07/19] Introduce `RESUBMIT_EXPIRY_DAYS` constant in `ProductRepository` to centralize re-submission expiry logic --- src/Product/ProductRepository.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Product/ProductRepository.php b/src/Product/ProductRepository.php index f23c7f61f1..4e5ae3a1a1 100644 --- a/src/Product/ProductRepository.php +++ b/src/Product/ProductRepository.php @@ -21,6 +21,13 @@ class ProductRepository implements Service { use PluginHelper; + /** + * Products whose synced_at is older than this many days are treated as nearly expired + * and re-submitted (see find_expiring_product_ids). The delta-sync freshness window is + * clamped to this so an unchanged product can never be skipped past its resubmission point. + */ + public const RESUBMIT_EXPIRY_DAYS = 25; + /** * @var ProductMetaHandler */ @@ -281,7 +288,7 @@ public function find_expiring_product_ids( int $last_id = 0, int $limit = -1 ): [ 'key' => ProductMetaHandler::KEY_SYNCED_AT, 'compare' => '<', - 'value' => strtotime( '-25 days' ), + 'value' => strtotime( '-' . self::RESUBMIT_EXPIRY_DAYS . ' days' ), ], ], ]; From 6c64d956cda465cb62598e26240a53beae6d54a6 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:55:47 +0200 Subject: [PATCH 08/19] Add concurrency control and retry handling to `ProductSyncer` for robust batch sync logic --- src/Product/ProductSyncer.php | 58 ++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/Product/ProductSyncer.php b/src/Product/ProductSyncer.php index bcf8dbbafe..7a730f94ed 100644 --- a/src/Product/ProductSyncer.php +++ b/src/Product/ProductSyncer.php @@ -4,6 +4,8 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Product; use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Mapi\MerchantApiException; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\ConnectException; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\RequestException; use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Mapi\Models\ProductInput; use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Mapi\Services\MapiProductInputsService; use Automattic\WooCommerce\GoogleListingsAndAds\Google\BatchInvalidProductEntry; @@ -28,6 +30,7 @@ class ProductSyncer implements Service { public const FAILURE_THRESHOLD = 5; // Number of failed attempts allowed per FAILURE_THRESHOLD_WINDOW public const FAILURE_THRESHOLD_WINDOW = '3 hours'; // PHP supported Date and Time format: https://www.php.net/manual/en/datetime.formats.php + public const DEFAULT_CONCURRENCY = 10; // Concurrent Merchant API batch requests during a product sync. /** * @var MapiProductInputsService @@ -128,7 +131,7 @@ static function ( array $entry ): ProductInput { ); try { - $result = $this->mapi_inputs->insert_many( $inputs ); + $result = $this->mapi_inputs->insert_many( $inputs, $this->get_sync_concurrency() ); } catch ( Exception $exception ) { do_action( 'woocommerce_gla_exception', $exception, __METHOD__ ); @@ -144,6 +147,10 @@ static function ( array $entry ): ProductInput { $updated_products[] = $synced_entry; $this->batch_helper->mark_as_synced( $synced_entry ); + + if ( isset( $entry['hash'] ) ) { + $this->product_helper->update_sync_hash( $entry['product'], $entry['hash'] ); + } } elseif ( isset( $result['failures'][ $index ] ) ) { $invalid_entry = $this->build_invalid_entry( $entry['product']->get_id(), $result['failures'][ $index ] ); @@ -193,7 +200,7 @@ protected function build_synced_google_product( ProductInput $input, string $cou * @return BatchInvalidProductEntry */ protected function build_invalid_entry( int $wc_product_id, Exception $reason ): BatchInvalidProductEntry { - if ( $reason instanceof MerchantApiException && $reason->get_http_status() >= 500 ) { + if ( $this->is_retryable_reason( $reason ) ) { $errors = [ GoogleProductService::INTERNAL_ERROR_REASON => $reason->getMessage() ]; } else { $errors = [ 'invalid' => $reason->getMessage() ]; @@ -202,6 +209,49 @@ protected function build_invalid_entry( int $wc_product_id, Exception $reason ): return new BatchInvalidProductEntry( $wc_product_id, null, $errors ); } + /** + * Whether a Merchant API status should be retried (429 rate limit or 5xx) rather + * than recorded as a permanent invalid product. + * + * @param int $status + * + * @return bool + */ + protected function is_retryable_status( int $status ): bool { + return 429 === $status || $status >= 500; + } + + /** + * Whether a sync failure should be retried: a transient Merchant API status (429/5xx) + * or a connection-level error that outlived the client's retry middleware. + * + * @param Exception $reason + * + * @return bool + */ + protected function is_retryable_reason( Exception $reason ): bool { + if ( $reason instanceof ConnectException ) { + return true; + } + + if ( $reason instanceof MerchantApiException ) { + return $this->is_retryable_status( $reason->get_http_status() ); + } + + // A transport error with no HTTP response (connection reset, etc.) is transient. + return $reason instanceof RequestException && ! $reason->hasResponse(); + } + + /** + * Number of concurrent Merchant API product requests to run per batch, + * filterable via woocommerce_gla_mapi_product_concurrency. + * + * @return int + */ + protected function get_sync_concurrency(): int { + return max( 1, (int) apply_filters( 'woocommerce_gla_mapi_product_concurrency', self::DEFAULT_CONCURRENCY ) ); + } + /** * Deletes an array of WooCommerce products from Google Merchant Center. * @@ -276,7 +326,7 @@ static function ( array $entry ): ProductInput { ); try { - $result = $this->mapi_inputs->delete_many( $inputs ); + $result = $this->mapi_inputs->delete_many( $inputs, $this->get_sync_concurrency() ); } catch ( Exception $exception ) { do_action( 'woocommerce_gla_exception', $exception, __METHOD__ ); @@ -325,7 +375,7 @@ protected function build_delete_invalid_entry( int $wc_product_id, string $googl if ( 404 === $status ) { $errors = [ GoogleProductService::NOT_FOUND_ERROR_REASON => $reason->getMessage() ]; - } elseif ( $status >= 500 ) { + } elseif ( $this->is_retryable_reason( $reason ) ) { $errors = [ GoogleProductService::INTERNAL_ERROR_REASON => $reason->getMessage() ]; } else { $errors = [ 'invalid' => $reason->getMessage() ]; From 8addac14400ddeb5d0b9d57f78842ac9481835db Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:56:00 +0200 Subject: [PATCH 09/19] Refactor `MapiProductInputsServiceTest` to replace `request_async` with `batch_async`, add sub-response handling tests, and ensure proper indexing and failure identification --- .../Services/MapiProductInputsServiceTest.php | 214 +++++++++++++++--- 1 file changed, 186 insertions(+), 28 deletions(-) diff --git a/tests/Unit/API/Google/Mapi/Services/MapiProductInputsServiceTest.php b/tests/Unit/API/Google/Mapi/Services/MapiProductInputsServiceTest.php index a6ff156dee..ef9985826d 100644 --- a/tests/Unit/API/Google/Mapi/Services/MapiProductInputsServiceTest.php +++ b/tests/Unit/API/Google/Mapi/Services/MapiProductInputsServiceTest.php @@ -139,19 +139,29 @@ public function test_insert_propagates_merchant_api_exception() { } public function test_insert_many_keys_successes_and_failures_by_index() { - $this->client->method( 'request_async' ) + $this->client->method( 'batch_async' ) ->willReturnCallback( - function ( string $method, string $path, array $body ) { - if ( 'bad' === $body['offerId'] ) { - return Create::rejectionFor( new MerchantApiException( 500, [], __METHOD__ ) ); + function ( array $requests ) { + $results = []; + foreach ( $requests as $index => $sub ) { + $offer_id = $sub['body']['offerId']; + if ( 'bad' === $offer_id ) { + $results[ $index ] = [ + 'status' => 500, + 'body' => [], + ]; + } else { + $results[ $index ] = [ + 'status' => 200, + 'body' => [ + 'name' => 'accounts/12345/productInputs/' . $offer_id, + 'offerId' => $offer_id, + ], + ]; + } } - return Create::promiseFor( - [ - 'name' => 'accounts/12345/productInputs/' . $body['offerId'], - 'offerId' => $body['offerId'], - ] - ); + return Create::promiseFor( $results ); } ); @@ -176,17 +186,23 @@ function ( string $method, string $path, array $body ) { public function test_insert_many_routes_each_input_to_its_own_data_source() { $paths_seen = []; - $this->client->method( 'request_async' ) + $this->client->method( 'batch_async' ) ->willReturnCallback( - function ( string $method, string $path, array $body ) use ( &$paths_seen ) { - $paths_seen[ $body['offerId'] ] = $path; + function ( array $requests ) use ( &$paths_seen ) { + $results = []; + foreach ( $requests as $index => $sub ) { + $offer_id = $sub['body']['offerId']; + $paths_seen[ $offer_id ] = $sub['path']; + $results[ $index ] = [ + 'status' => 200, + 'body' => [ + 'name' => 'accounts/12345/productInputs/' . $offer_id, + 'offerId' => $offer_id, + ], + ]; + } - return Create::promiseFor( - [ - 'name' => 'accounts/12345/productInputs/' . $body['offerId'], - 'offerId' => $body['offerId'], - ] - ); + return Create::promiseFor( $results ); } ); @@ -201,6 +217,132 @@ function ( string $method, string $path, array $body ) use ( &$paths_seen ) { $this->assertSame( $this->expected_path( self::DS_FR_CA ), $paths_seen['ca_sku'] ); } + public function test_insert_many_marks_missing_batch_subresponses_as_failures() { + $this->client->method( 'batch_async' ) + ->willReturnCallback( + function ( array $requests ) { + // Simulate the parser yielding no sub-response for 'bad'. + $results = []; + foreach ( $requests as $index => $sub ) { + if ( 'bad' === $sub['body']['offerId'] ) { + continue; + } + $results[ $index ] = [ + 'status' => 200, + 'body' => [ 'offerId' => $sub['body']['offerId'] ], + ]; + } + + return Create::promiseFor( $results ); + } + ); + + $result = $this->service->insert_many( + [ + $this->make_input( 'good1' ), + $this->make_input( 'bad' ), + $this->make_input( 'good2' ), + ] + ); + + $this->assertCount( 2, $result['successes'] ); + $this->assertCount( 1, $result['failures'] ); + $this->assertArrayHasKey( 1, $result['failures'] ); + // A missing sub-response is a retryable failure (>= 500), never a silent no-op. + $this->assertGreaterThanOrEqual( 500, $result['failures'][1]->get_http_status() ); + } + + public function test_insert_many_marks_all_inputs_failed_when_the_batch_request_rejects() { + $this->client->method( 'batch_async' ) + ->willReturn( Create::rejectionFor( new MerchantApiException( 503, [], __METHOD__ ) ) ); + + $result = $this->service->insert_many( + [ + $this->make_input( 'a' ), + $this->make_input( 'b' ), + $this->make_input( 'c' ), + ] + ); + + $this->assertCount( 0, $result['successes'] ); + $this->assertCount( 3, $result['failures'] ); + $this->assertInstanceOf( MerchantApiException::class, $result['failures'][0] ); + $this->assertSame( 503, $result['failures'][0]->get_http_status() ); + } + + public function test_insert_many_chunks_into_multiple_batches_and_keys_by_original_index() { + add_filter( + 'woocommerce_gla_mapi_batch_size', + function () { + return 2; + } + ); + + $batch_calls = 0; + $this->client->method( 'batch_async' ) + ->willReturnCallback( + function ( array $requests ) use ( &$batch_calls ) { + ++$batch_calls; + $results = []; + foreach ( $requests as $index => $sub ) { + $results[ $index ] = [ + 'status' => 200, + 'body' => [ 'offerId' => $sub['body']['offerId'] ], + ]; + } + + return Create::promiseFor( $results ); + } + ); + + $result = $this->service->insert_many( + [ + $this->make_input( 's0' ), + $this->make_input( 's1' ), + $this->make_input( 's2' ), + $this->make_input( 's3' ), + $this->make_input( 's4' ), + ] + ); + remove_all_filters( 'woocommerce_gla_mapi_batch_size' ); + + // 5 inputs at batch_size 2 => 3 batches (2 + 2 + 1), demuxed back to the original indices. + $this->assertSame( 3, $batch_calls ); + $this->assertCount( 5, $result['successes'] ); + foreach ( range( 0, 4 ) as $i ) { + $this->assertArrayHasKey( $i, $result['successes'] ); + $this->assertSame( "s{$i}", $result['successes'][ $i ]->get_offer_id() ); + } + } + + public function test_delete_many_ignores_a_subresponse_for_an_unrequested_id() { + $this->client->method( 'batch_async' ) + ->willReturnCallback( + function ( array $requests ) { + $results = []; + foreach ( $requests as $index => $sub ) { + $results[ $index ] = [ + 'status' => 200, + 'body' => [], + ]; + } + // A stray sub-response for an id that was never requested. + $results[999] = [ + 'status' => 200, + 'body' => [], + ]; + + return Create::promiseFor( $results ); + } + ); + + $result = $this->service->delete_many( [ $this->make_input( 'a' ) ] ); + + // The typed delete callback must not receive a null input: the stray id is ignored. + $this->assertCount( 1, $result['successes'] ); + $this->assertArrayNotHasKey( 999, $result['successes'] ); + } + public function test_patch_resolves_data_source_and_builds_correct_path() { $input = $this->make_input(); $mask = [ 'productAttributes.title' ]; @@ -358,14 +500,23 @@ public function test_delete_propagates_merchant_api_exception() { } public function test_delete_many_keys_successes_and_failures_by_index() { - $this->client->method( 'request_async' ) + $this->client->method( 'batch_async' ) ->willReturnCallback( - function ( string $method, string $path ) { - if ( false !== strpos( $path, 'bad' ) ) { - return Create::rejectionFor( new MerchantApiException( 404, [], __METHOD__ ) ); + function ( array $requests ) { + $results = []; + foreach ( $requests as $index => $sub ) { + $results[ $index ] = false !== strpos( $sub['path'], 'bad' ) + ? [ + 'status' => 404, + 'body' => [], + ] + : [ + 'status' => 200, + 'body' => [], + ]; } - return Create::promiseFor( [] ); + return Create::promiseFor( $results ); } ); @@ -388,12 +539,19 @@ function ( string $method, string $path ) { public function test_delete_many_routes_each_input_to_its_own_data_source() { $paths_seen = []; - $this->client->method( 'request_async' ) + $this->client->method( 'batch_async' ) ->willReturnCallback( - function ( string $method, string $path ) use ( &$paths_seen ) { - $paths_seen[] = [ $method, $path ]; + function ( array $requests ) use ( &$paths_seen ) { + $results = []; + foreach ( $requests as $index => $sub ) { + $paths_seen[] = [ $sub['method'], $sub['path'] ]; + $results[ $index ] = [ + 'status' => 200, + 'body' => [], + ]; + } - return Create::promiseFor( [] ); + return Create::promiseFor( $results ); } ); From e729a8fd92f8593a839b45d667457ad6d157c866 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:56:08 +0200 Subject: [PATCH 10/19] Add tests for `MerchantApiClient` batch handling with multipart requests and sub-response parsing --- .../API/Google/Mapi/MerchantApiClientTest.php | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php b/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php index 1992ae94eb..9d20eae52f 100644 --- a/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php +++ b/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php @@ -147,4 +147,83 @@ public function test_three_parallel_get_async_via_each_promise_all_fulfill() { $this->assertSame( 'accounts/1/products/b', $results['b']['name'] ); $this->assertSame( 'accounts/1/products/c', $results['c']['name'] ); } + + public function test_batch_demuxes_per_subrequest_results() { + $boundary = 'resp_boundary'; + $multipart = $this->build_multipart_response( + $boundary, + [ + 0 => [ 200, [ 'name' => 'accounts/1/products/gla_10' ] ], + 1 => [ 400, [ 'error' => [ 'message' => 'bad' ] ] ], + ] + ); + $this->mock->append( new Response( 200, [ 'Content-Type' => "multipart/mixed; boundary={$boundary}" ], $multipart ) ); + + $results = $this->client->batch( + [ + 0 => [ + 'method' => 'POST', + 'path' => 'products/v1/a/productInputs:insert', + 'body' => [ 'offerId' => 'gla_10' ], + ], + 1 => [ + 'method' => 'POST', + 'path' => 'products/v1/a/productInputs:insert', + 'body' => [ 'offerId' => 'gla_11' ], + ], + ] + ); + + $this->assertSame( 200, $results[0]['status'] ); + $this->assertSame( 'accounts/1/products/gla_10', $results[0]['body']['name'] ); + $this->assertSame( 400, $results[1]['status'] ); + $this->assertSame( 'bad', $results[1]['body']['error']['message'] ); + } + + public function test_batch_sends_multipart_request() { + $this->mock->append( new Response( 200, [ 'Content-Type' => 'multipart/mixed; boundary=b' ], "--b--\r\n" ) ); + + $this->client->batch( + [ + 0 => [ + 'method' => 'POST', + 'path' => 'products/v1/a/productInputs:insert', + 'body' => [ 'offerId' => 'gla_10' ], + ], + ] + ); + + $request = $this->history[0]['request']; + $this->assertSame( 'POST', $request->getMethod() ); + $this->assertSame( self::BASE_URL . 'batch', (string) $request->getUri() ); + $this->assertStringContainsString( 'multipart/mixed; boundary=', $request->getHeaderLine( 'Content-Type' ) ); + + $body = (string) $request->getBody(); + $this->assertStringContainsString( 'Content-ID: ', $body ); + $this->assertStringContainsString( 'POST /products/v1/a/productInputs:insert', $body ); + $this->assertStringContainsString( '"offerId":"gla_10"', $body ); + } + + /** + * Build a Google-style multipart/mixed batch response body. + * + * @param string $boundary + * @param array $subs Keyed by request key: [ status, body ]. + * + * @return string + */ + protected function build_multipart_response( string $boundary, array $subs ): string { + $out = ''; + foreach ( $subs as $key => $sub ) { + $out .= "--{$boundary}\r\n"; + $out .= "Content-Type: application/http\r\n"; + $out .= "Content-ID: \r\n\r\n"; + $out .= "HTTP/1.1 {$sub[0]} OK\r\n"; + $out .= "Content-Type: application/json; charset=UTF-8\r\n\r\n"; + $out .= wp_json_encode( $sub[1] ) . "\r\n\r\n"; + } + $out .= "--{$boundary}--\r\n"; + + return $out; + } } From 81ff864988c94571bd7a962a7934f389c4f1571f Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:56:18 +0200 Subject: [PATCH 11/19] Add unit tests for retry handling in `Client` to cover transient errors, connection issues, and retry limitations --- tests/Unit/API/ClientTest.php | 136 ++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/tests/Unit/API/ClientTest.php b/tests/Unit/API/ClientTest.php index c031560bd7..48599fff47 100644 --- a/tests/Unit/API/ClientTest.php +++ b/tests/Unit/API/ClientTest.php @@ -12,6 +12,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Client; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\ConnectException; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\RequestException; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Handler\MockHandler; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\HandlerStack; @@ -111,6 +112,141 @@ public function test_error_handler_regular_response() { $this->assertEquals( 'response', $response->getBody() ); } + public function test_retry_on_transient_error_retries_transient_status() { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new Response( 429, [], 'rate limited' ), + new Response( 200, [], 'ok' ), + ] + ); + $response = $client->request( 'GET', 'https://testing.local' ); + + // The 429 is retried and the following success is returned. + $this->assertEquals( 200, $response->getStatusCode() ); + } + + public function test_retry_on_transient_error_ignores_client_errors() { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new Response( 400, [], 'bad request' ), + new Response( 200, [], 'ok' ), + ] + ); + + // A 400 is not retried: it surfaces instead of consuming the queued success. + $this->expectException( RequestException::class ); + $client->request( 'GET', 'https://testing.local' ); + } + + public function test_retry_on_transient_error_retries_5xx_on_product_insert() { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new Response( 503, [], 'unavailable' ), + new Response( 200, [], 'ok' ), + ] + ); + + // productInputs.insert is an upsert, so a 5xx POST is safe to retry. + $response = $client->request( 'POST', 'https://testing.local/products/v1/accounts/1/productInputs:insert' ); + + $this->assertEquals( 200, $response->getStatusCode() ); + } + + public function test_retry_on_transient_error_does_not_retry_5xx_on_non_idempotent_post() { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new Response( 503, [], 'unavailable' ), + new Response( 200, [], 'ok' ), + ] + ); + + // A 5xx on a non-idempotent, non-product POST is not retried: it surfaces. + $this->expectException( RequestException::class ); + $client->request( 'POST', 'https://testing.local/datasources/v1/accounts/1/dataSources' ); + } + + public function test_retry_on_transient_error_retries_connection_errors() { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new ConnectException( 'connection timed out', new Request( 'GET', 'https://testing.local' ) ), + new Response( 200, [], 'ok' ), + ] + ); + + // A connection error (no response) is retried. + $response = $client->request( 'GET', 'https://testing.local' ); + + $this->assertEquals( 200, $response->getStatusCode() ); + } + + public function test_retry_on_transient_error_gives_up_after_the_retry_limit() { + add_filter( + 'woocommerce_gla_mapi_retry_limit', + function () { + return 1; + } + ); + + try { + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new Response( 503, [], 'unavailable' ), + new Response( 503, [], 'unavailable' ), + ] + ); + + // With a retry limit of 1, a persistent 5xx surfaces after the single retry. + $this->expectException( RequestException::class ); + $client->request( 'GET', 'https://testing.local' ); + } finally { + remove_all_filters( 'woocommerce_gla_mapi_retry_limit' ); + } + } + + public function test_retry_delay_caps_a_large_retry_after() { + $method = new ReflectionMethod( GoogleServiceProvider::class, 'retry_delay' ); + $method->setAccessible( true ); + + // Retry-After: 3600 would be 3,600,000ms uncapped; must be clamped to 30s so a + // large value can't stall the background sync job. + $delay = $method->invoke( $this->provider, 1, new Response( 503, [ 'Retry-After' => '3600' ] ) ); + + $this->assertSame( 30000, $delay ); + } + + public function test_retry_delay_caps_the_backoff() { + $method = new ReflectionMethod( GoogleServiceProvider::class, 'retry_delay' ); + $method->setAccessible( true ); + + // A high retry count must not exceed the cap (plus up to 1s jitter). + $delay = $method->invoke( $this->provider, 20, null ); + + $this->assertGreaterThanOrEqual( 30000, $delay ); + $this->assertLessThanOrEqual( 31000, $delay ); + } + + public function test_retry_on_transient_error_retries_no_response_transport_errors() { + $request = new Request( 'GET', 'https://testing.local' ); + $client = $this->mock_client_with_handler( + 'retry_on_transient_error', + [ + new RequestException( 'connection reset', $request ), + new Response( 200, [], 'ok' ), + ] + ); + + // A transport error with no response on an idempotent request is retried. + $response = $client->request( 'GET', 'https://testing.local' ); + + $this->assertEquals( 200, $response->getStatusCode() ); + } + /** * Confirm that the error handler throws an error to reconnect Jetpack when the header is not included. */ From 38d6e86da0b41fe6d64846fc9ba3885d792a490a Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:56:35 +0200 Subject: [PATCH 12/19] Add unit tests for `BatchProductHelper` to verify hash-based skipping, forced resync, and freshness logic behaviors --- tests/Unit/Product/BatchProductHelperTest.php | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/Unit/Product/BatchProductHelperTest.php b/tests/Unit/Product/BatchProductHelperTest.php index d262190ca9..5461ba8dc4 100644 --- a/tests/Unit/Product/BatchProductHelperTest.php +++ b/tests/Unit/Product/BatchProductHelperTest.php @@ -338,6 +338,75 @@ public function test_generate_mapi_update_entries_merges_parent_and_variation_at $this->assertSame( 'VariationColor', $attrs['color'] ); } + public function test_generate_skips_unchanged_recently_synced_product() { + $this->target_audience->expects( $this->any() )->method( 'get_main_target_country' )->willReturn( 'US' ); + $this->target_audience->expects( $this->any() )->method( 'get_target_countries' )->willReturn( [ 'US' ] ); + $this->rules_query->expects( $this->any() )->method( 'get_results' )->willReturn( [] ); + + $product = WC_Helper_Product::create_simple_product(); + + // First pass builds the entry and its payload hash. + $entries = $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ); + $this->assertCount( 1, $entries ); + $hash = $entries[0]['hash']; + + // Simulate a successful sync of that payload. + $this->product_meta->update_sync_hash( $product, $hash ); + $this->product_meta->update_synced_at( $product, time() ); + + // Unchanged and recently synced: skipped. + $this->assertEmpty( $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ) ); + + // Stale sync (older than the expiry window): not skipped, so it gets refreshed. + $this->product_meta->update_synced_at( $product, time() - ( 26 * DAY_IN_SECONDS ) ); + $this->assertCount( 1, $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ) ); + } + + public function test_force_resync_filter_includes_unchanged_product() { + $this->target_audience->expects( $this->any() )->method( 'get_main_target_country' )->willReturn( 'US' ); + $this->target_audience->expects( $this->any() )->method( 'get_target_countries' )->willReturn( [ 'US' ] ); + $this->rules_query->expects( $this->any() )->method( 'get_results' )->willReturn( [] ); + + $product = WC_Helper_Product::create_simple_product(); + $entries = $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ); + $this->product_meta->update_sync_hash( $product, $entries[0]['hash'] ); + $this->product_meta->update_synced_at( $product, time() ); + + // Would be skipped without the filter. + $this->assertEmpty( $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ) ); + + add_filter( 'woocommerce_gla_force_product_resync', '__return_true' ); + $forced = $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ); + remove_filter( 'woocommerce_gla_force_product_resync', '__return_true' ); + + $this->assertCount( 1, $forced ); + } + + public function test_freshness_filter_is_clamped_to_the_expiry_window() { + $this->target_audience->expects( $this->any() )->method( 'get_main_target_country' )->willReturn( 'US' ); + $this->target_audience->expects( $this->any() )->method( 'get_target_countries' )->willReturn( [ 'US' ] ); + $this->rules_query->expects( $this->any() )->method( 'get_results' )->willReturn( [] ); + + $product = WC_Helper_Product::create_simple_product(); + $entries = $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ); + $this->product_meta->update_sync_hash( $product, $entries[0]['hash'] ); + // Synced 30 days ago: past the 25-day resubmission window. + $this->product_meta->update_synced_at( $product, time() - ( 30 * DAY_IN_SECONDS ) ); + + // A freshness filter above the expiry window must not let the product be skipped, + // or ResubmitExpiringProducts would no-op and the product could expire out of MC. + add_filter( + 'woocommerce_gla_sync_hash_freshness', + function () { + return 60 * DAY_IN_SECONDS; + } + ); + $entries2 = $this->batch_product_helper->generate_mapi_update_entries( [ $product ] ); + remove_all_filters( 'woocommerce_gla_sync_hash_freshness' ); + + $this->assertCount( 1, $entries2 ); + } + /** * @return WC_Product[] */ From 4029c3333e36188310396e1c38fe9b1861a23861 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 13 Jul 2026 14:56:45 +0200 Subject: [PATCH 13/19] Add unit tests to `ProductSyncerTest` for retry handling, concurrency control, and sync hash logic, covering transient errors, rate-limited responses, and connection issues --- tests/Unit/Product/ProductSyncerTest.php | 293 ++++++++++++++++++++++- 1 file changed, 290 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Product/ProductSyncerTest.php b/tests/Unit/Product/ProductSyncerTest.php index fd20bfd851..458cc55c7b 100644 --- a/tests/Unit/Product/ProductSyncerTest.php +++ b/tests/Unit/Product/ProductSyncerTest.php @@ -22,6 +22,9 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Value\SyncStatus; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Exception as GoogleException; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Service\ShoppingContent\Product as GoogleProduct; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\ConnectException; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Exception\RequestException; +use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\Psr7\Request; use PHPUnit\Framework\MockObject\MockObject; use WC_Helper_Product; use WC_Product; @@ -113,12 +116,13 @@ function ( WC_Product $product ) { * * @param array $synced_products WC product IDs. * @param array $rejected_products WC product IDs. + * @param int $failure_status HTTP status to use for failures (500 or 429 to retry). */ - protected function mock_mapi_inputs( array $synced_products, array $rejected_products ): void { + protected function mock_mapi_inputs( array $synced_products, array $rejected_products, int $failure_status = 500 ): void { $this->mapi_inputs->expects( $this->any() ) ->method( 'insert_many' ) ->willReturnCallback( - function ( array $inputs ) use ( $synced_products, $rejected_products ) { + function ( array $inputs ) use ( $synced_products, $rejected_products, $failure_status ) { $successes = []; $failures = []; foreach ( $inputs as $index => $input ) { @@ -126,7 +130,7 @@ function ( array $inputs ) use ( $synced_products, $rejected_products ) { if ( isset( $synced_products[ $product_id ] ) ) { $successes[ $index ] = $input; } elseif ( isset( $rejected_products[ $product_id ] ) ) { - $failures[ $index ] = new MerchantApiException( 500, [], 'Internal Error!' ); + $failures[ $index ] = new MerchantApiException( $failure_status, [], 'Internal Error!' ); } } @@ -159,6 +163,289 @@ protected function assert_update_results_are_valid( $results, $synced_products, } } + public function test_update_rate_limited_products_are_retried_not_dropped() { + [ $synced_products, $rejected_products ] = $this->create_multiple_simple_product_sets( 2, 2 ); + + $batch_helper = $this->getMockBuilder( BatchProductHelper::class ) + ->setMethods( [ 'generate_mapi_update_entries' ] ) + ->setConstructorArgs( + [ + $this->product_meta, + $this->product_helper, + $this->target_audience, + $this->rules_query, + $this->container->get( AttributeManager::class ), + ] + ) + ->getMock(); + $batch_helper->expects( $this->once() ) + ->method( 'generate_mapi_update_entries' ) + ->willReturnCallback( + function ( array $products ) { + return array_map( + function ( WC_Product $product ) { + return [ + 'product' => $product, + 'country' => 'US', + 'input' => new ProductInput( "gla_{$product->get_id()}", 'en', 'US', [ 'title' => $product->get_title() ] ), + ]; + }, + $products + ); + } + ); + + $this->mock_mapi_inputs( $synced_products, $rejected_products, 429 ); + $product_syncer = $this->get_product_syncer( [ 'batch_helper' => $batch_helper ] ); + + $product_syncer->update( array_merge( $synced_products, $rejected_products ) ); + + // A 429 is transient: rate-limited products are rescheduled, not dropped as invalid. + $this->assertEquals( 1, did_action( 'woocommerce_gla_batch_retry_update_products' ) ); + foreach ( $rejected_products as $product ) { + $wc_product = wc_get_product( $product->get_id() ); + $this->assertEquals( SyncStatus::HAS_ERRORS, $this->product_meta->get_sync_status( $wc_product ) ); + } + } + + public function test_delete_rate_limited_products_are_retried_not_dropped() { + [ $deleted_products, $rejected_products ] = $this->create_multiple_simple_product_sets( 2, 2 ); + $products = array_merge( $deleted_products, $rejected_products ); + + array_walk( + $products, + function ( WC_Product $product ) { + $this->product_helper->mark_as_synced( + $product, + $this->generate_google_product_mock( "en~US~gla_{$product->get_id()}", 'US' ) + ); + } + ); + + $this->mock_mapi_delete( $deleted_products, $rejected_products, 429 ); + + $this->product_syncer->delete( $products ); + + // A 429 is transient: rate-limited deletes are rescheduled, not dropped as invalid. + $this->assertEquals( 1, did_action( 'woocommerce_gla_batch_retry_delete_products' ) ); + } + + public function test_sync_concurrency_is_filterable() { + [ $synced_products ] = $this->create_multiple_simple_product_sets( 1, 0 ); + + $batch_helper = $this->getMockBuilder( BatchProductHelper::class ) + ->setMethods( [ 'generate_mapi_update_entries' ] ) + ->setConstructorArgs( + [ + $this->product_meta, + $this->product_helper, + $this->target_audience, + $this->rules_query, + $this->container->get( AttributeManager::class ), + ] + ) + ->getMock(); + $batch_helper->expects( $this->once() ) + ->method( 'generate_mapi_update_entries' ) + ->willReturnCallback( + function ( array $products ) { + return array_map( + function ( WC_Product $product ) { + return [ + 'product' => $product, + 'country' => 'US', + 'input' => new ProductInput( "gla_{$product->get_id()}", 'en', 'US', [ 'title' => $product->get_title() ] ), + ]; + }, + $products + ); + } + ); + + $captured_concurrency = null; + $this->mapi_inputs->expects( $this->once() ) + ->method( 'insert_many' ) + ->willReturnCallback( + function ( array $inputs, int $concurrency ) use ( &$captured_concurrency ) { + $captured_concurrency = $concurrency; + + return [ + 'successes' => $inputs, + 'failures' => [], + ]; + } + ); + + add_filter( + 'woocommerce_gla_mapi_product_concurrency', + function () { + return 25; + } + ); + $this->get_product_syncer( [ 'batch_helper' => $batch_helper ] )->update( $synced_products ); + remove_all_filters( 'woocommerce_gla_mapi_product_concurrency' ); + + $this->assertEquals( 25, $captured_concurrency ); + } + + public function test_update_stores_the_sync_hash_on_success() { + [ $synced_products ] = $this->create_multiple_simple_product_sets( 1, 0 ); + $product = reset( $synced_products ); + + $batch_helper = $this->getMockBuilder( BatchProductHelper::class ) + ->setMethods( [ 'generate_mapi_update_entries' ] ) + ->setConstructorArgs( + [ + $this->product_meta, + $this->product_helper, + $this->target_audience, + $this->rules_query, + $this->container->get( AttributeManager::class ), + ] + ) + ->getMock(); + $batch_helper->expects( $this->once() ) + ->method( 'generate_mapi_update_entries' ) + ->willReturnCallback( + function ( array $products ) { + return array_map( + function ( WC_Product $product ) { + return [ + 'product' => $product, + 'country' => 'US', + 'input' => new ProductInput( "gla_{$product->get_id()}", 'en', 'US', [ 'title' => $product->get_title() ] ), + 'hash' => 'testhash123', + ]; + }, + $products + ); + } + ); + + $this->mapi_inputs->expects( $this->once() ) + ->method( 'insert_many' ) + ->willReturnCallback( + function ( array $inputs ) { + return [ + 'successes' => $inputs, + 'failures' => [], + ]; + } + ); + + $this->get_product_syncer( [ 'batch_helper' => $batch_helper ] )->update( $synced_products ); + + $this->assertEquals( 'testhash123', $this->product_meta->get_sync_hash( $product ) ); + } + + public function test_update_connection_errors_are_retried_not_dropped() { + [ , $rejected_products ] = $this->create_multiple_simple_product_sets( 0, 1 ); + + $batch_helper = $this->getMockBuilder( BatchProductHelper::class ) + ->setMethods( [ 'generate_mapi_update_entries' ] ) + ->setConstructorArgs( + [ + $this->product_meta, + $this->product_helper, + $this->target_audience, + $this->rules_query, + $this->container->get( AttributeManager::class ), + ] + ) + ->getMock(); + $batch_helper->expects( $this->once() ) + ->method( 'generate_mapi_update_entries' ) + ->willReturnCallback( + function ( array $products ) { + return array_map( + function ( WC_Product $product ) { + return [ + 'product' => $product, + 'country' => 'US', + 'input' => new ProductInput( "gla_{$product->get_id()}", 'en', 'US', [ 'title' => $product->get_title() ] ), + ]; + }, + $products + ); + } + ); + + $this->mapi_inputs->expects( $this->once() ) + ->method( 'insert_many' ) + ->willReturnCallback( + function ( array $inputs ) { + $failures = []; + foreach ( array_keys( $inputs ) as $index ) { + $failures[ $index ] = new ConnectException( 'Connection timed out', new Request( 'POST', 'https://example.test' ) ); + } + + return [ + 'successes' => [], + 'failures' => $failures, + ]; + } + ); + + $this->get_product_syncer( [ 'batch_helper' => $batch_helper ] )->update( $rejected_products ); + + // A connection error is transient: the product is rescheduled, not marked permanently invalid. + $this->assertEquals( 1, did_action( 'woocommerce_gla_batch_retry_update_products' ) ); + } + + public function test_update_no_response_transport_errors_are_retried_not_dropped() { + [ , $rejected_products ] = $this->create_multiple_simple_product_sets( 0, 1 ); + + $batch_helper = $this->getMockBuilder( BatchProductHelper::class ) + ->setMethods( [ 'generate_mapi_update_entries' ] ) + ->setConstructorArgs( + [ + $this->product_meta, + $this->product_helper, + $this->target_audience, + $this->rules_query, + $this->container->get( AttributeManager::class ), + ] + ) + ->getMock(); + $batch_helper->expects( $this->once() ) + ->method( 'generate_mapi_update_entries' ) + ->willReturnCallback( + function ( array $products ) { + return array_map( + function ( WC_Product $product ) { + return [ + 'product' => $product, + 'country' => 'US', + 'input' => new ProductInput( "gla_{$product->get_id()}", 'en', 'US', [ 'title' => $product->get_title() ] ), + ]; + }, + $products + ); + } + ); + + $this->mapi_inputs->expects( $this->once() ) + ->method( 'insert_many' ) + ->willReturnCallback( + function ( array $inputs ) { + $failures = []; + foreach ( array_keys( $inputs ) as $index ) { + $failures[ $index ] = new RequestException( 'connection reset', new Request( 'POST', 'https://example.test' ) ); + } + + return [ + 'successes' => [], + 'failures' => $failures, + ]; + } + ); + + $this->get_product_syncer( [ 'batch_helper' => $batch_helper ] )->update( $rejected_products ); + + // A transport error with no HTTP response is transient: the product is rescheduled. + $this->assertEquals( 1, did_action( 'woocommerce_gla_batch_retry_update_products' ) ); + } + public function test_delete() { // $deleted_products: products that were successfully synced and then deleted from Merchant Center // $rejected_products: products that were synced but deleting them resulted in errors and were rejected by Google API From ebe5bbd8362c6462252c8ffc64d02edc2e31f48c Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:58:23 +0200 Subject: [PATCH 14/19] Refactor `MerchantApiClient` to update batch boundary prefix, fix HTTP version in batch requests, and add error logging for unparsable batch responses --- src/API/Google/Mapi/MerchantApiClient.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/API/Google/Mapi/MerchantApiClient.php b/src/API/Google/Mapi/MerchantApiClient.php index d0059053c1..df34137ea9 100644 --- a/src/API/Google/Mapi/MerchantApiClient.php +++ b/src/API/Google/Mapi/MerchantApiClient.php @@ -22,7 +22,7 @@ */ class MerchantApiClient { - private const BATCH_BOUNDARY = 'gla_mapi_batch_boundary'; + private const BATCH_BOUNDARY_PREFIX = 'gla_mapi_batch_boundary'; /** @var ClientInterface */ protected $http; @@ -200,14 +200,14 @@ function ( $reason ) use ( $method_label ) { * @return Request */ protected function build_batch_request( array $requests ): Request { - $boundary = self::BATCH_BOUNDARY . '_' . wp_generate_password( 16, false ); + $boundary = self::BATCH_BOUNDARY_PREFIX . '_' . wp_generate_password( 16, false ); $body = ''; foreach ( $requests as $key => $sub ) { $body .= "--{$boundary}\r\n"; $body .= "Content-Type: application/http\r\n"; $body .= "Content-ID: \r\n\r\n"; - $body .= sprintf( "%s /%s\r\n", $sub['method'], ltrim( $sub['path'], '/' ) ); + $body .= sprintf( "%s /%s HTTP/1.1\r\n", $sub['method'], ltrim( $sub['path'], '/' ) ); if ( isset( $sub['body'] ) ) { $body .= "Content-Type: application/json\r\n\r\n"; @@ -237,6 +237,7 @@ protected function build_batch_request( array $requests ): Request { */ protected function parse_batch_response( ResponseInterface $response ): array { if ( ! preg_match( '/boundary=(?:"([^"]+)"|([^;\s]+))/', $response->getHeaderLine( 'Content-Type' ), $matches ) ) { + do_action( 'woocommerce_gla_error', 'Merchant API batch response had no parseable multipart boundary.', __METHOD__ ); return []; } From a659dc777f29004629f387dfb641d7a1326e5e5d Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:58:35 +0200 Subject: [PATCH 15/19] Fix backoff calculation and improve batch URL matching logic in `GoogleServiceProvider` --- src/Internal/DependencyManagement/GoogleServiceProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Internal/DependencyManagement/GoogleServiceProvider.php b/src/Internal/DependencyManagement/GoogleServiceProvider.php index b348672dd9..8232d08c5c 100644 --- a/src/Internal/DependencyManagement/GoogleServiceProvider.php +++ b/src/Internal/DependencyManagement/GoogleServiceProvider.php @@ -271,7 +271,7 @@ protected function retry_delay( int $retries, ?ResponseInterface $response = nul $backoff = ( 2 ** max( 0, $retries - 1 ) ) * 1000; - return (int) min( $backoff, $max_delay ) + wp_rand( 0, 1000 ); + return (int) min( $backoff + wp_rand( 0, 1000 ), $max_delay ); } /** @@ -289,7 +289,7 @@ protected function is_retryable_request( RequestInterface $request ): bool { $path = $request->getUri()->getPath(); - return false !== strpos( $path, 'productInputs' ) || false !== strpos( $path, 'batch' ); + return false !== strpos( $path, 'productInputs' ) || '/batch' === substr( $path, -6 ); } /** From 126ba6940bb948f5e27ea1c2a61d780c4313d4ef Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:58:46 +0200 Subject: [PATCH 16/19] Add `hash` to `generate_mapi_update_entries` return type in `BatchProductHelper` --- src/Product/BatchProductHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Product/BatchProductHelper.php b/src/Product/BatchProductHelper.php index 3aefb655cc..344825d165 100644 --- a/src/Product/BatchProductHelper.php +++ b/src/Product/BatchProductHelper.php @@ -190,7 +190,7 @@ public function generate_delete_request_entries( array $products ): array { * * @param WC_Product[] $products * - * @return array + * @return array */ public function generate_mapi_update_entries( array $products ): array { $entries = []; From d2d802dc2921a064bd41c4187517cfa6548be789 Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:58:57 +0200 Subject: [PATCH 17/19] Add `hash` field to `$entries` parameter in `ProductSyncer::submit_product_inputs` docblock --- src/Product/ProductSyncer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Product/ProductSyncer.php b/src/Product/ProductSyncer.php index 7a730f94ed..5d5770e093 100644 --- a/src/Product/ProductSyncer.php +++ b/src/Product/ProductSyncer.php @@ -108,7 +108,7 @@ public function update( array $products ): BatchProductResponse { /** * Submit MAPI ProductInput entries to Merchant Center via productInputs.insert. * - * @param array $entries + * @param array $entries * * @return BatchProductResponse Containing both the synced and invalid products. * From 13396f6af257675fd49682b86075e89c68c2951c Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:59:09 +0200 Subject: [PATCH 18/19] Update `MerchantApiClientTest` to fix HTTP version in batch request assertion --- tests/Unit/API/Google/Mapi/MerchantApiClientTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php b/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php index 9d20eae52f..0caaf12c60 100644 --- a/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php +++ b/tests/Unit/API/Google/Mapi/MerchantApiClientTest.php @@ -200,7 +200,7 @@ public function test_batch_sends_multipart_request() { $body = (string) $request->getBody(); $this->assertStringContainsString( 'Content-ID: ', $body ); - $this->assertStringContainsString( 'POST /products/v1/a/productInputs:insert', $body ); + $this->assertStringContainsString( 'POST /products/v1/a/productInputs:insert HTTP/1.1', $body ); $this->assertStringContainsString( '"offerId":"gla_10"', $body ); } From 6f1f1dcca719daae146f11479247ac648d6da65b Mon Sep 17 00:00:00 2001 From: alaca Date: Wed, 15 Jul 2026 13:59:19 +0200 Subject: [PATCH 19/19] Add unit test for `Client` to verify retry logic runs before error handling on the full handler stack --- tests/Unit/API/ClientTest.php | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/Unit/API/ClientTest.php b/tests/Unit/API/ClientTest.php index 48599fff47..b64ecf148a 100644 --- a/tests/Unit/API/ClientTest.php +++ b/tests/Unit/API/ClientTest.php @@ -140,6 +140,28 @@ public function test_retry_on_transient_error_ignores_client_errors() { $client->request( 'GET', 'https://testing.local' ); } + public function test_retry_runs_before_the_error_handler_on_the_full_stack() { + // Real stack order: error_handler pushed first (outermost), retry last (innermost). This + // POST is one is_retryable_request() rejects, so a 429 can only retry via the response + // branch, proving retry sees the response before error_handler throws. + $handler_stack = HandlerStack::create( + new MockHandler( + [ + new Response( 429, [], 'rate limited' ), + new Response( 200, [], 'ok' ), + ] + ) + ); + $handler_stack->remove( 'http_errors' ); + $handler_stack->push( $this->invoke_handler( 'error_handler' ), 'http_errors' ); + $handler_stack->push( $this->invoke_handler( 'retry_on_transient_error' ), 'retry_on_transient_error' ); + + $response = ( new Client( [ 'handler' => $handler_stack ] ) ) + ->request( 'POST', 'https://testing.local/datasources/v1/accounts/1/dataSources' ); + + $this->assertEquals( 200, $response->getStatusCode() ); + } + public function test_retry_on_transient_error_retries_5xx_on_product_insert() { $client = $this->mock_client_with_handler( 'retry_on_transient_error', @@ -224,11 +246,10 @@ public function test_retry_delay_caps_the_backoff() { $method = new ReflectionMethod( GoogleServiceProvider::class, 'retry_delay' ); $method->setAccessible( true ); - // A high retry count must not exceed the cap (plus up to 1s jitter). + // A high retry count is clamped to the 30s cap (jitter is added inside the cap). $delay = $method->invoke( $this->provider, 20, null ); - $this->assertGreaterThanOrEqual( 30000, $delay ); - $this->assertLessThanOrEqual( 31000, $delay ); + $this->assertSame( 30000, $delay ); } public function test_retry_on_transient_error_retries_no_response_transport_errors() {