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

Download pictures on demand, not during sync #247

Closed
avv-github opened this issue Oct 1, 2018 · 4 comments
Closed

Download pictures on demand, not during sync #247

avv-github opened this issue Oct 1, 2018 · 4 comments
Assignees
Milestone

Comments

@avv-github
Copy link

avv-github commented Oct 1, 2018

Hello, and congratulations on the great job so far.
I find this plugins very useful, and use it along with ownCloud 10 (early adopter since 6)

Now that my servers moved to roundcube 1.3, I was able to deploy your plugin, and connect to the ownCloud server nicely (no other carddav configured).
Installation was performed on Debian 9.4 using tarball carddav-3.0.3.tar.bz2

I noticed a strange behaviour though: if 2 accounts share the same carddav account, it seems the population process never ends... +/- 1000 cards.

Any idea or fix in the pipe? :)

Cheers,

A

@avv-github
Copy link
Author

Update: actually, the issue is not caused by this concurrent race... but rather by a timeout getting a picture from the web, which interrupts the process (and generate errors, I had to reisntal to make it clear)

Error:
7 Failed to connect to 212.190.223.210 port 80: Connection timed out in /usr/share/roundcube/plugins/carddav/vendor/nategood/httpful/src/Httpful/Request.php:1028

Hope this help.

Feel free if you need more information of course.

Cheers,

A

@mstilkerich
Copy link
Owner

Hello,

first sorry for the very late reply. I think this issue is still present even in the current development version. I have on my agenda to postpone the download of pictures to when they are requested for the first time, but at the moment there is higher priority topics to solve. In most cases, the pictures will be hosted by the same server as the addressbook, i.e. if the server is not reachable, the sync would not work anyway.

@mstilkerich mstilkerich self-assigned this Jul 18, 2020
@mstilkerich mstilkerich changed the title Same carddav used by 2 different accounts blocks the process Download pictures on demand, not during sync Oct 31, 2020
@mstilkerich mstilkerich added this to the 4.1.0 milestone Oct 31, 2020
@mstilkerich
Copy link
Owner

The largest amount of data that is transmitted during a sync is normally caused by contact pictures stored inside the VCards. These photos, however, are only needed for contacts that are either:

  • viewed in the addressbook
  • from which email was received and is viewed

At least in my case, my addressbook includes many contacts for which the photo will thus rarely be needed. Even then, we can distribute the time spent to get the photos over time, avoiding a single operation running very long.

TODOs:

  • It is possible to reference photos as URIs from a vcard, but even in this case the plugin will fetch these photos during the sync. Delay fetching to when the photo is requested first.
  • The multi-get request allows to ask for specific properties in the received vcards (if server supports it). In this case, we can speed up sync even for cases where the photos are stored inside the vcards. We would have to mark such partial vcards in the database and fetch the full vcard upon request of properties not received during the initial sync, including the photo.

@mstilkerich
Copy link
Owner

The delayed download part is fixed in v4.1. The selective syncing of VCard using the multiget request is currently not implemented, because I found no server that supports it. iCloud doesn't really need it, as it stored the photo externally -> this is addressed by what has been improved in this ticket. Sabre/Dav (and therefore most open-source carddav servers: owncloud, nextcloud, baïkal) currently only supports selective retrieval of VCards for the addressbook-query request, but not for multiget. I created a pull request to support it (sabre-io/dav#1310). If it gets accepted, I will revisit that part in the future, it would still take a while until adopted by owncloud/nextcloud/Baïkal.

Some details on how the delayed photo download implemented in v4.1 works:

  • When a photo is stored inline in the VCard and can be used as is, nothing changes
  • When a photo needs processing, the processing is done on first usage of the photo (not during retrieval of the VCard). Processing currently can be:
    • Download of an externally referenced photo
    • Cropping of the photo (X-ABCROP-RECTANGLE extension)
  • After a photo has been processed for the first time, it is stored in the roundcube cache
    • When a VCard is updated and the PHOTO property changes, the cached photo is replaced when it is requested first
    • Otherwise, cached photos are deleted upon garbage collection by roundcube; the current expiry time is set to 1 week
    • Currently the roundcube database cache is used, but I intend to allow selection of other backends (e.g. redis) in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants