Skip to content

Commit 5975008

Browse files
sandervanhooftsandervanhooft
committed
Harden webhook handlers against duplicate processing
1 parent db5d8aa commit 5975008

7 files changed

Lines changed: 306 additions & 51 deletions

File tree

‎src/Http/Controllers/AftercareWebhookController.php‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Illuminate\Http\Request;
66
use Illuminate\Support\Collection;
7+
use Illuminate\Support\Facades\DB;
78
use Illuminate\Support\Facades\Event;
89
use Laravel\Cashier\Cashier;
910
use Laravel\Cashier\Events\ChargebackReceived;
@@ -48,18 +49,32 @@ public function handleWebhook(Request $request)
4849
}
4950

5051
$molliePaymentAmountChargedBackTotal = mollie_object_to_money($molliePayment->amountChargedBack);
51-
$locallyKnownAmountChargedBack = $localPayment->getAmountChargedBack();
52+
$chargeback = DB::transaction(function () use ($localPayment, $molliePaymentAmountChargedBackTotal) {
53+
/** @var \Laravel\Cashier\Payment|null $localPayment */
54+
$localPayment = Cashier::$paymentModel::whereKey($localPayment->getKey())->lockForUpdate()->first();
55+
56+
if (! $localPayment) {
57+
return null;
58+
}
59+
60+
$locallyKnownAmountChargedBack = $localPayment->getAmountChargedBack();
61+
62+
if (! $locallyKnownAmountChargedBack->lessThan($molliePaymentAmountChargedBackTotal)) {
63+
return null;
64+
}
5265

53-
if ($locallyKnownAmountChargedBack->lessThan($molliePaymentAmountChargedBackTotal)) {
5466
$localPayment->amount_charged_back = (int) $molliePaymentAmountChargedBackTotal->getAmount();
5567
$localPayment->save();
5668

57-
$amountChargedBackNow = $molliePaymentAmountChargedBackTotal->subtract(
58-
$locallyKnownAmountChargedBack
59-
);
69+
return [
70+
$localPayment,
71+
$molliePaymentAmountChargedBackTotal->subtract($locallyKnownAmountChargedBack),
72+
];
73+
});
6074

75+
if ($chargeback) {
6176
Event::dispatch(
62-
new ChargebackReceived($localPayment, $amountChargedBackNow)
77+
new ChargebackReceived($chargeback[0], $chargeback[1])
6378
);
6479
}
6580
}

‎src/Http/Controllers/WebhookController.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,14 @@ public function handleWebhook(Request $request)
2828
switch ($payment->status) {
2929
case PaymentStatus::PAID:
3030
$order->handlePaymentPaid($payment);
31-
$payment->webhookUrl = route('webhooks.mollie.aftercare');
3231

33-
/** @var UpdateMolliePayment $updateMolliePayment */
34-
$updateMolliePayment = app()->make(UpdateMolliePayment::class);
35-
$updateMolliePayment->execute($payment);
32+
if ($order->wasPaymentStatusHandled()) {
33+
$payment->webhookUrl = route('webhooks.mollie.aftercare');
34+
35+
/** @var UpdateMolliePayment $updateMolliePayment */
36+
$updateMolliePayment = app()->make(UpdateMolliePayment::class);
37+
$updateMolliePayment->execute($payment);
38+
}
3639

3740
break;
3841
case PaymentStatus::FAILED:

‎src/Order/Order.php‎

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ class Order extends Model
7878

7979
protected $guarded = [];
8080

81+
/** @var bool */
82+
protected $paymentStatusHandled = false;
83+
8184
/**
8285
* @return int
8386
*/
@@ -445,35 +448,50 @@ public function creditApplied()
445448
*/
446449
public function handlePaymentFailed(MolliePayment $molliePayment)
447450
{
448-
$localPayment = DB::transaction(function () use ($molliePayment) {
449-
if ($this->creditApplied()) {
450-
$this->owner->addCredit($this->getCreditUsed());
451+
$this->paymentStatusHandled = false;
452+
$handled = false;
453+
$localPayment = DB::transaction(function () use ($molliePayment, &$handled) {
454+
/** @var static $order */
455+
$order = static::whereKey($this->getKey())->lockForUpdate()->firstOrFail();
456+
457+
if ($order->mollie_payment_status === PaymentStatus::FAILED) {
458+
return null;
451459
}
452460

453-
$this->update([
454-
'mollie_payment_status' => 'failed',
461+
if ($order->creditApplied()) {
462+
$order->owner->addCredit($order->getCreditUsed());
463+
}
464+
465+
$order->update([
466+
'mollie_payment_status' => PaymentStatus::FAILED,
455467
'balance_before' => 0,
456468
'credit_used' => 0,
457469
]);
458470

459471
// It's possible a payment from Cashier v1 is not yet tracked in the Cashier database.
460472
// In that case we create a record here.
461-
$localPayment = Cashier::$paymentModel::findByMolliePaymentOrCreate($molliePayment, $this->owner);
473+
$localPayment = Cashier::$paymentModel::findByMolliePaymentOrCreate($molliePayment, $order->owner);
462474
$localPayment->update([
463-
'mollie_payment_status' => 'failed',
464-
'order_id' => $this->id,
475+
'mollie_payment_status' => PaymentStatus::FAILED,
476+
'order_id' => $order->id,
465477
]);
466478

467-
$this->items->each(function (OrderItem $item) {
479+
$order->items->each(function (OrderItem $item) {
468480
$item->handlePaymentFailed();
469481
});
470482

471-
$this->owner->validateMollieMandate();
483+
$order->owner->validateMollieMandate();
472484

485+
$handled = true;
473486
return $localPayment;
474487
});
475488

476-
Event::dispatch(new OrderPaymentFailed($this, $localPayment));
489+
$this->refresh();
490+
$this->paymentStatusHandled = $handled;
491+
492+
if ($handled) {
493+
Event::dispatch(new OrderPaymentFailed($this, $localPayment));
494+
}
477495

478496
return $this;
479497
}
@@ -522,29 +540,54 @@ public function handlePaymentFailedDueToInvalidMandate()
522540
*/
523541
public function handlePaymentPaid(MolliePayment $molliePayment)
524542
{
525-
$localPayment = DB::transaction(function () use ($molliePayment) {
526-
$this->update(['mollie_payment_status' => 'paid']);
543+
$this->paymentStatusHandled = false;
544+
$handled = false;
545+
$localPayment = DB::transaction(function () use ($molliePayment, &$handled) {
546+
/** @var static $order */
547+
$order = static::whereKey($this->getKey())->lockForUpdate()->firstOrFail();
548+
549+
if ($order->mollie_payment_status === PaymentStatus::PAID) {
550+
return null;
551+
}
552+
553+
$order->update(['mollie_payment_status' => PaymentStatus::PAID]);
527554

528555
// It's possible a payment from Cashier v1 is not yet tracked in the Cashier database.
529556
// In that case we create a record here.
530-
$localPayment = Cashier::$paymentModel::findByMolliePaymentOrCreate($molliePayment, $this->owner);
557+
$localPayment = Cashier::$paymentModel::findByMolliePaymentOrCreate($molliePayment, $order->owner);
531558
$localPayment->update([
532-
'mollie_payment_status' => 'paid',
533-
'order_id' => $this->id,
559+
'mollie_payment_status' => PaymentStatus::PAID,
560+
'order_id' => $order->id,
534561
]);
535562

536-
$this->items->each(function (OrderItem $item) {
563+
$order->items->each(function (OrderItem $item) {
537564
$item->handlePaymentPaid();
538565
});
539566

567+
$handled = true;
540568
return $localPayment;
541569
});
542570

543-
Event::dispatch(new OrderPaymentPaid($this, $localPayment));
571+
$this->refresh();
572+
$this->paymentStatusHandled = $handled;
573+
574+
if ($handled) {
575+
Event::dispatch(new OrderPaymentPaid($this, $localPayment));
576+
}
544577

545578
return $this;
546579
}
547580

581+
/**
582+
* Determine if the last payment status handler changed this order.
583+
*
584+
* @return bool
585+
*/
586+
public function wasPaymentStatusHandled()
587+
{
588+
return $this->paymentStatusHandled;
589+
}
590+
548591
/**
549592
* @return \Money\Money
550593
*/

‎src/Refunds/Refund.php‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,24 @@ public function order(): HasOne
7878

7979
public function handleProcessed(): self
8080
{
81-
$refundItems = $this->items;
81+
$handled = false;
8282

83-
DB::transaction(function () use ($refundItems) {
83+
DB::transaction(function () use (&$handled) {
84+
/** @var static $refund */
85+
$refund = static::whereKey($this->getKey())->lockForUpdate()->firstOrFail();
86+
87+
if ($refund->mollie_refund_status !== RefundStatus::PENDING) {
88+
return;
89+
}
90+
91+
$refundItems = $refund->items;
8492
$orderItems = $refundItems->toNewOrderItemCollection()->save();
8593
$order = Cashier::$orderModel::createProcessedFromItems($orderItems);
8694

87-
$this->order_id = $order->id;
88-
$this->mollie_refund_status = RefundStatus::REFUNDED;
95+
$refund->order_id = $order->id;
96+
$refund->mollie_refund_status = RefundStatus::REFUNDED;
8997

90-
$this->save();
98+
$refund->save();
9199

92100
$refundItems->each(function (RefundItem $refundItem) {
93101
$originalOrderItem = $refundItem->originalOrderItem;
@@ -98,29 +106,49 @@ public function handleProcessed(): self
98106
}
99107
});
100108

101-
$this->originalOrder->increment('amount_refunded', (int) $refundItems->getTotal()->getAmount());
109+
$refund->originalOrder->increment('amount_refunded', (int) $refundItems->getTotal()->getAmount());
110+
111+
$handled = true;
102112
});
103113

104-
event(new RefundProcessed($this));
114+
$this->refresh();
115+
116+
if ($handled) {
117+
event(new RefundProcessed($this));
118+
}
105119

106120
return $this;
107121
}
108122

109123
public function handleFailed(): self
110124
{
111-
$refundItems = $this->items;
125+
$handled = false;
112126

113-
DB::transaction(function () use ($refundItems) {
114-
$this->mollie_refund_status = RefundStatus::FAILED;
127+
DB::transaction(function () use (&$handled) {
128+
/** @var static $refund */
129+
$refund = static::whereKey($this->getKey())->lockForUpdate()->firstOrFail();
115130

116-
$this->save();
131+
if ($refund->mollie_refund_status !== RefundStatus::PENDING) {
132+
return;
133+
}
134+
135+
$refundItems = $refund->items;
136+
$refund->mollie_refund_status = RefundStatus::FAILED;
137+
138+
$refund->save();
117139

118140
$refundItems->each(function (RefundItem $refundItem) {
119141
$refundItem->originalOrderItem->handlePaymentRefundFailed($refundItem);
120142
});
143+
144+
$handled = true;
121145
});
122146

123-
event(new RefundFailed($this));
147+
$this->refresh();
148+
149+
if ($handled) {
150+
event(new RefundFailed($this));
151+
}
124152

125153
return $this;
126154
}

‎tests/Http/Controllers/AftercareWebhookControllerTest.php‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -111,17 +111,26 @@ public function itDetectsNewRefunds()
111111
$mollieRefund->id = $mollieRefundId;
112112
$mollieRefund->status = MollieRefundStatus::REFUNDED;
113113

114-
$molliePayment = $this->getMockBuilder(MolliePayment::class)
115-
->setConstructorArgs([new MollieApiClient])
116-
->onlyMethods(['refunds'])
117-
->getMock();
118-
119-
$molliePayment
120-
->method('refunds')
121-
->willReturn(new RefundCollection(
122-
$this->createMock(\Mollie\Api\Contracts\Connector::class),
123-
[$mollieRefund],
124-
));
114+
$mollieRefunds = new RefundCollection(
115+
$this->createStub(\Mollie\Api\Contracts\Connector::class),
116+
[$mollieRefund],
117+
);
118+
119+
$molliePayment = new class(new MollieApiClient, $mollieRefunds) extends MolliePayment {
120+
private RefundCollection $refunds;
121+
122+
public function __construct(MollieApiClient $client, RefundCollection $refunds)
123+
{
124+
parent::__construct($client);
125+
126+
$this->refunds = $refunds;
127+
}
128+
129+
public function refunds(): RefundCollection
130+
{
131+
return $this->refunds;
132+
}
133+
};
125134

126135
$molliePayment->id = $molliePaymentId;
127136
$molliePayment->status = MolliePaymentStatus::PAID;

0 commit comments

Comments
 (0)