Skip to content

Commit

Permalink
Overwrite clone method to prevent duplicate data when saving a clone. (
Browse files Browse the repository at this point in the history
…#37313)

* Add unit test to simulate duplicate meta insert.

* Overwrite clone method to prevent duplicate datq when saving a clone.

* Add changelog.

* Coding standard fixes.

* Fix phpcs

---------

Co-authored-by: Jorge A. Torres <jorge.torres@automattic.com>
  • Loading branch information
2 people authored and WooCommerce Bot committed Mar 22, 2023
1 parent aa3e560 commit 4fe077b
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 0 deletions.
4 changes: 4 additions & 0 deletions plugins/woocommerce/changelog/fix-order-cache-duplicate
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Overwrite clone method to prevent duplicate datq when saving a clone.
11 changes: 11 additions & 0 deletions plugins/woocommerce/includes/abstracts/abstract-wc-order.php
Expand Up @@ -131,6 +131,17 @@ public function __construct( $order = 0 ) {
}
}

/**
* This method overwrites the base class's clone method to make it a no-op. In base class WC_Data, we are unsetting the meta_id to clone.
* It seems like this was done to avoid conflicting the metadata when duplicating products. However, doing that does not seems necessary for orders.
* In-fact, when we do that for orders, we lose the capability to clone orders with custom meta data by caching plugins. This is because, when we clone an order object for caching, it will clone the metadata without the ID. Unfortunately, when this cached object with nulled meta ID is retreived, WC_Data will consider it as a new meta and will insert it as a new meta-data causing duplicates.
*
* Eventually, we should move away from overwriting the __clone method in base class itself, since it's easily possible to still duplicate the product without having to hook into the __clone method.
*
* @since 7.6.0
*/
public function __clone() {}

/**
* Get internal type.
*
Expand Down
1 change: 1 addition & 0 deletions plugins/woocommerce/src/Caches/OrderCache.php
Expand Up @@ -2,6 +2,7 @@

namespace Automattic\WooCommerce\Caches;

use Automattic\WooCommerce\Caching\CacheException;
use Automattic\WooCommerce\Caching\ObjectCache;

/**
Expand Down
51 changes: 51 additions & 0 deletions plugins/woocommerce/tests/php/src/Caching/OrderCacheTest.php
@@ -0,0 +1,51 @@
<?php

use Automattic\WooCommerce\Caches\OrderCache;
use Automattic\WooCommerce\Internal\DataStores\Orders\OrdersTableDataStore;
use Automattic\WooCommerce\Utilities\OrderUtil;

/**
* Class OrderCacheTest.
*/
class OrderCacheTest extends \WC_Unit_Test_Case {

/**
* System under test.
*
* @var OrderCache
*/
private $sut;

/**
* Setup test.
*/
public function setUp(): void {
parent::setUp();
$this->sut = new OrderCache();
}

/**
* Test that the order cache does not cause duplicate data storage.
*/
public function test_meta_is_not_duplicated_when_cached() {
global $wpdb;
if ( ! OrderUtil::orders_cache_usage_is_enabled() ) {
// tip: add HPOS=1 env variable to run this test.
$this->markTestSkipped( 'HPOS based caching is not enabled.' );
}
$order = WC_Helper_Order::create_order();
$order->add_meta_data( 'test', 'test' );
$order->save_meta_data();

$order = wc_get_order( $order->get_id() );
$this->assertTrue( $this->sut->is_cached( $order->get_id() ), 'Order was not cached, but it was expected to be cached. Are you sure that HPOS based caching is enabled.' );

$order2 = wc_get_order( $order->get_id() );
$order2->save_meta_data();

$orders_meta_table = OrdersTableDataStore::get_meta_table_name();
$query = $wpdb->prepare( "SELECT id FROM $orders_meta_table WHERE order_id = %d AND meta_key = %s", $order->get_id(), 'test' ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
$this->assertEquals( 1, count( $wpdb->get_col( $query ) ) ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Already prepared query.
}

}

0 comments on commit 4fe077b

Please sign in to comment.