Score:2

Safely save data to the order in onNotify()

jp flag

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.

Gabriel Fernandez avatar
cn flag
Have you tried to use events, such as `commerce_order.place.post_transition`?
jp flag
Interesting idea - i will look into how that works
jp flag
Good suggestion! Unfortunately, according to my tests, onNotify() doesn't place the order by itself (so this event isn't fired), when the payment is added. So the only way to place the order in onNotify(), is to "apply the transition" to the order and then save the order yourself - but then we're back to the concurrency problem above, unfortunately, since both onNotify() and onReturn() saves the order. (And i guess we would also have been back to that problem, if commerce placed the order by itself)
Score:1
cn flag

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.

jp flag
Thanks for your good answer! One question though - even if i in onReturn wait for the onNotify changes to be done, won't "$this->order" still contain the old values from before onNotify was run? (so redirectToStep will save the old values). If so, is there a way to reload "$this->order" in onReturn (after the wait)?
jp flag
(Hmm... And couldn't saving the order in onNotify actually revert the change to "checkout_step" on the order done it redirectToStep to its old value?)
Alireza Tabatabaeian avatar
cn flag
Actually I am not pretty sure about this situation but I guess this will work Ok, give it a try and if things didn't goes very well then we can think about other approaches
mangohost

Post an answer

Most people don’t grasp that asking a lot of questions unlocks learning and improves interpersonal bonding. In Alison’s studies, for example, though people could accurately recall how many questions had been asked in their conversations, they didn’t intuit the link between questions and liking. Across four studies, in which participants were engaged in conversations themselves or read transcripts of others’ conversations, people tended not to realize that question asking would influence—or had influenced—the level of amity between the conversationalists.