Skip to content

Conversation

@benjaminfrueh
Copy link
Contributor

@benjaminfrueh benjaminfrueh commented Sep 25, 2025

📝 Summary

Text sometimes throws a DocumentSaveConflictException when 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

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
Copy link
Collaborator

@max-nextcloud max-nextcloud left a 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
Copy link

codecov bot commented Sep 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.52%. Comparing base (2545f19) to head (c4d8dfd).
⚠️ Report is 104 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
Copy link
Member

@tobiasKaminsky tobiasKaminsky left a 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 🎉

@benjaminfrueh benjaminfrueh force-pushed the feat/document-checksum branch 3 times, most recently from 8fcd361 to 1b6cf0d Compare October 1, 2025 19:16
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
@benjaminfrueh benjaminfrueh force-pushed the feat/document-checksum branch from 1b6cf0d to c4d8dfd Compare October 1, 2025 19:32
@benjaminfrueh benjaminfrueh self-assigned this Oct 2, 2025
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Oct 2, 2025
@benjaminfrueh benjaminfrueh moved this from 🧭 Planning evaluation (don't pick) to 👀 In review in 📝 Office team Oct 2, 2025
@tobiasKaminsky
Copy link
Member

/backport to stable31

@blizzz blizzz merged commit 7099241 into main Oct 6, 2025
81 of 83 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ☑️ Done in 📝 Office team Oct 6, 2025
@blizzz blizzz deleted the feat/document-checksum branch October 6, 2025 13:10
@mejo-
Copy link
Member

mejo- commented Oct 6, 2025

/backport to stable32

@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

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.)

@juliusknorr
Copy link
Member

juliusknorr commented Oct 21, 2025

@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:
image

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

7 participants