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

Prevent publishing of the same page at the same time #2612

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

sascha-karnatz
Copy link
Contributor

What is this pull request for?

Introduce the idea of a PageMutex, which is locking the page for the time of an operation and prevent multiple updates at the same time. The PageMutex is now used in the Publisher to prevent multiple updates of the same page.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@sascha-karnatz sascha-karnatz requested a review from a team November 14, 2023 09:17
@sascha-karnatz sascha-karnatz force-pushed the harden-page-publisher branch 2 times, most recently from 48edabe to a2a844a Compare November 14, 2023 09:32
The idea is to provide a possibility to lock a page inside a transaction to prevent, that two (or more) processes working on the same page. This will be included into the Page::Publisher to prevent, that in parallel running worker are updating the same page and creating duplicate entries. This implementation is heavily inspired by Solidus Spree::OrderMutex.
Prevent publishing the same page at the same time. The PageMutex is locking the page for the time it takes until the page is published. After that the lock is released and it can publish again.
@sascha-karnatz sascha-karnatz force-pushed the harden-page-publisher branch 2 times, most recently from 978c193 to 5e0a246 Compare November 14, 2023 09:58
The CI test pipeline breaks for Rails 7.1. The eager loading results into a Zeitwerk error in the turbo-rails gem. The current solution is to include actioncable if the application is running in a Github Action until this bug is fixed.

Ref: hotwired/turbo-rails#512
@sascha-karnatz sascha-karnatz added the backport-to-7.0-stable Needs to be backported to 7.0-stable label Nov 14, 2023
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@tvdeyen tvdeyen merged commit ee7ec8b into AlchemyCMS:main Nov 15, 2023
30 checks passed
@alchemycms-bot
Copy link
Collaborator

💚 All backports created successfully

Status Branch Result
7.0-stable

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@sascha-karnatz sascha-karnatz deleted the harden-page-publisher branch November 15, 2023 08:10
tvdeyen added a commit that referenced this pull request Nov 15, 2023
[7.0-stable] Merge pull request #2612 from sascha-karnatz/harden-page-publisher
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-7.0-stable Needs to be backported to 7.0-stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants