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] Core UserCartRecalculationListener triggers order processing for any endpoint for authorized user, resulting in deadlocks #681

Closed
diimpp opened this issue Oct 23, 2020 · 2 comments
Assignees

Comments

@diimpp
Copy link
Member

diimpp commented Oct 23, 2020

If shop user is logged in and have at least one previously assigned cart, then CoreBundle/EventListener/UserCartRecalculationListener will trigger orderProcessing on each authorized request to any shop-api endpoint.

This happens due the fact that ShopApiPlugin doesn't define CartContext and Core\CartContext considers last assigned cart as current cart, which is not true for shop-api. It also expected be triggered only at login in html website, but in shop-api it happens in each request with JWT token.

This results in metric ton of deadlocks for any authorized user and renders SPA app unusable.

Solution is to override listener and disable it.

@mamazu
Copy link
Member

mamazu commented Oct 28, 2020

And then we should just recalculate the cart on every request inside the shop-api/checkout/ routes right?

@diimpp
Copy link
Member Author

diimpp commented Oct 28, 2020

I don't think we should additionally recalculate cart in every request at cart or checkout as most of those endpoints already doing their own order processing.

Original sylius strategy doesn't quite fit shop-api stateless state and any new strategies will heavily depend on how frontend will use api endpoints. (For example new /carts/{token}/refresh, which can be called on login by frontend)

So I think best course of action is to just disable Core\UserCartRecalculationListener and don't do anything else.

/Offtopic,
in my opinion shop-api v1 is kind of done and not salvageable as it was released too early and only v2 with redesign will save it. (All contexts should be re-defined in shop-api, Query object should be introduces in addition to Command, etcetera)
So we should look more in maintenance, than new features.

For example I've implemented out of stock validator in my project, but it's not pretty and runs on modified version of shop-api and it would be plenty of work to make proper PR.

mamazu added a commit that referenced this issue Nov 9, 2020
…mit)

This PR was merged into the 1.0-dev branch.

Discussion
----------

With this the `UserCartRecalculationListener` listens to `lexik_jwt_authentication.on_jwt_created` instead of `security.interactive_login`, recalculating the cart only on an actual login.

Fixes: #667 #681

Commits
-------

a387684 Decorating the UserCartRecalculationListener
f3a3a8c Fixed address book test
adbe2a7 Codestyle
d500d9d Fix phpstan
60b511f Override the CartRecalculationListener instead of decorating it
0929b8b Updated listeners service definition
84367c7 Added compiler pass to remove the core UserCartRecalculationListener
@mamazu mamazu closed this as completed Nov 10, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants