From 4fe077b0ada2a4f138a2beae8ed9f055e6013438 Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Wed, 22 Mar 2023 04:53:57 +0530 Subject: [PATCH] Overwrite clone method to prevent duplicate data when saving a clone. (#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 --- .../changelog/fix-order-cache-duplicate | 4 ++ .../includes/abstracts/abstract-wc-order.php | 11 ++++ plugins/woocommerce/src/Caches/OrderCache.php | 1 + .../tests/php/src/Caching/OrderCacheTest.php | 51 +++++++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 plugins/woocommerce/changelog/fix-order-cache-duplicate create mode 100644 plugins/woocommerce/tests/php/src/Caching/OrderCacheTest.php diff --git a/plugins/woocommerce/changelog/fix-order-cache-duplicate b/plugins/woocommerce/changelog/fix-order-cache-duplicate new file mode 100644 index 000000000000..1fb3ace0c405 --- /dev/null +++ b/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. diff --git a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php index 951cb4dfab19..cab8f59eaabb 100644 --- a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php +++ b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php @@ -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. * diff --git a/plugins/woocommerce/src/Caches/OrderCache.php b/plugins/woocommerce/src/Caches/OrderCache.php index 6f49fa308dd9..fed5b0be0d70 100644 --- a/plugins/woocommerce/src/Caches/OrderCache.php +++ b/plugins/woocommerce/src/Caches/OrderCache.php @@ -2,6 +2,7 @@ namespace Automattic\WooCommerce\Caches; +use Automattic\WooCommerce\Caching\CacheException; use Automattic\WooCommerce\Caching\ObjectCache; /** diff --git a/plugins/woocommerce/tests/php/src/Caching/OrderCacheTest.php b/plugins/woocommerce/tests/php/src/Caching/OrderCacheTest.php new file mode 100644 index 000000000000..cd165a223a3e --- /dev/null +++ b/plugins/woocommerce/tests/php/src/Caching/OrderCacheTest.php @@ -0,0 +1,51 @@ +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. + } + +}