Bugfix/36644 pruneOutdatedSyncTokens deletes all entries#38639
Bugfix/36644 pruneOutdatedSyncTokens deletes all entries#38639szaimen merged 2 commits intonextcloud:masterfrom charno:bugfix/36644-pruneOutdatedSyncTokens-deletes-all-entries
Conversation
charno
requested review from
ChristophWurst,
miaulalala and
tcitworld
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@tcitworld Thanks for the feedback, I'll have a look in the evening. Do you know by chance how I can run the linters using vs code and the dev container? Also I just noticed that you tagged this for Version 28. Do you plan to backport this to 26 / 27? |
|
/backport to stable27 |
|
/backport to stable26 |
Unfortunately I don't know. However, it seems the only issues you have should be resolved alongside with the issues I pointed out above.
Yup, that's planned. |
|
Wow, good catch! |
Signed-off-by: Christof Arnosti <[email protected]>
|
More detailed testing (request from Issue) and cleanups are done. Now for hoping linter / psalm is happy |
|
@kesselb Can you also add / push the changes to CardDavBackend? I'm away from my dev workstation for the next days |
|
I'll do some cleanup on weekend (Port changes done to Cal to Card, squishing formatting commits), then from my side I think it is finished |
|
I have done the formatting changes, now hopefully everything should be fine |
pruneOutdatedSyncTokens accidentally deletes all entries of the calendarchanges table instead of leaving $limit elements in the table Signed-off-by: Christof Arnosti <[email protected]>
|
Thanks for reviewing! I'm happy when this bug is gone, this one was especially painful for me |
st3iny
added
4. to release
|
/backport to stable25 |
|
CI failure unrelated |
|
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
|
/backport to stable26 |
|
/backport to stable27 |
|
Thanks for merging!
This will probably not be possible since the functionality this fixes is not present in 25 |
|
The backport to stable27 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b fix/foo-stable27
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27 |
1 similar comment
|
The backport to stable27 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b fix/foo-stable27
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27 |
|
The backport to stable26 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable26
git pull origin stable26
# Create the new backport branch
git checkout -b fix/foo-stable26
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26 |
1 similar comment
|
The backport to stable26 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable26
git pull origin stable26
# Create the new backport branch
git checkout -b fix/foo-stable26
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26 |
|
The backport to stable25 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable25
git pull origin stable25
# Create the new backport branch
git checkout -b fix/foo-stable25
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25 |
7 participants
Summary
With Nextcloud 26, CalDAV / CardDAV Clients which are using SyncToken (for example Thunderbird) don't receive calendar / contact updates which happened before the last run of
pruneOutdatedSyncTokens. During collaborative research we found out thatpruneOutdatedSyncTokensdoes delete all entries of calendarchanges / addressbookchanges by accident.This pull request changes the
pruneOutdatedSyncTokensfunctions to first select the highest ID in the*changestable and use it in a WHERE clause during deletion of the tokens.Sine calendar on Thunderbird / maybe other clients are regularly missing events I would say this is a quite high impact bug. For me and others, calendar syncing is currently practically unusable.
Checklist