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

feat(cardav): support result truncation for addressbook federation #50092

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Jan 8, 2025

  • Resolves: #

Summary

TODO

  • ...

Checklist

@hamza221 hamza221 self-assigned this Jan 8, 2025
@hamza221 hamza221 added 3. to review Waiting for reviews 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 8, 2025
@AndyScherzinger AndyScherzinger added this to the Nextcloud 31 milestone Jan 8, 2025
@hamza221 hamza221 force-pushed the feat/sync-truncation branch from c7ee3ab to 1cb4891 Compare January 10, 2025 08:08
@@ -851,6 +855,10 @@ public function deleteCard($addressBookId, $cardUri) {
* @return array
*/
public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel, $limit = null) {
if ($limit === null) {
$limit = $this->config->getSystemValueInt('carddav_sync_request_limit', 1);
Copy link
Member

@st3iny st3iny Jan 14, 2025

Choose a reason for hiding this comment

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

TODO: Debugging leftover has to be removed (default limit 1) before the merge.

@hamza221 hamza221 marked this pull request as ready for review January 21, 2025 12:37
@hamza221 hamza221 added feature: carddav Related to CardDAV internals feature: federation labels Jan 21, 2025
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

How to test this? Does this also work with clients or is it limited strictly to server-to-server synchronization?

apps/dav/lib/CardDAV/CardDavBackend.php Outdated Show resolved Hide resolved
@hamza221
Copy link
Contributor Author

hamza221 commented Jan 21, 2025

How to test this?

The limit is set to 1 here. The ways to test this that I can think of

  • Intercept the requests and check the sync token changes ,+ results (make sure that all the cards are synched, with the correct number of steps and that the sync token always reflects the correct state)
  • Logs or breakpoints can also be used

PS: In case of federation both instances need to be patched.

Does this also work with clients or is it limited strictly to server-to-server synchronisation?

It should work for client sync too, But testing server to server is also needed to cover the whole PR

This was referenced Jan 21, 2025
@blizzz blizzz mentioned this pull request Jan 29, 2025
1 task
@blizzz blizzz modified the milestones: Nextcloud 31, Nextcloud 32 Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants