Safely save data to the order in onNotify()

Question

I'm new to Drupal and have to make an off-site payment gateway (with Drupal Commerce 2). It all works, but sometimes it doesn't.

The remote payment provider's server sends notification requests to the server, about the status of the payment, so I have both onReturn and onNotify in the PaymentGateway class.

Since onReturn is not guaranteed to be called (the customer might close the browser, etc, and the provider does not necessarily send him back in my case), but onNotify is guaranteed to be called, I create and save the Payment object in onNotify, not in onReturn, when the payment is complete. (This is also what the documentation suggests to do: https://docs.drupalcommerce.org/commerce2/developer-guide/payments/create-payment-gateway/off-site-gateways/handling-ipn)

So my code looks something like this. (It's a very simplified pseudo-code; validation checks aren't included.)

class RedirectCheckout extends OffsitePaymentGatewayBase implements SupportsNotificationsInterface {

  public function onReturn() {
    $is_order_accepted = /* Check the remote payment provider has accepted the order */
    if (!$is_order_accepted) {
       throw new NeedsRedirectException()
    }
    // If all is good, do nothing.
  }

  public function onNotify() {
    /** @var OrderInterface $order */
    $order = /* Load the order the notification is about */

    $is_order_accepted = /* Check the remote payment provider has accepted the order */
    if ($is_order_accepted) {
      $payment = $payment_storage->create();
      $payment->save();
      $order->setData('transaction_id', $transactionId);
      $order->save(); // This is what sometimes overwritten by  onReturn(), I believe.
    }
  }
}

Notice that I need to save some data on the order, when an order is accepted (which is not available when the order is created, only when a payment has succeeded).
The Drupal Commerce documentation says you "don't need to (and should not)" touch the order, but I have to save some extra data on the order that other parts on the system expects to be there.

This often works. However, the two onReturn and onNotify requests from the remote server sometimes arrives at almost the same time, which I believe leads to a race condition.

Unfortunately, even though I am not doing anything to the order in onReturn, Commerce seems to still save the order. I believe this can sometime overwrite the data saved to the order by onNotify. For example:

• onReturn starts running, and loads the order (This is done by the commerce library itself, so I can't do anything about this.)
• The notify request arrives, so onNotify starts running, loads and saves the order, and returns
• After this, the onReturn method returns, gives back control to Commerce, which saves the order again; since it loaded the order object before onNotify saved it, it overwrites whatever onNotify wrote with old data

(Perhaps the converse order can also be problematic, where onNotify can overwrite any data saved to the order by Drupal Commerce behind the scenes, if any, during the onReturn request.)

Is there a good way to handle this, for example work around the race-condition to be able to save order data in onNotify?

I'm using Drupal 8.6.

Answer 1

You are right about race condition, but onReturn does not modify order at all

This is what happens in the background:

• the commerce_payment.checkout.return route on commerce payment module is called which at first it let order payment gateway plugin to create an payment for order
• then no matter what happens in the onReturn method of plugin order checkout flow will be redirected on another stage (if onReturn throws exception it will be redirected to previous stage else it will be redirect to next stage)
• in this situation the method redirectToStep in the Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase will modify and save order which is where the problem happens.

this is the implementation of redirectToStep:

/**
 * {@inheritdoc}
 */
public function redirectToStep($step_id) {
  if (!$this->isStepVisible($step_id)) {
    throw new \InvalidArgumentException(sprintf('Invalid step ID "%s" passed to redirectToStep().', $step_id));
  }

  $this->order->set('checkout_step', $step_id);
  $this->onStepChange($step_id);
  $this->order->save();

  throw new NeedsRedirectException(Url::fromRoute('commerce_checkout.form', [
    'commerce_order' => $this->order->id(),
    'step' => $step_id,
  ])->toString());
}

so to solve the problem in your onReturn method you should stand for onNotify changes to be done (a while loop which waits until changes on order are set)

hopes it helps.