Skip to content

Commit 0109306

Browse files
jimmynjimmyn
committed
refactor: remove locking mechanism and idempotency flag
BREAKING CHANGE: Removed lock and _monei_payment_id_processed flag Analysis revealed WooCommerce creates orders BEFORE payment (unlike PrestaShop), so duplicate order creation is impossible. The lock and processed flag were: 1. Broken - wp_cache not persistent without external cache 2. Harmful - flag blocked AUTHORIZED→SUCCEEDED and SUCCEEDED→REFUNDED transitions 3. Unnecessary - WooCommerce's payment_complete() is already idempotent Removed components: - WC_Monei_Lock_Helper class - Lock acquisition/release in IPN and redirect handlers - _monei_payment_id_processed flag checks and setting - wp_cache stubs from PHPStan bootstrap The order status check provides sufficient protection against duplicate processing. Any duplicate order notes are cosmetic and acceptable.
1 parent 8561db1 commit 0109306

File tree

5 files changed

+53
-212
lines changed

5 files changed

+53
-212
lines changed

‎class-woocommerce-gateway-monei.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ private function includes() {
139139
$container = ContainerProvider::getContainer();
140140
include_once 'includes/woocommerce-gateway-monei-core-functions.php';
141141
include_once 'includes/class-wc-monei-ipn.php';
142-
include_once 'includes/class-wc-monei-lock-helper.php';
143142
include_once 'includes/class-wc-monei-logger.php';
144143
include_once 'includes/class-wc-monei-payment-method-display.php';
145144

‎includes/class-wc-monei-ipn.php‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -85,21 +85,6 @@ public function check_ipn_request() {
8585
exit;
8686
}
8787

88-
// Acquire lock to prevent concurrent processing of the same payment.
89-
// IMPORTANT: Must use same lock key pattern as redirect verification to prevent race conditions.
90-
$payment_id = $payload['id'] ?? '';
91-
$lock_key = WC_Monei_Lock_Helper::get_payment_lock_key( $payment_id );
92-
$lock_value = wp_rand();
93-
94-
if ( ! WC_Monei_Lock_Helper::acquire_lock( $lock_key, $lock_value ) ) {
95-
// Another process is handling this payment.
96-
$this->logging && WC_Monei_Logger::log( '[MONEI] Webhook already being processed [payment_id=' . $payment_id . ']', 'debug' );
97-
http_response_code( 200 );
98-
header( 'Content-Type: text/plain; charset=utf-8' );
99-
echo 'OK';
100-
exit;
101-
}
102-
10388
try {
10489
$this->handle_valid_ipn( $payload );
10590
do_action( 'woocommerce_monei_handle_valid_ipn', $payload );
@@ -113,9 +98,6 @@ public function check_ipn_request() {
11398
http_response_code( 400 );
11499
header( 'Content-Type: text/plain; charset=utf-8' );
115100
echo 'Bad Request';
116-
} finally {
117-
// Always release the lock.
118-
WC_Monei_Lock_Helper::release_lock( $lock_key, $lock_value );
119101
}
120102

121103
exit;
@@ -144,22 +126,13 @@ protected function handle_valid_ipn( $payload ) {
144126
return;
145127
}
146128

147-
// Check if this payment was already processed (idempotency check).
148-
$processed_payment_id = $order->get_meta( '_monei_payment_id_processed', true );
149-
if ( $processed_payment_id === $monei_id ) {
150-
// Payment already processed, skip to prevent duplicate processing.
151-
$this->logging && WC_Monei_Logger::log( '[MONEI] Payment already processed [payment_id=' . $monei_id . ', order_id=' . $order_id . ']', 'debug' );
152-
return;
153-
}
154-
155129
/**
156130
* Saving related information into order meta.
157131
*/
158132
$order->update_meta_data( '_payment_order_number_monei', $monei_id );
159133
$order->update_meta_data( '_payment_order_status_monei', $status );
160134
$order->update_meta_data( '_payment_order_status_code_monei', $status_code );
161135
$order->update_meta_data( '_payment_order_status_message_monei', $status_message );
162-
$order->update_meta_data( '_monei_payment_id_processed', $monei_id );
163136

164137
// Fetch payment from API to get payment method information
165138
try {

‎includes/class-wc-monei-lock-helper.php‎

Lines changed: 0 additions & 72 deletions
This file was deleted.

‎includes/class-wc-monei-redirect-hooks.php‎

Lines changed: 53 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -200,96 +200,72 @@ private function verify_and_complete_order( $order_id, $payment ) {
200200
return;
201201
}
202202

203-
// Acquire lock to prevent race condition with IPN webhook.
204-
// IMPORTANT: Must use same lock key pattern as IPN to prevent race conditions.
205-
$lock_key = WC_Monei_Lock_Helper::get_payment_lock_key( $payment->getId() );
206-
$lock_value = wp_rand();
203+
/** @var string $payment_status */
204+
$payment_status = $payment->getStatus();
205+
$order_status = $order->get_status();
207206

208-
if ( ! WC_Monei_Lock_Helper::acquire_lock( $lock_key, $lock_value, 60 ) ) {
209-
WC_Monei_Logger::log( sprintf( '[MONEI] Order completion already in progress [order_id=%s, payment_id=%s]', $order_id, $payment->getId() ), 'debug' );
207+
WC_Monei_Logger::log( sprintf( '[MONEI] Redirect verification [payment_id=%s, order_id=%s, payment_status=%s, order_status=%s]', $payment->getId(), $order_id, $payment_status, $order_status ), 'debug' );
208+
209+
// Only process if order is still pending/on-hold/failed and payment succeeded
210+
if ( ! in_array( $order_status, array( 'pending', 'on-hold', 'failed' ), true ) ) {
211+
WC_Monei_Logger::log( sprintf( '[MONEI] Order already processed, skipping [order_id=%s, status=%s]', $order_id, $order_status ), 'debug' );
210212
return;
211213
}
212214

213-
try {
214-
// Check if payment was already processed (prevent duplicate processing)
215-
$processed_payment_id = $order->get_meta( '_monei_payment_id_processed', true );
216-
if ( $processed_payment_id === $payment->getId() ) {
217-
WC_Monei_Logger::log( sprintf( '[MONEI] Payment already processed via IPN [payment_id=%s, order_id=%s]', $payment->getId(), $order_id ), 'debug' );
215+
// If payment is SUCCEEDED or AUTHORIZED, complete the order
216+
if ( PaymentStatus::SUCCEEDED === $payment_status || PaymentStatus::AUTHORIZED === $payment_status ) {
217+
$amount = $payment->getAmount();
218+
$order_total = $order->get_total();
219+
220+
// Verify amounts match (with 1 cent exception for subscriptions)
221+
if ( ( (int) $amount !== monei_price_format( $order_total ) ) && ( 1 !== $amount ) ) {
222+
$order->update_status(
223+
'on-hold',
224+
sprintf(
225+
/* translators: 1: Order amount, 2: Payment amount */
226+
__( 'Validation error: Order vs. Payment amounts do not match (order: %1$s - received: %2$s).', 'monei' ),
227+
monei_price_format( $order_total ),
228+
$amount
229+
)
230+
);
231+
WC_Monei_Logger::log( sprintf( '[MONEI] Amount mismatch [order_id=%s, order_amount=%s, payment_amount=%s]', $order_id, monei_price_format( $order_total ), $amount ), 'error' );
218232
return;
219233
}
220234

221-
/** @var string $payment_status */
222-
$payment_status = $payment->getStatus();
223-
$order_status = $order->get_status();
224-
225-
WC_Monei_Logger::log( sprintf( '[MONEI] Redirect verification [payment_id=%s, order_id=%s, payment_status=%s, order_status=%s]', $payment->getId(), $order_id, $payment_status, $order_status ), 'debug' );
235+
$order->update_meta_data( '_payment_order_number_monei', $payment->getId() );
236+
$order->update_meta_data( '_payment_order_status_monei', $payment_status );
237+
$order->update_meta_data( '_payment_order_status_code_monei', $payment->getStatusCode() );
238+
$order->update_meta_data( '_payment_order_status_message_monei', $payment->getStatusMessage() );
226239

227-
// Only process if order is still pending/on-hold/failed and payment succeeded
228-
if ( ! in_array( $order_status, array( 'pending', 'on-hold', 'failed' ), true ) ) {
229-
WC_Monei_Logger::log( sprintf( '[MONEI] Order already processed, skipping [order_id=%s, status=%s]', $order_id, $order_status ), 'debug' );
230-
return;
240+
// Store formatted payment method display
241+
$payment_method_display = $this->paymentMethodFormatter->get_payment_method_display_from_payment( $payment );
242+
if ( $payment_method_display ) {
243+
$order->update_meta_data( '_monei_payment_method_display', $payment_method_display );
231244
}
232245

233-
// If payment is SUCCEEDED or AUTHORIZED, complete the order
234-
if ( PaymentStatus::SUCCEEDED === $payment_status || PaymentStatus::AUTHORIZED === $payment_status ) {
235-
$amount = $payment->getAmount();
236-
$order_total = $order->get_total();
237-
238-
// Verify amounts match (with 1 cent exception for subscriptions)
239-
if ( ( (int) $amount !== monei_price_format( $order_total ) ) && ( 1 !== $amount ) ) {
240-
$order->update_status(
241-
'on-hold',
242-
sprintf(
243-
/* translators: 1: Order amount, 2: Payment amount */
244-
__( 'Validation error: Order vs. Payment amounts do not match (order: %1$s - received: %2$s).', 'monei' ),
245-
monei_price_format( $order_total ),
246-
$amount
247-
)
248-
);
249-
WC_Monei_Logger::log( sprintf( '[MONEI] Amount mismatch [order_id=%s, order_amount=%s, payment_amount=%s]', $order_id, monei_price_format( $order_total ), $amount ), 'error' );
250-
return;
251-
}
252-
253-
// Mark payment as processed to prevent duplicate processing by IPN
254-
$order->update_meta_data( '_monei_payment_id_processed', $payment->getId() );
255-
$order->update_meta_data( '_payment_order_number_monei', $payment->getId() );
256-
$order->update_meta_data( '_payment_order_status_monei', $payment_status );
257-
$order->update_meta_data( '_payment_order_status_code_monei', $payment->getStatusCode() );
258-
$order->update_meta_data( '_payment_order_status_message_monei', $payment->getStatusMessage() );
259-
260-
// Store formatted payment method display
261-
$payment_method_display = $this->paymentMethodFormatter->get_payment_method_display_from_payment( $payment );
262-
if ( $payment_method_display ) {
263-
$order->update_meta_data( '_monei_payment_method_display', $payment_method_display );
264-
}
246+
if ( PaymentStatus::AUTHORIZED === $payment_status ) {
247+
$order->update_meta_data( '_payment_not_captured_monei', 1 );
248+
$order_note = __( 'Payment verified via redirect - <strong>Payment Authorized</strong>', 'monei' ) . '. <br><br>';
249+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
250+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
251+
$order->add_order_note( $order_note );
252+
$order->update_status( 'on-hold', __( 'Order On-Hold by MONEI', 'monei' ) );
253+
} else {
254+
// SUCCEEDED
255+
$order_note = __( 'Payment verified via redirect - <strong>Payment Completed</strong>', 'monei' ) . '. <br><br>';
256+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
257+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
258+
$order->add_order_note( $order_note );
259+
$order->payment_complete();
265260

266-
if ( PaymentStatus::AUTHORIZED === $payment_status ) {
267-
$order->update_meta_data( '_payment_not_captured_monei', 1 );
268-
$order_note = __( 'Payment verified via redirect - <strong>Payment Authorized</strong>', 'monei' ) . '. <br><br>';
269-
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
270-
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
271-
$order->add_order_note( $order_note );
272-
$order->update_status( 'on-hold', __( 'Order On-Hold by MONEI', 'monei' ) );
273-
} else {
274-
// SUCCEEDED
275-
$order_note = __( 'Payment verified via redirect - <strong>Payment Completed</strong>', 'monei' ) . '. <br><br>';
276-
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
277-
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
278-
$order->add_order_note( $order_note );
279-
$order->payment_complete();
280-
281-
$payment_method_woo_id = $order->get_payment_method();
282-
if ( 'completed' === monei_get_settings( 'orderdo', $payment_method_woo_id ) ) {
283-
$order->update_status( 'completed', __( 'Order Completed by MONEI', 'monei' ) );
284-
}
261+
$payment_method_woo_id = $order->get_payment_method();
262+
if ( 'completed' === monei_get_settings( 'orderdo', $payment_method_woo_id ) ) {
263+
$order->update_status( 'completed', __( 'Order Completed by MONEI', 'monei' ) );
285264
}
286-
287-
$order->save();
288-
WC_Monei_Logger::log( sprintf( '[MONEI] Order completed via redirect verification [order_id=%s, payment_status=%s]', $order_id, $payment_status ), 'debug' );
289265
}
290-
} finally {
291-
// Always release the lock.
292-
WC_Monei_Lock_Helper::release_lock( $lock_key, $lock_value );
266+
267+
$order->save();
268+
WC_Monei_Logger::log( sprintf( '[MONEI] Order completed via redirect verification [order_id=%s, payment_status=%s]', $order_id, $payment_status ), 'debug' );
293269
}
294270
}
295271
}

‎tests/phpstan-bootstrap.php‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -130,41 +130,6 @@ function set_transient( $transient, $value, $expiration = 0 ) {
130130
}
131131
}
132132

133-
if ( ! function_exists( 'wp_cache_add' ) ) {
134-
/**
135-
* @param string $key
136-
* @param mixed $data
137-
* @param string $group
138-
* @param int $expire
139-
* @return bool
140-
*/
141-
function wp_cache_add( $key, $data, $group = '', $expire = 0 ) {
142-
return true;
143-
}
144-
}
145-
146-
if ( ! function_exists( 'wp_cache_get' ) ) {
147-
/**
148-
* @param string $key
149-
* @param string $group
150-
* @return mixed
151-
*/
152-
function wp_cache_get( $key, $group = '' ) {
153-
return false;
154-
}
155-
}
156-
157-
if ( ! function_exists( 'wp_cache_delete' ) ) {
158-
/**
159-
* @param string $key
160-
* @param string $group
161-
* @return bool
162-
*/
163-
function wp_cache_delete( $key, $group = '' ) {
164-
return true;
165-
}
166-
}
167-
168133
if ( ! function_exists( 'get_transient' ) ) {
169134
/**
170135
* @param string $transient

0 commit comments

Comments
 (0)