-
Notifications
You must be signed in to change notification settings - Fork 108
feat: Save a checksum for documents and use it to detect conflicts #7677
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
Conversation
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
dcda69b to
d0200ce
Compare
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for looking into this!
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7677 +/- ##
==========================================
- Coverage 59.57% 59.52% -0.06%
==========================================
Files 502 503 +1
Lines 39059 39118 +59
Branches 1129 1130 +1
==========================================
+ Hits 23270 23285 +15
- Misses 15681 15725 +44
Partials 108 108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with Android Notes app and works 🎉
8fcd361 to
1b6cf0d
Compare
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
1b6cf0d to
c4d8dfd
Compare
|
/backport to stable31 |
|
/backport to stable32 |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
@benjaminfrueh I found that it seems less likely but still sometimes happens on our instance that we see a conflict during saves. I managed to see this if a save request happened to take longer (2-3s) and i meanwhile continued typing: Maybe there is a way to reproduce that locally with some manual delay? I'm wondering if we can prevent throwing conflicts during times another php process is currently handling a save |

📝 Summary
Text sometimes throws a
DocumentSaveConflictExceptionwhen a files mtime/etag is updated without changing the content.Solution:
Compute and store a CRC32 checksum for each document. On saving or syncing, the checksum is updated, and
assertNoOutsideConflict()uses it to detect real content changes, preventing false conflicts.🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)