Ensuring only single product creation with unique SKU for concurrent requests

plugins/woocommerce/changelog/sku-check-before-product-creation

@@ -0,0 +1,4 @@
+Significance: patch
+Type: update
+
+Ensuring product creation with unique sku for concurrent requests

plugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.php

@@ -96,6 +96,46 @@ class WC_Product_Data_Store_CPT extends WC_Data_Store_WP implements WC_Object_Da
 	 */
 	protected $updated_props = array();
 
+
+	/**
+	 * Method to obtain DB lock on SKU to make sure we only
+	 * create product with unique SKU for concurrent requests.
+	 *
+	 * We are doing so by inserting a row in the wc_product_meta_lookup table
+	 * upfront with the SKU of the product we are trying to insert.
+	 *
+	 * If the SKU is already present in the table, it means that another
+	 * request is processing the same SKU and we should not proceed
+	 * with the insert.
+	 *
+	 * Using $wpdb->options as it always has some data, if we select from a table
+	 * that does not have any data, then our query will always return null set
+	 * and the where subquery won't be fired, effectively bypassing any lock.
+	 *
+	 * @param WC_Product $product Product object.
+	 * @return bool True if lock is obtained (unique SKU), false otherwise.
+	 */
+	private function obtain_lock_on_sku_for_concurrent_requests( $product ) {
+		global $wpdb;
+		$product_id = $product->get_id();
+		$sku        = $product->get_sku();
+
+		$query = $wpdb->prepare(
+			"INSERT INTO $wpdb->wc_product_meta_lookup (product_id, sku)
+			SELECT %d, %s FROM $wpdb->options
+			WHERE NOT EXISTS (
+				SELECT * FROM $wpdb->wc_product_meta_lookup WHERE sku = %s LIMIT 1
+			) LIMIT 1;",
+			$product_id,
+			$sku,
+			$sku
+		);
+		// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
+		$result = $wpdb->query( $query );
+
+		return (bool) $result;
+	}
+
 	/*
 	|--------------------------------------------------------------------------
 	| CRUD Methods
@@ -106,6 +146,7 @@ class WC_Product_Data_Store_CPT extends WC_Data_Store_WP implements WC_Object_Da
 	 * Method to create a new product in the database.
 	 *
 	 * @param WC_Product $product Product object.
+	 * @throws Exception If SKU is already under processing.
 	 */
 	public function create( &$product ) {
 		if ( ! $product->get_date_created( 'edit' ) ) {
@@ -137,6 +178,19 @@ public function create( &$product ) {
 
 		if ( $id && ! is_wp_error( $id ) ) {
 			$product->set_id( $id );
+			$sku = $product->get_sku();
+
+			/**
+			 * If SKU is already under processing aka Duplicate SKU
+			 * because of concurrent requests, then we should not proceed
+			 * Delete the product and throw an exception only if the request is
+			 * initiated via REST API
+			 */
+			if ( ! empty( $sku ) && WC()->is_rest_api_request() && ! $this->obtain_lock_on_sku_for_concurrent_requests( $product ) ) {
+				$product->delete( true );
+				// translators: 1: SKU.
+				throw new Exception( esc_html( sprintf( __( 'The SKU (%1$s) you are trying to insert is already under processing', 'woocommerce' ), $sku ) ) );
+			}
 
 			$this->update_post_meta( $product, true );
 			$this->update_terms( $product, true );

plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-crud-controller.php

@@ -166,7 +166,21 @@ protected function save_object( $request, $creating = false ) {
 				return $object;
 			}
 
-			$object->save();
+			try {
+				$object->save();
+			} catch ( Exception $e ) {
+				$error = "woocommerce_rest_{$this->post_type}_not_created";
+
+				wc_get_logger()->error(
+					$e->getMessage(),
+					array(
+						'source' => 'woocommerce-rest-api',
+						'error'  => $error,
+						'code'   => 400,
+					)
+				);
+				return new WP_Error( $error, $e->getMessage(), array( 'status' => 400 ) );
+			}
 
 			return $this->get_object( $object->get_id() );
 		} catch ( WC_Data_Exception $e ) {

plugins/woocommerce/tests/legacy/framework/helpers/class-wc-helper-product.php

@@ -12,6 +12,13 @@
  */
 class WC_Helper_Product {
 
+	/**
+	 * Counter to insert unique SKU for concurrent tests.
+	 *
+	 * @var int $sku_counter
+	 */
+	private static $sku_counter = 0;
+
 	/**
 	 * Delete a product.
 	 *
@@ -39,7 +46,7 @@ public static function create_simple_product( $save = true, $props = array() ) {
 				'name'          => 'Dummy Product',
 				'regular_price' => 10,
 				'price'         => 10,
-				'sku'           => 'DUMMY SKU',
+				'sku'           => 'DUMMY SKU' . self::$sku_counter,
 				'manage_stock'  => false,
 				'tax_status'    => 'taxable',
 				'downloadable'  => false,
@@ -48,6 +55,8 @@ public static function create_simple_product( $save = true, $props = array() ) {
 				'weight'        => '1.1',
 			);
 
+		++self::$sku_counter;
+
 		$product->set_props( array_merge( $default_props, $props ) );
 
 		if ( $save ) {

plugins/woocommerce/tests/legacy/unit-tests/product/product-simple.php

@@ -65,7 +65,7 @@ public function test_get_permalink() {
 	 * @since 2.3
 	 */
 	public function test_get_sku() {
-		$this->assertEquals( 'DUMMY SKU', $this->product->get_sku() );
+		$this->assertMatchesRegularExpression( '/^DUMMY SKU\d+$/', $this->product->get_sku() );
 	}
 
 	/**

plugins/woocommerce/tests/legacy/unit-tests/rest-api/Helpers/ProductHelper.php

@@ -22,6 +22,13 @@
  */
 class ProductHelper {
 
+	/**
+	 * Counter to insert unique SKU for concurrent tests.
+	 *
+	 * @var int $sku_counter
+	 */
+	private static $sku_counter = 0;
+
 	/**
 	 * Delete a product.
 	 *
@@ -48,7 +55,7 @@ public static function create_simple_product( $save = true ) {
 				'name'          => 'Dummy Product',
 				'regular_price' => 10,
 				'price'         => 10,
-				'sku'           => 'DUMMY SKU',
+				'sku'           => 'DUMMY SKU' . self::$sku_counter,
 				'manage_stock'  => false,
 				'tax_status'    => 'taxable',
 				'downloadable'  => false,
@@ -58,6 +65,8 @@ public static function create_simple_product( $save = true ) {
 			)
 		);
 
+		++self::$sku_counter;
+
 		if ( $save ) {
 			$product->save();
 			return \wc_get_product( $product->get_id() );

plugins/woocommerce/tests/legacy/unit-tests/rest-api/Tests/Version2/products.php

@@ -57,7 +57,7 @@ public function test_get_products() {
 
 		$this->assertEquals( 2, count( $products ) );
 		$this->assertEquals( 'Dummy Product', $products[0]['name'] );
-		$this->assertEquals( 'DUMMY SKU', $products[0]['sku'] );
+		$this->assertMatchesRegularExpression( '/^DUMMY SKU\d+$/', $products[0]['sku'] );
 		$this->assertEquals( 'Dummy External Product', $products[1]['name'] );
 		$this->assertEquals( 'DUMMY EXTERNAL SKU', $products[1]['sku'] );
 	}
@@ -171,7 +171,7 @@ public function test_update_product() {
 		$response = $this->server->dispatch( new WP_REST_Request( 'GET', '/wc/v2/products/' . $product->get_id() ) );
 		$data     = $response->get_data();
 
-		$this->assertEquals( 'DUMMY SKU', $data['sku'] );
+		$this->assertMatchesRegularExpression( '/^DUMMY SKU\d+$/', $data['sku'] );
 		$this->assertEquals( 10, $data['regular_price'] );
 		$this->assertEmpty( $data['sale_price'] );
 

plugins/woocommerce/tests/legacy/unit-tests/rest-api/Tests/Version3/products.php

@@ -61,7 +61,7 @@ public function test_get_products() {
 
 		$this->assertEquals( 2, count( $products ) );
 		$this->assertEquals( 'Dummy Product', $products[0]['name'] );
-		$this->assertEquals( 'DUMMY SKU', $products[0]['sku'] );
+		$this->assertMatchesRegularExpression( '/^DUMMY SKU\d+$/', $products[0]['sku'] );
 		$this->assertEquals( 'Dummy External Product', $products[1]['name'] );
 		$this->assertEquals( 'DUMMY EXTERNAL SKU', $products[1]['sku'] );
 	}
@@ -228,7 +228,7 @@ public function test_update_product() {
 		$data         = $response->get_data();
 		$date_created = gmdate( 'Y-m-d\TH:i:s', current_time( 'timestamp' ) );
 
-		$this->assertEquals( 'DUMMY SKU', $data['sku'] );
+		$this->assertMatchesRegularExpression( '/^DUMMY SKU\d+$/', $data['sku'] );
 		$this->assertEquals( 10, $data['regular_price'] );
 		$this->assertEmpty( $data['sale_price'] );
 

plugins/woocommerce/tests/php/includes/data-stores/class-wc-product-data-store-cpt-test.php

@@ -66,4 +66,52 @@ public function test_rating_counts_are_summed_correctly(): void {
 		$product = wc_get_product( $product->get_id() );
 		$this->assertEquals( 10, $product->get_rating_count(), 'The product rating count is the expected value.' );
 	}
+
+	/**
+	 * Test that only one product is created with a unique SKU
+	 * during concurrent requests and when request is initiated via REST API.
+	 *
+	 * Throw error when two concurrent requests try to create a product with the same SKU.
+	 *
+	 * @return void
+	 */
+	public function test_create_product_with_unique_sku_on_concurrent_requests() {
+		$this->expectException(
+			'Exception',
+		);
+		$this->expectExceptionMessage(
+			'The SKU (DUMMY SKU) you are trying to insert is already under processing'
+		);
+
+		// exception is only thrown during the REST API request.
+		$_SERVER['REQUEST_URI'] = '/wp-json/wc/v3/products';
+		$this->create_products_concurrently();
+	}
+
+	/**
+	 * Helper function to create products concurrently with same SKU
+	 *
+	 * @return void
+	 */
+	private static function create_products_concurrently() {
+		$default_props =
+			array(
+				'name'          => 'Dummy Product',
+				'regular_price' => 10,
+				'price'         => 10,
+				'sku'           => 'DUMMY SKU',
+			);
+
+		$product1 = new WC_Product_Simple();
+		$product2 = new WC_Product_Simple();
+		$product3 = new WC_Product_Simple();
+
+		$product1->set_props( $default_props );
+		$product2->set_props( $default_props );
+		$product3->set_props( $default_props );
+
+		$product1->save();
+		$product2->save();
+		$product3->save();
+	}
 }

plugins/woocommerce/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php

@@ -11,6 +11,13 @@
  */
 class FiltererTest extends \WC_Unit_Test_Case {
 
+	/**
+	 * Counter to insert unique SKU for concurrent tests.
+	 *
+	 * @var int $sku_counter
+	 */
+	private static $sku_counter = 0;
+
 	/**
 	 * Runs before all the tests in the class.
 	 */
@@ -161,14 +168,16 @@ private function create_product_core( $class_name, $product_attributes ) {
 				'name'          => 'Product',
 				'regular_price' => 1,
 				'price'         => 1,
-				'sku'           => 'DUMMY SKU',
+				'sku'           => 'DUMMY SKU' . self::$sku_counter,
 				'manage_stock'  => false,
 				'tax_status'    => 'taxable',
 				'downloadable'  => false,
 				'virtual'       => false,
 			)
 		);
 
+		++self::$sku_counter;
+
 		$product->set_attributes( $attributes );
 
 		return $product;