Fix: Infinite loop caused by calling $order->save_meta_data() during woocommerce_update_order hook
#59652
Conversation
…PT->clear_caches() is called.
Testing GuidelinesHi @kalessil , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughThe changes relocate the logic for clearing the order cache from the order object's Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WC_Order
participant DataStore
participant OrderCache
User->>WC_Order: save_meta_data()
WC_Order->>DataStore: clear_caches(order_id)
DataStore->>OrderCache: remove(order_id)
Note right of OrderCache: Order cache entry is cleared
sequenceDiagram
participant RefundUpdater
participant RefundDataStore
participant OrderCache
RefundUpdater->>RefundDataStore: update(refund)
RefundDataStore->>RefundDataStore: persist_updates(refund)
RefundDataStore->>refund: apply_changes()
Note right of RefundDataStore: Changes applied after update
Estimated code review effort2 (~20 minutes) 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{php,js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
Files:
**/*.{php,js,ts,jsx,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (2)📓 Common learningsplugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php (1)Learnt from: gigitux ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
🔇 Additional comments (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
LGTM to me overall. @prettyboymp can you please take a look into my comments before I start manual testing?
plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php
Outdated
Show resolved
Hide resolved
plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableRefundDataStore.php
Outdated
Show resolved
Hide resolved
Remove the call to clear_cache in OrdersTableRefundDataStore, it was only the test that needed updated.
…`woocommerce_update_order` hook (#59652) * Remove in-memory OrderCache entry when Abstract_WC_ORder_Data_Store_CPT->clear_caches() is called. * Add changelog * Clear caches after a refund update. * Switch to using imports Remove the call to clear_cache in OrdersTableRefundDataStore, it was only the test that needed updated.
|
IMPORTANT: Merging this PR to the appropriate branches is critical to the release process and ensures that the bug does not cause regressions in the future releases. Cherry picking was successful for |
…r->save_meta_data() during `woocommerce_update_order` hook (#59949) Fix: Infinite loop caused by calling $order->save_meta_data() during `woocommerce_update_order` hook (#59652) * Remove in-memory OrderCache entry when Abstract_WC_ORder_Data_Store_CPT->clear_caches() is called. * Add changelog * Clear caches after a refund update. * Switch to using imports Remove the call to clear_cache in OrdersTableRefundDataStore, it was only the test that needed updated. Co-authored-by: Michael Pretty <prettyboymp@users.noreply.github.com>
Changes proposed in this Pull Request:
This modifies Abstract_WC_ORder_Data_Store_CPT->clear_caches() to also remove in-memory OrderCache entry for the given order.
There is common pattern in extensions to do the following:
When this is run on an order for the first time, the meta data entry is added and saved, but ends up in an infinite loop, because of the following scenario:
$order = wc_get_order( $order_id );retrieves the previously instantiated Order instance from in-memory OrderCache,$order->save_meta_data();Saves the changes back to the DB, but then again runs thewoocommerce_update_orderhook before theOrderCacheentry is moved.Closes WOOPLUG-4910 / #59455 .
(For Bug Fixes) Bug introduced in PR #46023 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
To test that it fixes the issue with the infinite loop:
Requires object caching
Enable order data caching in the datastoreunder WooCommerce -> Settings -> Advanced -> Features.woocommerce_update_orderhook. So you will need to either change the meta key you're testing with or use a new order each time.If the problem persists, doing the above will trigger an infinite loop. Otherwise, it should save normally.
General testing -
Do some general testing around orders.
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment