add transient to prevent from duplication

sahand-zeynol commented 3 months ago

Fixes #2331

Changes proposed in this Pull Request:

Summary

This PR addresses mentioned issue #2331 where the payment_complete function is called multiple times, potentially leading to unexpected behavior and order status inconsistencies.

Why is this change needed?

The "payment_complete" function in the woocommerce stripe plugin's process_response method is sometimes invoked multiple times simultaneously. This can cause multiple updates to the order status, which may lead to race conditions and inconsistent order states. Ensuring that payment_complete is called only once is crucial for maintaining the integrity and consistency of order processing.

What does this change do?

This change introduces a transient-based locking mechanism to ensure that the payment_complete function is called only once per order. By setting a transient lock before invoking payment_complete and checking for this lock at the beginning of the function, we can prevent multiple simultaneous executions. This ensures that the order status is updated correctly and consistently.

Testing instructions

[ ] Covered with tests (or have a good reason not to test in description ☝️)
[ ] Added changelog entry in both changelog.txt and readme.txt (or does not apply)
[ ] Tested on mobile (or does not apply)

Post merge

[ ] Added testing instructions to the Release Testing Instructions wiki page (or does not apply)

james-allan commented 3 months ago

Hey @sahand-zeynol. Thanks for the PR.

I haven't tested this yet, however I wanted to flag after looking at the changes that we have a similar function for this purpose: WC_Stripe_Payment_Gateway::lock_order_payment().

So rather than introducing a new transient, we should use it instead.

james-allan commented 3 months ago

This PR addresses mentioned issue https://github.com/woocommerce/woocommerce-gateway-stripe/issues/2331 where the payment_complete function is called multiple times, potentially leading to unexpected behavior and order status inconsistencies.

@sahand-zeynol do you know if the duplication you're experiencing and attempting to fix with this change comes from webhook processing?

The reason I ask is because the webhook processing logic already has a check if the payment is locked (code link) and adding a lock to the process_response() function comes with risks because it's a low level function called by all payment processing functions. Functions which already apply a lock and then call process_response() will essentially kill the process.

I suspect what's happening is that payment is being processed by another flow which isn't checking if the payment is locked. eg maybe_process_upe_redirect().

sahand-zeynol commented 3 months ago

Actually, I couldn't find which function caused this duplication, but I think it's meaningful to make process_response() in the transient and lock the payment, I am using this method on my website and it works fine. following this explanation https://github.com/woocommerce/woocommerce-gateway-stripe/issues/2331#issuecomment-2085708821

woocommerce / woocommerce-gateway-stripe