Commit 0769182751a for woocommerce
commit 0769182751a42f02c2447a595a2a1191608d4e17
Author: Oleksandr Aratovskyi <79862886+oaratovskyi@users.noreply.github.com>
Date: Fri Oct 2 11:46:25 2026 +0300
Share product ID extraction between factory and scheduled sales (#68945)
* refactor(products): share product ID extraction
PR #68016 added a separate parser for scheduled sale data-store rows. The core query returns IDs, while the parser handled malformed values that core cannot produce.
Move the factory extraction behavior into ProductUtil and reuse it in scheduled sales. Keep the false-row guard so the global post cannot be selected. Remove tests for artificial malformed rows.
Refs #68016
* chore(changelog): note shared product ID extraction
The WooCommerce plugin requires a changelog entry for shipped changes.
Record the development-only product ID extraction refactor as a patch entry.
Refs #68016
* refactor(products): Preserve factory product ID resolver
Product ID extraction moved from the factory to ProductUtil.
Removing the factory resolver also removes an entry point available to reflection-based consumers.
Keep the private method as a delegate and route product loading through it, preserving the existing call path.
* test(products): Restore scheduled sale reference coverage
ScheduledSaleRun accepts product and post references as well as numeric IDs.
The refactor removed the test that checked their saved prices, while the new utility test only checks ID extraction.
Restore the end-to-end assertion for each accepted reference shape.
* Keep global post lookup in the factory and return int product IDs
* Move product factory global post test out of the legacy suite
* Update plugins/woocommerce/src/Internal/Utilities/ProductUtil.php
Co-authored-by: Jorge A. Torres <jorge.torres@automattic.com>
* Update plugins/woocommerce/src/Internal/Utilities/ProductUtil.php
Co-authored-by: Jorge A. Torres <jorge.torres@automattic.com>
* Update plugins/woocommerce/includes/class-wc-product-factory.php
Co-authored-by: Jorge A. Torres <jorge.torres@automattic.com>
---------
Co-authored-by: Jorge A. Torres <jorge.torres@automattic.com>
diff --git a/plugins/woocommerce/changelog/refactor-shared-product-id-extraction b/plugins/woocommerce/changelog/refactor-shared-product-id-extraction
new file mode 100644
index 00000000000..5dfdf22e0a1
--- /dev/null
+++ b/plugins/woocommerce/changelog/refactor-shared-product-id-extraction
@@ -0,0 +1,3 @@
+Significance: patch
+Type: dev
+Comment: Share product ID extraction between the product factory and scheduled sales.
diff --git a/plugins/woocommerce/includes/class-wc-product-factory.php b/plugins/woocommerce/includes/class-wc-product-factory.php
index 8ccf7d6b84e..7085d9073b3 100644
--- a/plugins/woocommerce/includes/class-wc-product-factory.php
+++ b/plugins/woocommerce/includes/class-wc-product-factory.php
@@ -9,6 +9,7 @@
*/
use Automattic\WooCommerce\Internal\Caches\ProductCache;
+use Automattic\WooCommerce\Internal\Utilities\ProductUtil;
use Automattic\WooCommerce\Enums\ProductType;
defined( 'ABSPATH' ) || exit;
@@ -124,25 +125,20 @@ class WC_Product_Factory {
}
/**
- * Get the product ID depending on what was passed.
+ * Get the product ID from a product reference.
*
- * @since 3.0.0
- * @param WC_Product|WP_Post|int|bool $product Product instance, post instance, numeric or false to use global $post.
- * @return int|bool false on failure
+ * @since 3.0.0
+ *
+ * @param mixed $product Product instance, post instance, numeric or false to use global $post.
+ * @return int Product ID, or 0 when no ID is available.
*/
- private function get_product_id( $product ) {
+ private function get_product_id( $product ): int {
global $post;
if ( false === $product && isset( $post, $post->ID ) && 'product' === get_post_type( $post->ID ) ) {
- return absint( $post->ID );
- } elseif ( is_numeric( $product ) ) {
- return $product;
- } elseif ( $product instanceof WC_Product ) {
- return $product->get_id();
- } elseif ( ! empty( $product->ID ) ) {
- return $product->ID;
- } else {
- return false;
+ $product = $post;
}
+
+ return wc_get_container()->get( ProductUtil::class )->get_product_id( $product );
}
}
diff --git a/plugins/woocommerce/src/Internal/ScheduledSaleRun.php b/plugins/woocommerce/src/Internal/ScheduledSaleRun.php
index 5cb8725a7a2..db391efed7f 100644
--- a/plugins/woocommerce/src/Internal/ScheduledSaleRun.php
+++ b/plugins/woocommerce/src/Internal/ScheduledSaleRun.php
@@ -11,7 +11,6 @@ use Automattic\WooCommerce\Internal\Caches\ProductCache;
use Automattic\WooCommerce\Internal\Caches\ProductCacheController;
use Automattic\WooCommerce\Internal\Utilities\ProductUtil;
use Automattic\WooCommerce\Utilities\FeaturesUtil;
-use WC_Product;
/**
* Starts or ends scheduled sales, processing and releasing one batch at a time.
@@ -89,17 +88,17 @@ class ScheduledSaleRun {
throw new \InvalidArgumentException( 'Scheduled sale mode must be either start or end.' );
}
- $this->mode = $mode;
- $this->product_ids = array();
+ $this->mode = $mode;
+ $this->product_ids = array();
+ $this->product_util = wc_get_container()->get( ProductUtil::class );
foreach ( $entries as $entry ) {
- $product_id = $this->normalize_entry( $entry );
- if ( null !== $product_id ) {
+ $product_id = $this->product_util->get_product_id( $entry );
+ if ( $product_id > 0 ) {
$this->product_ids[ $product_id ] = $product_id;
}
}
- $this->product_util = wc_get_container()->get( ProductUtil::class );
$this->product_cache = FeaturesUtil::feature_is_enabled( ProductCacheController::FEATURE_NAME )
? wc_get_container()->get( ProductCache::class )
: null;
@@ -118,32 +117,6 @@ class ScheduledSaleRun {
}
}
- /**
- * Normalize a data-store row to a positive product ID.
- *
- * @param mixed $entry Product reference returned by the data store.
- * @return int|null Positive product ID, or null for an invalid row.
- */
- private function normalize_entry( $entry ): ?int {
- if ( $entry instanceof WC_Product ) {
- $entry = $entry->get_id();
- } elseif ( is_object( $entry ) ) {
- $entry = $entry->ID ?? null;
- }
-
- if ( is_int( $entry ) ) {
- $product_id = $entry;
- } elseif ( is_float( $entry ) && is_finite( $entry ) && floor( $entry ) === $entry ) {
- $product_id = (int) $entry;
- } elseif ( is_string( $entry ) && ctype_digit( $entry ) ) {
- $product_id = (int) $entry;
- } else {
- return null;
- }
-
- return $product_id > 0 ? $product_id : null;
- }
-
/**
* Prime, process, and release one batch.
*
diff --git a/plugins/woocommerce/src/Internal/Utilities/ProductUtil.php b/plugins/woocommerce/src/Internal/Utilities/ProductUtil.php
index b90e79d744a..ed1877b7163 100644
--- a/plugins/woocommerce/src/Internal/Utilities/ProductUtil.php
+++ b/plugins/woocommerce/src/Internal/Utilities/ProductUtil.php
@@ -23,6 +23,26 @@ class ProductUtil {
*/
public const OUTOFSTOCK_COUNT_TRANSIENT = 'wc_outofstock_count';
+ /**
+ * Get a product ID from a product, post, or numeric value.
+ *
+ * @since 11.3.0
+ *
+ * @param mixed $product Product instance, post instance, or numeric ID.
+ * @return int Product ID, or 0 when no ID is available.
+ */
+ public function get_product_id( $product ): int {
+ if ( is_numeric( $product ) ) {
+ return (int) $product;
+ } elseif ( $product instanceof \WC_Product ) {
+ return (int) $product->get_id();
+ } elseif ( is_object( $product ) && ! empty( $product->ID ) ) {
+ return (int) $product->ID;
+ } else {
+ return 0;
+ }
+ }
+
/**
* Delete all product transients for a set of products.
*
diff --git a/plugins/woocommerce/tests/php/includes/class-wc-product-factory-test.php b/plugins/woocommerce/tests/php/includes/class-wc-product-factory-test.php
new file mode 100644
index 00000000000..b78f617e8bb
--- /dev/null
+++ b/plugins/woocommerce/tests/php/includes/class-wc-product-factory-test.php
@@ -0,0 +1,28 @@
+<?php
+declare( strict_types = 1 );
+
+/**
+ * Tests for WC_Product_Factory.
+ */
+class WC_Product_Factory_Test extends WC_Unit_Test_Case {
+ /**
+ * @testdox get_product uses the global product post when no product is passed.
+ */
+ public function test_get_product_uses_global_product_post(): void {
+ $test_product = WC_Helper_Product::create_simple_product();
+ $page_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
+ $old_post = $GLOBALS['post'] ?? null;
+
+ try {
+ $GLOBALS['post'] = get_post( $test_product->get_id() ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restored in finally.
+ $product = WC()->product_factory->get_product();
+ $this->assertInstanceOf( WC_Product::class, $product );
+ $this->assertSame( $test_product->get_id(), $product->get_id() );
+
+ $GLOBALS['post'] = get_post( $page_id ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restored in finally.
+ $this->assertFalse( WC()->product_factory->get_product(), 'A non-product global post must not resolve to a product.' );
+ } finally {
+ $GLOBALS['post'] = $old_post; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restore the prior global.
+ }
+ }
+}
diff --git a/plugins/woocommerce/tests/php/src/Internal/ScheduledSaleRunTest.php b/plugins/woocommerce/tests/php/src/Internal/ScheduledSaleRunTest.php
index 8a9de5fa5ee..07f81c6609a 100644
--- a/plugins/woocommerce/tests/php/src/Internal/ScheduledSaleRunTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/ScheduledSaleRunTest.php
@@ -33,6 +33,22 @@ class ScheduledSaleRunTest extends WC_Unit_Test_Case {
( new ScheduledSaleRun( $rows, $mode ) )->process();
}
+ /**
+ * @testdox A false data-store row does not select the global product post.
+ */
+ public function test_false_row_does_not_select_global_product(): void {
+ $product = WC_Helper_Product::create_missed_sale_end_product();
+ $old_post = $GLOBALS['post'] ?? null;
+
+ try {
+ $GLOBALS['post'] = get_post( $product->get_id() ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restored in finally.
+ $this->process_run( array( false ), ScheduledSaleRun::MODE_END );
+ $this->assertEquals( 50, get_post_meta( $product->get_id(), '_price', true ), 'A false row must not settle the global product.' );
+ } finally {
+ $GLOBALS['post'] = $old_post; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restore the prior global.
+ }
+ }
+
/**
* @testdox Starting a sale stores the sale price as the active price.
*/
@@ -259,7 +275,7 @@ class ScheduledSaleRunTest extends WC_Unit_Test_Case {
}
/**
- * @testdox Supported data-store result shapes still settle.
+ * @testdox Accepted product references still settle scheduled sales.
*/
public function test_settles_supported_data_store_result_shapes(): void {
$ids = array();
@@ -274,7 +290,7 @@ class ScheduledSaleRunTest extends WC_Unit_Test_Case {
get_post( $ids[2] ),
);
- $this->process_run( $rows, 'end' );
+ $this->process_run( $rows, ScheduledSaleRun::MODE_END );
foreach ( $ids as $index => $id ) {
$this->assertEquals(
@@ -285,43 +301,6 @@ class ScheduledSaleRunTest extends WC_Unit_Test_Case {
}
}
- /**
- * Malformed data-store rows that could cast onto an unrelated post.
- *
- * @return array
- */
- public function provider_malformed_rows(): array {
- return array(
- 'string with trailing junk' => array( fn( int $id ) => $id . 'abc' ),
- 'fractional float' => array( fn( int $id ) => $id + 0.9 ),
- 'fractional string' => array( fn( int $id ) => $id . '.9' ),
- 'object with junk ID' => array( fn( int $id ) => (object) array( 'ID' => $id . 'abc' ) ),
- 'object with fractional ID' => array( fn( int $id ) => (object) array( 'ID' => $id + 0.9 ) ),
- 'object without ID' => array( fn( int $id ) => (object) array( 'id' => $id ) ),
- 'negative int' => array( fn( int $id ) => -$id ),
- 'exponent string' => array( fn( int $id ) => $id . 'e0' ),
- );
- }
-
- /**
- * @testdox A malformed row does not stop the run or evict an unrelated post.
- * @dataProvider provider_malformed_rows
- *
- * @param callable $make_row Builds the malformed row from the decoy post id.
- */
- public function test_drops_malformed_rows( callable $make_row ): void {
- $product = WC_Helper_Product::create_missed_sale_end_product();
- $decoy_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
- $row = $make_row( $decoy_id );
- get_post( $decoy_id );
- $this->assertNotFalse( wp_cache_get( $decoy_id, 'posts' ), 'Fixture precondition: the decoy page should be primed before the run.' );
-
- $this->process_run( array( $product->get_id(), $row ), 'end' );
-
- $this->assertEquals( 100, get_post_meta( $product->get_id(), '_price', true ), 'A malformed row stopped the run before the real product settled.' );
- $this->assertNotFalse( wp_cache_get( $decoy_id, 'posts' ), 'The release cast ' . wp_json_encode( $row ) . " onto post {$decoy_id} and evicted an unrelated page." );
- }
-
/**
* @testdox An id listed twice in one batch is processed once.
*/
diff --git a/plugins/woocommerce/tests/php/src/Internal/Utilities/ProductUtilTest.php b/plugins/woocommerce/tests/php/src/Internal/Utilities/ProductUtilTest.php
index 45c99ed58e1..498d3ac387f 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Utilities/ProductUtilTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Utilities/ProductUtilTest.php
@@ -16,6 +16,46 @@ use Automattic\WooCommerce\StoreApi\Utilities\ProductQuery;
* Tests for the internal ProductUtil class.
*/
class ProductUtilTest extends \WC_Unit_Test_Case {
+ /**
+ * @testdox get_product_id extracts IDs from the product references accepted by the factory.
+ */
+ public function test_get_product_id_extracts_product_references(): void {
+ $product = \WC_Helper_Product::create_simple_product();
+ $sut = wc_get_container()->get( ProductUtil::class );
+
+ $this->assertSame( $product->get_id(), $sut->get_product_id( $product ) );
+ $this->assertSame( $product->get_id(), $sut->get_product_id( get_post( $product->get_id() ) ) );
+ $this->assertSame( $product->get_id(), $sut->get_product_id( (string) $product->get_id() ) );
+ }
+
+ /**
+ * @testdox get_product_id returns 0 for values without a product ID.
+ */
+ public function test_get_product_id_returns_zero_without_id(): void {
+ $sut = wc_get_container()->get( ProductUtil::class );
+
+ $this->assertSame( 0, $sut->get_product_id( null ) );
+ $this->assertSame( 0, $sut->get_product_id( 'abc' ) );
+ $this->assertSame( 0, $sut->get_product_id( array( 'ID' => 1 ) ) );
+ $this->assertSame( 0, $sut->get_product_id( (object) array( 'id' => 1 ) ) );
+ }
+
+ /**
+ * @testdox get_product_id ignores the global post when passed false.
+ */
+ public function test_get_product_id_ignores_global_post_for_false(): void {
+ $product = \WC_Helper_Product::create_simple_product();
+ $sut = wc_get_container()->get( ProductUtil::class );
+ $old_post = $GLOBALS['post'] ?? null;
+
+ try {
+ $GLOBALS['post'] = get_post( $product->get_id() ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restored in finally.
+ $this->assertSame( 0, $sut->get_product_id( false ) );
+ } finally {
+ $GLOBALS['post'] = $old_post; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Restore the prior global.
+ }
+ }
+
/**
* @testdox `get_counts_for_type` returns per-status counts for the given post type.
*/