Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrency in Payum #881

Open
dpfaffenbauer opened this issue Jan 15, 2021 · 2 comments
Open

Concurrency in Payum #881

dpfaffenbauer opened this issue Jan 15, 2021 · 2 comments

Comments

@dpfaffenbauer
Copy link
Contributor

We recently ran into concurrency issues with Payum. It happens quite a lot that the notify and capture (redirect) happen at the exact same time, resulting in the payment being updated twice and depending on your installation, executing other stuff multiple times. (like workflows or stuff)

We solved this problem a bit "hacky" where it could be solved quite easily in payum directly. The solution is to pessimistic write lock the payment entity. That way the first request blocks the other one to further process the payment.

Here is how we did it:

I think this should be discussed here to further prevent such issues for other people/projects using payum.

@Tetragramat
Copy link
Contributor

We solved it with locking whole payment in payum using redis store lock.
First request locks payment and all subsequent request get 423 response with 5 sec meta refresh and button to continue to after url.

<?php

namespace App\EventListener\Payum;

use App\Entity\PaymentToken;
use Payum\Core\Bridge\Symfony\Event\ExecuteEvent;
use Payum\Core\Extension\Context;
use Payum\Core\Model\Identity;
use Payum\Core\Model\PaymentInterface;
use Payum\Core\Model\PayoutInterface;
use Payum\Core\Reply\HttpResponse;
use Payum\Core\Request\Generic;
use Payum\Core\Request\RenderTemplate;
use Symfony\Component\Lock\LockFactory;
use Symfony\Component\Lock\LockInterface;
use function get_class;
use function method_exists;
use function parse_str;
use function parse_url;
use function urldecode;
use const PHP_URL_QUERY;

class LockingListener
{
	private const LOCK_TTL = 30.0;

	private ?LockInterface $lock = null;
	private LockFactory $lockFactory;

	public function __construct(LockFactory $lockFactory)
	{
		$this->lockFactory = $lockFactory;
	}

	public function onPreExecute(ExecuteEvent $event) : void
	{
		if (!$this->lock && $identifier = $this->getModelIdentifier($event->getContext()->getRequest())) {
			$this->lock = $this->lockFactory->createLock('Payum:' . $identifier, self::LOCK_TTL);

			if (!$this->lock->acquire()) {
				$this->lock = null;

				throw $this->getResponse($event->getContext());
			}
		}
	}

	public function onPostExecute(ExecuteEvent $event) : void
	{
		// run only for first level (last execution in stack)
		if (!empty($event->getContext()->getPrevious())) {
			return;
		}

		if ($this->lock) {
			$this->lock->release();
			$this->lock = null;
		}
	}

	private function getResponse(Context $context): HttpResponse
	{
		$request = $context->getRequest();

		if ($request instanceof Generic && $request->getToken()) {
			if ($request->getToken()->getAfterUrl() !== null) {
				$afterUrl = $this->extractAfterUrl($request->getToken()->getAfterUrl());
			} else {
				$afterUrl = $this->extractAfterUrl($request->getToken()->getTargetUrl());
			}
		} else {
			$afterUrl = null;
		}

		$renderTemplate = new RenderTemplate('Payum\error_locked.html.twig', [
			'afterUrl' => $afterUrl,
		]);

		$context->getGateway()->execute($renderTemplate);

		return new HttpResponse($renderTemplate->getResult(), 423);
	}

	private function extractAfterUrl(string $url): ?string
	{
		parse_str(parse_url($url, PHP_URL_QUERY), $query);

		return isset($query['afterUrl']) ? urldecode($query['afterUrl']) : null;
	}

	/**
	 * @param object $request
	 *
	 * @return string|null returns className#ID
	 */
	private function getModelIdentifier(object $request) : ?string
	{
		if (!$request instanceof Generic) {
			return null;
		}

		$model = $request->getModel();

		if ($model instanceof PaymentToken) {
			return (string) $model->getDetails();
		} elseif ($model instanceof Identity) {
			return (string) $model;
		}

		$firstModel = $request->getFirstModel();

		if (($firstModel instanceof PaymentInterface || $firstModel instanceof PayoutInterface) && method_exists($firstModel, 'getId')) {
			return get_class($firstModel) . '#' . $firstModel->getId();
		}

		return null;
	}
}

@lruozzi9
Copy link

Partially inspired by @Tetragramat's comment we have just released a Payum extension that provides the ability to lock concurrent requests: https://github.com/webgriffe/PayumLockRequestExtension. There is also a Symfony bundle: https://github.com/webgriffe/PayumLockRequestExtensionBundle.

We are currently using it in a production environment and it seems to work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants