From b4f5d492fa4273bc2b3fd6893b8183d4f5b8e909 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 19 Feb 2020 14:09:36 +0000 Subject: [PATCH 1/7] Add expires column to table --- src/Library.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Library.php b/src/Library.php index c7330bbbb81..24002395fd1 100644 --- a/src/Library.php +++ b/src/Library.php @@ -72,6 +72,7 @@ public static function maybe_create_tables() { `product_id` bigint(20) NOT NULL, `stock_quantity` double NOT NULL DEFAULT 0, `timestamp` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, + `expires` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, PRIMARY KEY (`order_id`, `product_id`) ) $collate; " From 8154f9a5ec3efdc561766dc069b7808e7a5a6591 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 19 Feb 2020 14:10:24 +0000 Subject: [PATCH 2/7] Use core class if it exists. --- .../StoreApi/Controllers/CartOrder.php | 26 ++++++------------- .../StoreApi/Schemas/CartItemSchema.php | 12 ++++++--- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/RestApi/StoreApi/Controllers/CartOrder.php b/src/RestApi/StoreApi/Controllers/CartOrder.php index 7fc952016be..b2864ca01e0 100644 --- a/src/RestApi/StoreApi/Controllers/CartOrder.php +++ b/src/RestApi/StoreApi/Controllers/CartOrder.php @@ -17,7 +17,6 @@ use \WP_REST_Request as RestRequest; use \WC_REST_Exception as RestException; use Automattic\WooCommerce\Blocks\RestApi\StoreApi\Schemas\OrderSchema; -use Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStock; /** * Cart Order API. @@ -127,9 +126,15 @@ public function create_item( $request ) { // Create or retrieve the draft order for the current cart. $order_object = $this->create_order_from_cart( $request ); - // Try to reserve stock, if available. - $this->reserve_stock( $order_object ); + // Try to reserve stock for 10 mins, if available. + // @todo Remove once min support for WC reaches 4.0.0. + if ( \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) ) { + $reserve_stock = new \Automattic\WooCommerce\Checkout\Helpers\ReserveStock(); + } else { + $reserve_stock = new \Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStock(); + } + $reserve_stock->reserve_stock_for_order( $order_object, 10 ); $response = $this->prepare_item_for_response( $order_object, $request ); $response->set_status( 201 ); return $response; @@ -167,21 +172,6 @@ protected function update_session( RestRequest $request ) { WC()->customer->save(); } - /** - * Put a temporary hold on stock for this order. - * - * @throws RestException Exception when stock cannot be reserved. - * @param \WC_Order $order Order object. - */ - protected function reserve_stock( \WC_Order $order ) { - $reserve_stock_helper = new ReserveStock(); - $result = $reserve_stock_helper->reserve_stock_for_order( $order ); - - if ( is_wp_error( $result ) ) { - throw new RestException( $result->get_error_code(), $result->get_error_message(), $result->get_error_data( 'status' ) ); - } - } - /** * Create order and set props based on global settings. * diff --git a/src/RestApi/StoreApi/Schemas/CartItemSchema.php b/src/RestApi/StoreApi/Schemas/CartItemSchema.php index 9d59e5e96ef..ea339fb92f7 100644 --- a/src/RestApi/StoreApi/Schemas/CartItemSchema.php +++ b/src/RestApi/StoreApi/Schemas/CartItemSchema.php @@ -11,7 +11,6 @@ use Automattic\WooCommerce\Blocks\RestApi\Utilities\ProductImages; use Automattic\WooCommerce\Blocks\RestApi\Utilities\ProductSummary; -use Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStock; /** * CartItemSchema class. @@ -335,8 +334,15 @@ protected function get_low_stock_remaining( \WC_Product $product ) { return null; } - $draft_order = WC()->session->get( 'store_api_draft_order' ); - $reserve_stock = new ReserveStock(); + $draft_order = WC()->session->get( 'store_api_draft_order' ); + + // @todo Remove once min support for WC reaches 4.0.0. + if ( \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) ) { + $reserve_stock = new \Automattic\WooCommerce\Checkout\Helpers\ReserveStock(); + } else { + $reserve_stock = new \Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStock(); + } + $reserved_stock = $reserve_stock->get_reserved_stock( $product, isset( $draft_order['id'] ) ? $draft_order['id'] : 0 ); $remaining_stock = $product->get_stock_quantity() - $reserved_stock; From e94ca02bf060c23513fe11fe23af69326630d650 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 19 Feb 2020 14:10:45 +0000 Subject: [PATCH 3/7] Add Exception class --- .../Utilities/ReserveStockException.php | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 src/RestApi/StoreApi/Utilities/ReserveStockException.php diff --git a/src/RestApi/StoreApi/Utilities/ReserveStockException.php b/src/RestApi/StoreApi/Utilities/ReserveStockException.php new file mode 100644 index 00000000000..b6f8946cf12 --- /dev/null +++ b/src/RestApi/StoreApi/Utilities/ReserveStockException.php @@ -0,0 +1,60 @@ +error_code = $code; + $this->error_data = array_merge( array( 'status' => $http_status_code ), $data ); + + parent::__construct( $message, $http_status_code ); + } + + /** + * Returns the error code. + * + * @return string + */ + public function getErrorCode() { + return $this->error_code; + } + + /** + * Returns error data. + * + * @return array + */ + public function getErrorData() { + return $this->error_data; + } +} From 9d4fe2f96fb587653b79bb12ff61a4e2e648819f Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 19 Feb 2020 14:10:56 +0000 Subject: [PATCH 4/7] Update for currcurrency --- .../StoreApi/Utilities/ReserveStock.php | 239 +++++++++++------- 1 file changed, 147 insertions(+), 92 deletions(-) diff --git a/src/RestApi/StoreApi/Utilities/ReserveStock.php b/src/RestApi/StoreApi/Utilities/ReserveStock.php index 31569b36bd4..a210f69b19e 100644 --- a/src/RestApi/StoreApi/Utilities/ReserveStock.php +++ b/src/RestApi/StoreApi/Utilities/ReserveStock.php @@ -1,6 +1,6 @@ get_var( $this->get_query_for_reserved_stock( $product->get_stock_managed_by_id(), $exclude_order_id ) ); + } + /** * Put a temporary hold on stock for an order if enough is available. * + * @throws ReserveStockException If stock cannot be reserved. + * * @param \WC_Order $order Order object. - * @return bool|WP_Error + * @param int $minutes How long to reserve stock in minutes. Defaults to woocommerce_hold_stock_minutes. */ - public function reserve_stock_for_order( \WC_Order $order ) { - $stock_to_reserve = []; - $items = array_filter( - $order->get_items(), - function( $item ) { - return $item->is_type( 'line_item' ) && $item->get_product() instanceof \WC_Product; - } - ); + public function reserve_stock_for_order( \WC_Order $order, $minutes = 0 ) { + $minutes = $minutes ? $minutes : (int) get_option( 'woocommerce_hold_stock_minutes', 60 ); - foreach ( $items as $item ) { - $product = $item->get_product(); - - if ( ! $product->is_in_stock() ) { - return new WP_Error( - 'product_out_of_stock', - sprintf( - /* translators: %s: product name */ - __( '%s is out of stock and cannot be purchased.', 'woo-gutenberg-products-block' ), - $product->get_name() - ), - [ 'status' => 403 ] - ); - } + if ( ! $minutes ) { + return; + } - // If stock management is off, no need to reserve any stock here. - if ( ! $product->managing_stock() || $product->backorders_allowed() ) { - continue; + try { + $items = array_filter( + $order->get_items(), + function( $item ) { + return $item->is_type( 'line_item' ) && $item->get_product() instanceof \WC_Product && $item->get_quantity() > 0; + } + ); + $rows = []; + + foreach ( $items as $item ) { + $product = $item->get_product(); + + if ( ! $product->is_in_stock() ) { + throw new ReserveStockException( + 'product_out_of_stock', + sprintf( + /* translators: %s: product name */ + __( '%s is out of stock and cannot be purchased.', 'woo-gutenberg-products-block' ), + $product->get_name() + ), + 403 + ); + } + + // If stock management is off, no need to reserve any stock here. + if ( ! $product->managing_stock() || $product->backorders_allowed() ) { + continue; + } + + $managed_by_id = $product->get_stock_managed_by_id(); + $rows[ $managed_by_id ] = isset( $rows[ $managed_by_id ] ) ? $rows[ $managed_by_id ] + $item->get_quantity() : $item->get_quantity(); } - $product_id = $product->get_stock_managed_by_id(); - $stock_to_reserve[ $product_id ] = isset( $stock_to_reserve[ $product_id ] ) ? $stock_to_reserve[ $product_id ] : 0; - $reserved_stock = $this->get_reserved_stock( $product, $order->get_id() ); - - if ( ( $product->get_stock_quantity() - $reserved_stock - $stock_to_reserve[ $product_id ] ) < $item->get_quantity() ) { - return new WP_Error( - 'product_not_enough_stock', - sprintf( - /* translators: %s: product name */ - __( 'Not enough units of %s are available in stock to fulfil this order.', 'woo-gutenberg-products-block' ), - $product->get_name() - ), - [ 'status' => 403 ] - ); + if ( ! empty( $rows ) ) { + foreach ( $rows as $product_id => $quantity ) { + $this->reserve_stock_for_product( $product_id, $quantity, $order, $minutes ); + } } - - // Queue for later DB insertion. - $stock_to_reserve[ $product_id ] += $item->get_quantity(); + } catch ( ReserveStockException $e ) { + $this->release_stock_for_order( $order ); + throw $e; } - - $this->reserve_stock( $stock_to_reserve, $order->get_id() ); - - return true; } /** - * Reserve stock by inserting rows into the DB. + * Release a temporary hold on stock for an order. * - * @param array $stock_to_reserve Array of Product ID => Qty pairs. - * @param integer $order_id Order ID for which to reserve stock. + * @param \WC_Order $order Order object. */ - protected function reserve_stock( $stock_to_reserve, $order_id ) { + public function release_stock_for_order( \WC_Order $order ) { global $wpdb; - $stock_to_reserve = array_filter( $stock_to_reserve ); - - if ( ! $stock_to_reserve ) { - return; - } - - $stock_to_reserve_rows = []; - - foreach ( $stock_to_reserve as $product_id => $stock_quantity ) { - $stock_to_reserve_rows[] = '(' . esc_sql( $order_id ) . ',"' . esc_sql( $product_id ) . '","' . esc_sql( $stock_quantity ) . '")'; - } - - $values = implode( ',', $stock_to_reserve_rows ); - - // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared - $wpdb->query( "REPLACE INTO {$wpdb->wc_reserved_stock} ( order_id, product_id, stock_quantity ) VALUES {$values};" ); + $wpdb->delete( + $wpdb->wc_reserved_stock, + [ + 'order_id' => $order->get_id(), + ] + ); } /** - * Query for any existing holds on stock for this item. + * Reserve stock for a product by inserting rows into the DB. * - * - Can ignore reserved stock for a specific order. - * - Ignores stock for orders which are no longer drafts (assuming real stock reduction was performed). - * - Ignores stock reserved over 10 mins ago. + * @throws ReserveStockException If a row cannot be inserted. * - * @param \WC_Product $product Product to get reserved stock for. - * @param integer $exclude_order_id Optional order to exclude from the results. - * @return integer Amount of stock already reserved. + * @param int $product_id Product ID which is having stock reserved. + * @param int $stock_quantity Stock amount to reserve. + * @param \WC_Order $order Order object which contains the product. + * @param int $minutes How long to reserve stock in minutes. */ - public function get_reserved_stock( \WC_Product $product, $exclude_order_id = 0 ) { + private function reserve_stock_for_product( $product_id, $stock_quantity, \WC_Order $order, $minutes ) { global $wpdb; - $reserved_stock = $wpdb->get_var( + $query_for_stock = $this->get_query_for_stock( $product_id ); + $query_for_reserved_stock = $this->get_query_for_reserved_stock( $product_id, $order->get_id() ); + + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared + $result = $wpdb->query( $wpdb->prepare( " - SELECT SUM( stock_table.`stock_quantity` ) FROM $wpdb->wc_reserved_stock stock_table - LEFT JOIN $wpdb->posts posts ON stock_table.`order_id` = posts.ID - WHERE stock_table.`product_id` = %d - AND posts.post_status = 'wc-checkout-draft' - AND stock_table.`order_id` != %d - AND stock_table.`timestamp` > ( NOW() - INTERVAL 10 MINUTE ) + REPLACE INTO {$wpdb->wc_reserved_stock} ( order_id, product_id, stock_quantity, expires ) + SELECT %d, %d, %d, ( NOW() + INTERVAL %d MINUTE ) from DUAL + WHERE ( $query_for_stock FOR UPDATE ) - ( $query_for_reserved_stock FOR UPDATE ) >= %d ", - $product->get_stock_managed_by_id(), - $exclude_order_id + $order->get_id(), + $product_id, + $stock_quantity, + $minutes, + $stock_quantity ) ); + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared + + if ( ! $result ) { + $product = wc_get_product( $product_id ); + throw new ReserveStockException( + 'product_not_enough_stock', + sprintf( + /* translators: %s: product name */ + __( 'Not enough units of %s are available in stock to fulfil this order.', 'woo-gutenberg-products-block' ), + $product ? $product->get_name() : '#' . $product_id + ), + 403 + ); + } + } - // Deals with legacy stock reservation which the core Woo checkout performs. - $hold_stock_minutes = (int) get_option( 'woocommerce_hold_stock_minutes', 0 ); - $reserved_stock += ( $hold_stock_minutes > 0 ) ? wc_get_held_stock_quantity( $product ) : 0; + /** + * Returns query statement for getting current `_stock` of a product. + * + * @todo Once merged to woo core data store, this method can be removed. + * @internal MAX function below is used to make sure result is a scalar. + * @param int $product_id Product ID. + * @return string|void Query statement. + */ + private function get_query_for_stock( $product_id ) { + global $wpdb; + return $wpdb->prepare( + " + SELECT COALESCE ( MAX( meta_value ), 0 ) FROM $wpdb->postmeta as meta_table + WHERE meta_table.meta_key = '_stock' + AND meta_table.post_id = %d + ", + $product_id + ); + } - return $reserved_stock; + /** + * Returns query statement for getting reserved stock of a product. + * + * @param int $product_id Product ID. + * @param integer $exclude_order_id Optional order to exclude from the results. + * @return string|void Query statement. + */ + private function get_query_for_reserved_stock( $product_id, $exclude_order_id = 0 ) { + global $wpdb; + return $wpdb->prepare( + " + SELECT COALESCE( SUM( stock_table.`stock_quantity` ), 0 ) FROM $wpdb->wc_reserved_stock stock_table + LEFT JOIN $wpdb->posts posts ON stock_table.`order_id` = posts.ID + WHERE posts.post_status IN ( 'wc-checkout-draft', 'wc-pending' ) + AND stock_table.`expires` > NOW() + AND stock_table.`product_id` = %d + AND stock_table.`order_id` != %d + ", + $product_id, + $exclude_order_id + ); } } From 9678054ab4ccc8d4050631ce66220fea258db883 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 19 Feb 2020 14:20:06 +0000 Subject: [PATCH 5/7] Update tests to handle exception --- .../StoreApi/Utilities/ReserveStock.php | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php b/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php index 387252d46b3..6a097a3db4e 100644 --- a/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php +++ b/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php @@ -32,8 +32,7 @@ public function test_reserve_stock_for_order() { $order->set_status( 'checkout-draft' ); $order->save(); - $result = $class->reserve_stock_for_order( $order ); - $this->assertTrue( $result ); + $class->reserve_stock_for_order( $order ); $this->assertEquals( 4, $this->get_reserved_stock_by_product_id( $product->get_stock_managed_by_id() ) ); // Repeat. @@ -41,17 +40,38 @@ public function test_reserve_stock_for_order() { $order->set_status( 'checkout-draft' ); $order->save(); - $result = $class->reserve_stock_for_order( $order ); - $this->assertTrue( $result ); + $class->reserve_stock_for_order( $order ); $this->assertEquals( 8, $this->get_reserved_stock_by_product_id( $product->get_stock_managed_by_id() ) ); + } - // Repeat again - should not be enough stock for this. - $order = OrderHelper::create_order( 1, $product ); + /** + * Test that stock is reserved for draft orders. + * + * @expectedException Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStockException + */ + public function test_reserve_stock_for_order_throws_exception() { + $class = new ReserveStock(); + + $product = ProductHelper::create_simple_product(); + $product->set_manage_stock( true ); + $product->set_stock( 10 ); + $product->save(); + + $order = OrderHelper::create_order( 1, $product ); // Note this adds 4 to the order. $order->set_status( 'checkout-draft' ); $order->save(); - $result = $class->reserve_stock_for_order( $order ); - $this->assertTrue( is_wp_error( $result ) ); + $order2 = OrderHelper::create_order( 1, $product ); + $order2->set_status( 'checkout-draft' ); + $order2->save(); + + $order3 = OrderHelper::create_order( 1, $product ); + $order3->set_status( 'checkout-draft' ); + $order3->save(); + + $class->reserve_stock_for_order( $order ); + $class->reserve_stock_for_order( $order2 ); + $class->reserve_stock_for_order( $order3 ); } /** From 754c055ff92ddc168b16414ccf2430a4c375df36 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 4 Mar 2020 14:25:17 +0000 Subject: [PATCH 6/7] Add back legacy stock handling --- src/RestApi/StoreApi/Utilities/ReserveStock.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/RestApi/StoreApi/Utilities/ReserveStock.php b/src/RestApi/StoreApi/Utilities/ReserveStock.php index a210f69b19e..b1a9afb0f4b 100644 --- a/src/RestApi/StoreApi/Utilities/ReserveStock.php +++ b/src/RestApi/StoreApi/Utilities/ReserveStock.php @@ -118,6 +118,14 @@ private function reserve_stock_for_product( $product_id, $stock_quantity, \WC_Or $query_for_stock = $this->get_query_for_stock( $product_id ); $query_for_reserved_stock = $this->get_query_for_reserved_stock( $product_id, $order->get_id() ); + $required_stock = $stock_quantity; + + // Deals with legacy stock reservations from woo core. + $support_legacy_held_stock = ! \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) && absint( get_option( 'woocommerce_hold_stock_minutes', 0 ) ) > 0; + + if ( $support_legacy_held_stock ) { + $required_stock += wc_get_held_stock_quantity( wc_get_product( $product_id ) ); + } // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared $result = $wpdb->query( @@ -131,7 +139,7 @@ private function reserve_stock_for_product( $product_id, $stock_quantity, \WC_Or $product_id, $stock_quantity, $minutes, - $stock_quantity + $required_stock ) ); // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared From 68f1a714c6e5020545843046401a83151b8cfffd Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Fri, 6 Mar 2020 11:39:16 +0000 Subject: [PATCH 7/7] Update comments --- tests/php/RestApi/StoreApi/Utilities/ReserveStock.php | 2 +- tests/php/RestApi/Utilities/ProductSummary.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php b/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php index 6a097a3db4e..25f728529f3 100644 --- a/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php +++ b/tests/php/RestApi/StoreApi/Utilities/ReserveStock.php @@ -45,7 +45,7 @@ public function test_reserve_stock_for_order() { } /** - * Test that stock is reserved for draft orders. + * Test that trying to reserve stock too much throws an exception. * * @expectedException Automattic\WooCommerce\Blocks\RestApi\StoreApi\Utilities\ReserveStockException */ diff --git a/tests/php/RestApi/Utilities/ProductSummary.php b/tests/php/RestApi/Utilities/ProductSummary.php index bc0e78500e4..c172d5cf739 100644 --- a/tests/php/RestApi/Utilities/ProductSummary.php +++ b/tests/php/RestApi/Utilities/ProductSummary.php @@ -24,7 +24,7 @@ public function tearDown() { } /** - * Test that stock is reserved for draft orders. + * Test that we can get a valid summary. */ public function test_get_summary() { $product = new \WC_Product();