From e7875ab5de0ee85c0223f1e1f4af98fbdf41a633 Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Fri, 25 Aug 2023 20:21:02 +0530 Subject: [PATCH 1/5] Update modified date when a metadata is saved for HPOS. --- .../changelog/fix-cache_data_store | 4 ++ .../Orders/OrdersTableDataStore.php | 40 +++++++++--- .../Orders/OrdersTableDataStoreTests.php | 62 +++++++++++++++++++ 3 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 plugins/woocommerce/changelog/fix-cache_data_store diff --git a/plugins/woocommerce/changelog/fix-cache_data_store b/plugins/woocommerce/changelog/fix-cache_data_store new file mode 100644 index 00000000000..62ac2f77443 --- /dev/null +++ b/plugins/woocommerce/changelog/fix-cache_data_store @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Update modified date when a metadata is saved for HPOS. diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index d76c1b8a358..74d227e2d2b 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -2163,16 +2163,6 @@ FROM $order_meta_table '_wp_trash_meta_time' => time(), ); - foreach ( $trash_metadata as $meta_key => $meta_value ) { - $this->add_meta( - $order, - (object) array( - 'key' => $meta_key, - 'value' => $meta_value, - ) - ); - } - $wpdb->update( self::get_orders_table_name(), array( @@ -2186,6 +2176,16 @@ FROM $order_meta_table $order->set_status( 'trash' ); + foreach ( $trash_metadata as $meta_key => $meta_value ) { + $this->add_meta( + $order, + (object) array( + 'key' => $meta_key, + 'value' => $meta_value, + ) + ); + } + $data_synchronizer = wc_get_container()->get( DataSynchronizer::class ); if ( $data_synchronizer->data_sync_is_enabled() ) { wp_trash_post( $order->get_id() ); @@ -2794,6 +2794,8 @@ CREATE TABLE $meta_table ( */ public function delete_meta( &$object, $meta ) { $delete_meta = $this->data_store_meta->delete_meta( $object, $meta ); + $this->update_date_modified( $object ); + $this->clear_caches( $object ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2814,6 +2816,8 @@ CREATE TABLE $meta_table ( */ public function add_meta( &$object, $meta ) { $add_meta = $this->data_store_meta->add_meta( $object, $meta ); + $this->update_date_modified( $object ); + $this->clear_caches( $object ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2834,6 +2838,8 @@ CREATE TABLE $meta_table ( */ public function update_meta( &$object, $meta ) { $update_meta = $this->data_store_meta->update_meta( $object, $meta ); + $this->update_date_modified( $object ); + $this->clear_caches( $object ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2843,4 +2849,18 @@ CREATE TABLE $meta_table ( return $update_meta; } + + /** + * Update date modified, but try to be efficient about it. Use some heuristic to not do it too often. + * + * @param WC_Abstract_Order $order Order object. + */ + protected function update_date_modified( $order ) { + $current_date_time = new \WC_DateTime( 'now', new \DateTimeZone( 'GMT' ) ); + // Prevent this happening multiple time in same request. + if ( $order->get_date_modified() < $current_date_time && empty( $order->get_changes() ) ) { + $order->set_date_modified( current_time( 'mysql' ) ); + $order->save(); + } + } } diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php index 3bcd3cdd943..900f3864f09 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php @@ -2740,5 +2740,67 @@ class OrdersTableDataStoreTests extends HposTestCase { remove_action( 'woocommerce_update_order', $callback ); } + /** + * @testDox Stale data is not read when sync is off, but then switched on again after a while. + * @testWith [true] + * + * @param bool $different_request Whether to simulate different requests (as much as we can in a unit test) + */ + public function test_stale_data_is_not_read_sync_off_on( $different_request ) { + $this->toggle_cot_authoritative( true ); + $this->enable_cot_sync(); + $cot_store = wc_get_container()->get( OrdersTableDataStore::class ); + + $order = OrderHelper::create_order(); + $order->set_customer_id( 1 ); // Change a custom table column. + $order->set_billing_address_1( 'test' ); // Change an address column and a meta row. + $order->set_download_permissions_granted( true ); // Change an operational data column. + $order->save(); + + $different_request && $this->reset_order_data_store_state( $cot_store ); + + $order->add_meta_data( 'test_key', 'test_value' ); + $order->save_meta_data(); + + $different_request && $this->reset_order_data_store_state( $cot_store ); + + $r_order = wc_get_order( $order->get_id() ); + $this->assertEquals( 1, $r_order->get_customer_id() ); + $this->assertEquals( 'test', $r_order->get_billing_address_1() ); + $this->assertTrue( $order->get_download_permissions_granted() ); + $this->assertEquals( 'test_value', $r_order->get_meta( 'test_key', true ) ); + + $different_request && $this->reset_order_data_store_state( $cot_store ); + sleep(2); + + $this->disable_cot_sync(); + $r_order->update_meta_data( 'test_key', 'test_value_updated' ); + $r_order->add_meta_data( 'test_key2', 'test_value2' ); + $r_order->save_meta_data(); + + $different_request && $this->reset_order_data_store_state( $cot_store ); + + $this->enable_cot_sync(); + $r_order = wc_get_order( $order->get_id() ); + $this->assertEquals( 1, $r_order->get_customer_id() ); + $this->assertEquals( 'test', $r_order->get_billing_address_1() ); + $this->assertTrue( $order->get_download_permissions_granted() ); + $this->assertEquals( 'test_value_updated', $r_order->get_meta( 'test_key', true ) ); + $this->assertEquals( 'test_value2', $r_order->get_meta( 'test_key2', true ) ); + } + + /** + * Helper method to reset order data store state (to help simulate multiple requests). + * + * @param OrdersTableDataStore $sut System under test. + */ + private function reset_order_data_store_state( $sut ) { + $reset_state = function () use ( $sut ) { + self::$backfilling_order_ids = []; + self::$reading_order_ids = []; + }; + $reset_state->call( $sut ); + wp_cache_flush(); + } } From 9a01cfdde71a641b844301513ef37002c410ef09 Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Mon, 28 Aug 2023 15:20:17 +0530 Subject: [PATCH 2/5] Consolidate all ops in one function. Additionally, move the `type` column from always change to optional, by moving it so that its applied if there is atleast one other change. --- .../Orders/OrdersTableDataStore.php | 33 +++++++++---------- .../Orders/OrdersTableDataStoreTests.php | 1 + 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 74d227e2d2b..e46d24dfa81 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -1858,8 +1858,8 @@ FROM $order_meta_table if ( $row ) { $result[] = array( 'table' => self::get_orders_table_name(), - 'data' => array_merge( $row['data'], array( 'id' => $order->get_id() ) ), - 'format' => array_merge( $row['format'], array( 'id' => '%d' ) ), + 'data' => array_merge( $row['data'], array( 'id' => $order->get_id(), 'type' => $order->get_type() ) ), + 'format' => array_merge( $row['format'], array( 'id' => '%d', 'type' => '%s' ) ), ); } @@ -1928,8 +1928,6 @@ FROM $order_meta_table protected function get_db_row_from_order( $order, $column_mapping, $only_changes = false ) { $changes = $only_changes ? $order->get_changes() : array_merge( $order->get_data(), $order->get_changes() ); - $changes['type'] = $order->get_type(); - // Make sure 'status' is correctly prefixed. if ( array_key_exists( 'status', $column_mapping ) && array_key_exists( 'status', $changes ) ) { $changes['status'] = $this->get_post_status( $order ); @@ -2794,8 +2792,7 @@ CREATE TABLE $meta_table ( */ public function delete_meta( &$object, $meta ) { $delete_meta = $this->data_store_meta->delete_meta( $object, $meta ); - $this->update_date_modified( $object ); - $this->clear_caches( $object ); + $this->after_meta_change( $object, $meta ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2815,9 +2812,8 @@ CREATE TABLE $meta_table ( * @return int|bool meta ID or false on failure */ public function add_meta( &$object, $meta ) { - $add_meta = $this->data_store_meta->add_meta( $object, $meta ); - $this->update_date_modified( $object ); - $this->clear_caches( $object ); + $meta->id = $this->data_store_meta->add_meta( $object, $meta ); + $this->after_meta_change( $object, $meta ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2825,21 +2821,20 @@ CREATE TABLE $meta_table ( self::$backfilling_order_ids = array_diff( self::$backfilling_order_ids, array( $object->get_id() ) ); } - return $add_meta; + return $meta->id; } /** * Update meta. * - * @param WC_Data $object WC_Data object. - * @param stdClass $meta (containing ->id, ->key and ->value). + * @param WC_Data $object WC_Data object. + * @param \WC_Meta_Data $meta (containing ->id, ->key and ->value). * - * @return bool + * @return */ public function update_meta( &$object, $meta ) { $update_meta = $this->data_store_meta->update_meta( $object, $meta ); - $this->update_date_modified( $object ); - $this->clear_caches( $object ); + $this->after_meta_change( $object, $meta ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { self::$backfilling_order_ids[] = $object->get_id(); @@ -2851,12 +2846,16 @@ CREATE TABLE $meta_table ( } /** - * Update date modified, but try to be efficient about it. Use some heuristic to not do it too often. + * Perform after meta change operations, including updating the date_modified field, clearing caches and applying changes. * * @param WC_Abstract_Order $order Order object. + * @param \WC_Meta_Data $meta Metadata object. */ - protected function update_date_modified( $order ) { + protected function after_meta_change( &$order, $meta ) { $current_date_time = new \WC_DateTime( 'now', new \DateTimeZone( 'GMT' ) ); + $meta->apply_changes(); + $this->clear_caches( $order ); + // Prevent this happening multiple time in same request. if ( $order->get_date_modified() < $current_date_time && empty( $order->get_changes() ) ) { $order->set_date_modified( current_time( 'mysql' ) ); diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php index 900f3864f09..da66db16c09 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php @@ -2743,6 +2743,7 @@ class OrdersTableDataStoreTests extends HposTestCase { /** * @testDox Stale data is not read when sync is off, but then switched on again after a while. * @testWith [true] + * [false] * * @param bool $different_request Whether to simulate different requests (as much as we can in a unit test) */ From aa08add6e3c250338ac23b54de139004ef456aab Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Mon, 28 Aug 2023 15:58:04 +0530 Subject: [PATCH 3/5] Defencive checks around meta. --- .../src/Internal/DataStores/Orders/OrdersTableDataStore.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index e46d24dfa81..ea912b21bfe 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -2853,7 +2853,7 @@ CREATE TABLE $meta_table ( */ protected function after_meta_change( &$order, $meta ) { $current_date_time = new \WC_DateTime( 'now', new \DateTimeZone( 'GMT' ) ); - $meta->apply_changes(); + method_exists( $meta, 'apply_changes' ) && $meta->apply_changes(); $this->clear_caches( $order ); // Prevent this happening multiple time in same request. From d2178198247e03067c27183cef1dde0db7068795 Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Mon, 28 Aug 2023 16:37:31 +0530 Subject: [PATCH 4/5] Use same precision as DB timezone to prevent unnecessary updates. --- .../src/Internal/DataStores/Orders/OrdersTableDataStore.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index ea912b21bfe..763ae5bdf47 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -2852,7 +2852,7 @@ CREATE TABLE $meta_table ( * @param \WC_Meta_Data $meta Metadata object. */ protected function after_meta_change( &$order, $meta ) { - $current_date_time = new \WC_DateTime( 'now', new \DateTimeZone( 'GMT' ) ); + $current_date_time = new \WC_DateTime( current_time( 'mysql'), new \DateTimeZone( 'GMT' ) ); method_exists( $meta, 'apply_changes' ) && $meta->apply_changes(); $this->clear_caches( $order ); From 9b10f04c9894b72a23dde8826f82613982a06074 Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Mon, 28 Aug 2023 17:14:14 +0530 Subject: [PATCH 5/5] Formatting changes and sanitization. --- .../DataStores/Orders/OrdersTableDataStore.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 763ae5bdf47..487f401daa5 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -2785,8 +2785,8 @@ CREATE TABLE $meta_table ( /** * Deletes meta based on meta ID. * - * @param WC_Data $object WC_Data object. - * @param stdClass $meta (containing at least ->id). + * @param WC_Data $object WC_Data object. + * @param \stdClass $meta (containing at least ->id). * * @return bool */ @@ -2806,13 +2806,13 @@ CREATE TABLE $meta_table ( /** * Add new piece of meta. * - * @param WC_Data $object WC_Data object. - * @param stdClass $meta (containing ->key and ->value). + * @param WC_Data $object WC_Data object. + * @param \stdClass $meta (containing ->key and ->value). * * @return int|bool meta ID or false on failure */ public function add_meta( &$object, $meta ) { - $meta->id = $this->data_store_meta->add_meta( $object, $meta ); + $add_meta = $this->data_store_meta->add_meta( $object, $meta ); $this->after_meta_change( $object, $meta ); if ( $object instanceof WC_Abstract_Order && $this->should_backfill_post_record() ) { @@ -2821,14 +2821,14 @@ CREATE TABLE $meta_table ( self::$backfilling_order_ids = array_diff( self::$backfilling_order_ids, array( $object->get_id() ) ); } - return $meta->id; + return $add_meta; } /** * Update meta. * - * @param WC_Data $object WC_Data object. - * @param \WC_Meta_Data $meta (containing ->id, ->key and ->value). + * @param WC_Data $object WC_Data object. + * @param \stdClass $meta (containing ->id, ->key and ->value). * * @return */ @@ -2852,7 +2852,7 @@ CREATE TABLE $meta_table ( * @param \WC_Meta_Data $meta Metadata object. */ protected function after_meta_change( &$order, $meta ) { - $current_date_time = new \WC_DateTime( current_time( 'mysql'), new \DateTimeZone( 'GMT' ) ); + $current_date_time = new \WC_DateTime( current_time( 'mysql', 1 ), new \DateTimeZone( 'GMT' ) ); method_exists( $meta, 'apply_changes' ) && $meta->apply_changes(); $this->clear_caches( $order );