fix(dav): compare sync token retention against an absolute cutoff#60178
fix(dav): compare sync token retention against an absolute cutoff#60178mooons wants to merge 1 commit into
Conversation
8c595a8 to
938e2c1
Compare
|
I have restored the PR template. Please fill it out. |
ChristophWurst
left a comment
There was a problem hiding this comment.
Changes make sense
Thanks for the contribution!
tcitworld
left a comment
There was a problem hiding this comment.
Logic looks good, but tests say no.
|
Hi @mooons Thank you for the contribution! Could you also fix the failing tests? There were 2 failures:
/home/runner/actions-runner/_work/server/server/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php:1403
/home/runner/actions-runner/_work/server/server/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php:880 |
Signed-off-by: mooons <10822203+mooons@users.noreply.github.com>
938e2c1 to
df48a6a
Compare
Done. I've pushed the test fixes. Thanks for reviewing this! |
|
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.) |
Summary
PruneOutdatedSyncTokensJobcomputesretentionas a duration in seconds fromsyncTokensRetentionDays, but both DAV backends currently compare that value directly againstcreated_atinpruneOutdatedSyncTokens().created_atstores Unix timestamps, so this ends up using a relative duration as if it were an absolute timestamp. With the default 60-day retention, the generated condition is effectivelycreated_at <= 5184000, which does not match normal rows.Fix
Convert the duration into an absolute cutoff inside the backend methods before building the delete query:
and compare
created_atagainst$cutoff.This preserves the current method signatures and config semantics.
Root cause
The retention value is produced as an age duration in
PruneOutdatedSyncTokensJob, but treated as an absolute timestamp in:OCA\DAV\CalDAV\CalDavBackend::pruneOutdatedSyncTokens()OCA\DAV\CardDAV\CardDavBackend::pruneOutdatedSyncTokens()Impact
Without this fix, old rows in
oc_calendarchangesandoc_addressbookchangesare not pruned as intended even when the background job runs.Related: #51120
Checklist
3. to review, feature component)stable32)AI (if applicable)