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

[Bug] CartBlamerListener is never called on carts as it listens on incorrect event #679

Open
diimpp opened this issue Oct 17, 2020 · 3 comments
Labels

Comments

@diimpp
Copy link
Member

diimpp commented Oct 17, 2020

CartBlamer's purpose is to be called on each request by authenticated customer and in case current request is one of /carts/{token} requests, then attach current customer to the cart.

This never happens and therefore AssignCustomerToCart is not called until after checkout is completed.

<tag name="kernel.event_listener" event="lexik_jwt_authentication.on_jwt_created" method="onJwtLogin" />

Reason is that it listens to on_jwt_created (Which happens only on login itself, no cart token there obliviously) instead of on_jwt_authenticated. https://github.com/lexik/LexikJWTAuthenticationBundle/blob/master/Events.php (on_authentication_success doesn't seems to work regardless the name or more suited event class)

I think more proper solution would be to listen to kernel.request event and check if current user is authenticated, then it will decouple plugin from any specific authentication library.

Another problem is the the way carts route is detected - by trying to find cart under any route with {token} argument.
Maybe we can do some kind of shop-api firewall check or match /shop-api/carts route prefix.

@diimpp
Copy link
Member Author

diimpp commented Oct 17, 2020

$token = $request->request->get('token');

Also it should be $request->attributes->get('token'), not $request->request->get('token')

@lchrusciel lchrusciel added the Bug label Oct 23, 2020
@lchrusciel
Copy link
Member

Some resolution of /shop-api/cart routes makes sense to me.

This was done this way because I wanted to reduce the amount of calls to CartBlamer. Previously, due to symfony.on_iteraction_login event was trigger with every request (due to being stateless). This resulted in the recalculation logic of the cart for each request. From this moment it was pretty easy to run into race conditions with higher traffic.

@diimpp
Copy link
Member Author

diimpp commented Oct 23, 2020

Extra race condition/deadlocks are avoidable with check if customer already assigned, so no extra order processing would be necessary.

By the way, race conditions are pretty heavy right now without this call at all. They happen at concurrent order processing calls per same cart per customer.
We had this issue on my projects for months, until I've found a solution with LockingOrderProcesser decoration with symfony/lock as described at #667

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

No branches or pull requests

2 participants