Avoid reading items from DB of a order whose ID is zero (#46161)

* Add unit test simulating order mix up when order ID is zero.

* Better tests.

* Dont try reading from DB when order is zero to prevent mixups.

* PHPCS fixes and changelog.

* Syntax sugar

Co-authored-by: Corey McKrill <916023+coreymckrill@users.noreply.github.com>

---------

Co-authored-by: Corey McKrill <916023+coreymckrill@users.noreply.github.com>
This commit is contained in:
Vedanshu Jain 2024-04-04 12:52:20 +05:30 committed by GitHub
parent 4bce1da492
commit 3d2b33ca09
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 81 additions and 7 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: fix
Prevent reading items with zero order ID to avoid mixups.

View File

@ -12,6 +12,7 @@ if ( ! defined( 'ABSPATH' ) ) {
exit;
}
// phpcs:disable Squiz.Classes.ClassFileName.NoMatch, Squiz.Classes.ValidClassName.NotCamelCaps -- Backward compatibility.
/**
* Abstract Order Data Store: Stored in CPT.
*
@ -80,6 +81,13 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
}
$id = wp_insert_post(
/**
* Filters the data for a new order before it is inserted into the database.
*
* @param array $data Array of data for the new order.
*
* @since 3.3.0
*/
apply_filters(
'woocommerce_new_order_data',
array(
@ -115,7 +123,7 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
* @param int $order_id The order id to check.
* @return bool True if an order exists with the given name.
*/
public function order_exists( $order_id ) : bool {
public function order_exists( $order_id ): bool {
if ( ! $order_id ) {
return false;
}
@ -135,7 +143,7 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
$order->set_defaults();
$post_object = get_post( $order->get_id() );
if ( ! $order->get_id() || ! $post_object || ! in_array( $post_object->post_type, wc_get_order_types(), true ) ) {
throw new Exception( __( 'Invalid order.', 'woocommerce' ) );
throw new Exception( esc_html__( 'Invalid order.', 'woocommerce' ) );
}
$this->set_order_props(
@ -291,7 +299,7 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
/**
* Fires immediately after an order is deleted.
*
* @since
* @since 2.7.0
*
* @param int $order_id ID of the order that has been deleted.
*/
@ -345,6 +353,13 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
$order_status = $order->get_status( 'edit' );
if ( ! $order_status ) {
/**
* Filters the default order status to use when creating a new order.
*
* @param string $order_status Default order status.
*
* @since 3.7.0
*/
$order_status = apply_filters( 'woocommerce_default_order_status', 'pending' );
}
@ -352,7 +367,7 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
$valid_statuses = get_post_stati();
// Add a wc- prefix to the status, but exclude some core statuses which should not be prefixed.
// @todo In the future this should only happen based on `wc_is_order_status`, but in order to
// In the future this should only happen based on `wc_is_order_status`, but in order to
// preserve back-compatibility this happens to all statuses except a select few. A doing_it_wrong
// Notice will be needed here, followed by future removal.
if ( ! in_array( $post_status, array( 'auto-draft', 'draft', 'trash' ), true ) && in_array( 'wc-' . $post_status, $valid_statuses, true ) ) {
@ -380,7 +395,7 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
protected function get_post_title() {
// @codingStandardsIgnoreStart
/* translators: %s: Order date */
return sprintf( __( 'Order &ndash; %s', 'woocommerce' ), (new DateTime('now'))->format( _x( 'M d, Y @ h:i A', 'Order date parsed by DateTime::format', 'woocommerce' ) ) );
return sprintf( __( 'Order &ndash; %s', 'woocommerce' ), ( new DateTime( 'now' ) )->format( _x( 'M d, Y @ h:i A', 'Order date parsed by DateTime::format', 'woocommerce' ) ) );
// @codingStandardsIgnoreEnd
}
@ -466,6 +481,14 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
}
}
/**
* Action fired after updating order properties.
*
* @param WC_Abstract_Order $order Order object.
* @param string[] $updated_props Array of updated properties.
*
* @since 2.7.0
*/
do_action( 'woocommerce_order_object_updated_props', $order, $updated_props );
}
@ -491,6 +514,11 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme
public function read_items( $order, $type ) {
global $wpdb;
// When the order is not yet saved, we cannot get the items from DB. Trying to do so will risk reading items of different orders that were saved incorrectly.
if ( 0 === $order->get_id() ) {
return array();
}
// Get from cache if available.
$items = 0 < $order->get_id() ? wp_cache_get( 'order-items-' . $order->get_id(), 'orders' ) : false;

View File

@ -7,6 +7,7 @@
use Automattic\WooCommerce\Testing\Tools\CodeHacking\Hacks\FunctionsMockerHack;
// phpcs:disable Squiz.Classes.ClassFileName.NoMatch, Squiz.Classes.ValidClassName.NotCamelCaps -- Backward compatibility.
/**
* Class WC_Abstract_Order.
*/
@ -148,7 +149,7 @@ class WC_Abstract_Order_Test extends WC_Unit_Test_Case {
FunctionsMockerHack::add_function_mocks(
array(
'wc_get_price_excluding_tax' => function( $product, $args = array() ) use ( &$product_passed_to_get_price, &$args_passed_to_get_price ) {
'wc_get_price_excluding_tax' => function ( $product, $args = array() ) use ( &$product_passed_to_get_price, &$args_passed_to_get_price ) {
$product_passed_to_get_price = $product;
$args_passed_to_get_price = $args;
@ -311,7 +312,7 @@ class WC_Abstract_Order_Test extends WC_Unit_Test_Case {
public function test_cache_does_not_interferes_with_order_object() {
add_action(
'woocommerce_new_order',
function( $order_id ) {
function ( $order_id ) {
// this makes the cache store a specific order class instance, but it's quickly replaced by a generic one
// as we're in the middle of a save and this gets executed before the logic in WC_Abstract_Order.
$order = wc_get_order( $order_id );
@ -348,4 +349,45 @@ class WC_Abstract_Order_Test extends WC_Unit_Test_Case {
$order_terms = wp_list_pluck( wp_get_object_terms( $order->get_id(), $custom_taxonomy->name ), 'name' );
$this->assertContains( 'new_term', $order_terms );
}
/**
* @testDox Test that order items are not mixed when order_id is zero.
*/
public function test_order_items_shouldnot_mix_with_zero_id() {
$order1 = new WC_Order();
$order2 = new WC_Order();
$product1_for_order1 = WC_Helper_Product::create_simple_product();
$product2_for_order1 = WC_Helper_Product::create_simple_product();
$product_for_order2 = WC_Helper_Product::create_simple_product();
$item1_1 = new WC_Order_Item_Product();
$item1_1->set_product( $product1_for_order1 );
$item1_1->set_quantity( 1 );
$item1_1->save();
$item1_2 = new WC_Order_Item_Product();
$item1_2->set_product( $product2_for_order1 );
$item1_2->set_quantity( 1 );
$item1_2->save();
$item2 = new WC_Order_Item_Product();
$item2->set_product( $product_for_order2 );
$item2->set_quantity( 1 );
$item2->save();
$order1->add_item( $item1_1 );
$order2->add_item( $item2 );
$order1->add_item( $item1_2 );
$this->assertCount( 1, $order2->get_items( 'line_item' ) );
$this->assertCount( 2, $order1->get_items( 'line_item' ) );
$order1_items = array_keys( $order1->get_items( 'line_item' ) );
$this->assertContains( $item1_1->get_id(), $order1_items );
$this->assertContains( $item1_1->get_id(), $order1_items );
$this->assertEquals( $item2->get_id(), array_keys( $order2->get_items( 'line_item' ) )[0] );
}
}