Multi vcf download plugin

[skjnldsv]

@charismatic-claire @brjhaverkamp

This plugin allow us to POSTGET a list of vcards as a multi dimensional array and retrieve everything as a single vcf file. Needed for nextcloud/contacts#39

To discuss:

1. shall we go with a GET instead? DONE
2. shall we use a simple array instead of a multi dimensional one? DONE

curl 'https://dev.skjnldsv.com/remote.php/dav/addressbooks/users/admin/contacts/?export' \
	-X REPORT  --user 'admin:admin' \
	--data-binary '<x1:addressbook-multiget xmlns:x1="urn:ietf:params:xml:ns:carddav"><x0:prop xmlns:x0="DAV:"><x0:getcontenttype/><x0:getetag/><x0:resourcetype/><x0:displayname/><x0:owner/><x0:resourcetype/><x0:sync-token/><x0:current-user-privilege-set/><x0:getcontenttype/><x0:getetag/><x0:resourcetype/><x1:address-data/></x0:prop><x0:href xmlns:x0="DAV:">83F18885-8FF6-4839-9D10-C997D5BD9B96.vcf</x0:href><x0:href xmlns:x0="DAV:">470623A2-0FCB-4BA0-A6D2-6C25F4113C02.vcf</x0:href></x1:addressbook-multiget>'

[ChristophWurst]

1. shall we go with a GET instead?

If that call does not change any server state, yes.

[skjnldsv]

If that call does not change any server state, yes.

Yes, I'm thinking that as well, but since muti dimentionnal arrays are terribly looking as url, I wonder if some browsers can go 💥 with that?
/remote.php/dav/addressbooks/users/admin/?vcards%5Bcontacts%5D%5B%5D=3A6B15C7-EDCF-4F14-B9DB-D427806B82A2.vcf&vcards%5Btest%5D%5B%5D=790AD673-6C80-47F9-95A7-89E9AD21B801.vcf

[rullzer]

Also you hit the various browser url length limits at some point

[skjnldsv]

Also you hit the various browser url length limits at some point

Arf, damn!
What should I do then? :(

Please update

[ChristophWurst]

Some general coding style feedback. Can't argue much about the logic as I'm not familiar with the details of DAV and VCF 🤷‍♂️

[ChristophWurst]

Suggested change

	$addressbooks = array_merge($addressbooks, [$addressbook => $vcards]);
	$addressbooks[$addressbook] = $vcards;

would do the exact same I guess

[skjnldsv]

Actually I can drop this I think now. I don't use $addressbooks anymore! 👍

[rullzer]

Also you hit the various browser url length limits at some point

Arf, damn!
What should I do then? :(

Well using POST helps for that. With GET you could run into isues if people select their whole addressbook

[skjnldsv]

Well using POST helps for that. With GET you could run into isues if people select their whole addressbook

Hum, no better ideas out there? Anyone?

[MorrisJobke]

Hum, no better ideas out there? Anyone?

This is the only way I also have in mind :/

[ChristophWurst]

Also you hit the various browser url length limits at some point

Found https://stackoverflow.com/a/2659995/2239067 and https://stackoverflow.com/a/417184/2239067 telling me the limit is at ~2KB. Based on the example URL provided I assume a typical URL parameter is up to 100 bytes longs, allowing just 20 contacts/parameters.

Since this is DAV anyway, this answer might be relevant: https://stackoverflow.com/questions/978061/http-get-with-request-body#comment10368662_978061

IMO using POST for this seems wrong as it's an idempotent operation. Still, GET is also not right, esp. with the technical limitations.

[charismatic-claire]

How about the REPORT request, that was specifically invented for that kind of a request against a CardDAV server? See Building a CardDAV client, search for the keyword "multiget".

Or is that impossible, because we are limited to GET, POST, PUT and DELETE here?

[georgehrke]

@skjnldsv I don't think we need a dedicated plugin for that. You can perform a simple addressbook-multiget, put them all together into one big vcf in javascript and use a data-uri link to download.

[georgehrke]

How about the REPORT request, that was specifically invented for that kind of a request against a CardDAV server? See Building a CardDAV client, search for the keyword "multiget".

@charismatic-claire I agree that multiget is the way to go, but please take note that multiget does not return one big vcf file, but a WebDav like 207-multistatus response with individual vcfs. See https://tools.ietf.org/html/rfc6352#section-8.7.1 :)

[skjnldsv]

@georgehrke @charismatic-claire
So I would then build the vcf in the browser?
That's something I really don't like. If I have a selection of 1000 vcfs, I want to just trigger a download. Any javascript computation will explode the browser, especially if the vcfs are big :(

[charismatic-claire]

@skjnldsv
I think the whole point of building that plugin server side was to not let the browser do the heavy lifting. But I see your point. If the Contacts app fires a multiget request via REPORT and receives lots of single VCFs as a response, it will have to do exactly that... Hm... Can't our plugin just return one single VCF as a result? But that would be against the standard, right? Hm... So the question becomes: Is there a standard for our use case, or do we have to invent one? And if so, is that a good idea? Do I miss something here?

[georgehrke]

Well, you could adapt the VCFExportPlugin to allow appending ?export to the url when sending REPORT requests.

[skjnldsv]

Well, you could adapt the VCFExportPlugin to allow appending ?export to the url when sending REPORT requests.

then build a blob from the report request and trigger a redirect to the blob?
Could be clean yes!!

@charismatic-claire you're right on the point yes!
Thank you both!! I will adapt this later.

@charismatic-claire could you then adapt this behavior to the contacts app? :)
https://www.npmjs.com/package/file-saver

[charismatic-claire]

@skjnldsv I'll be glad to help :-)

[skjnldsv]

@georgehrke done! :)
Should you implement this into the cdavlib then?

[skjnldsv]

@MorrisJobke @rullzer @ChristophWurst a little review for the php?

[ChristophWurst]

Is this plugin even needed now? The discussion above – esp #12004 (comment) and #12004 (comment) – tells me this might not be required if I understand correctly.

[skjnldsv]

@ChristophWurst I updated the plugin to use the multi-get like @georgehrke said :)

If you do a multiget with ?export at the end, it will return a vcf file and not a dav response ;)

curl 'https://dev.skjnldsv.com/remote.php/dav/addressbooks/users/admin/contacts/?export' \
	-X REPORT  --user 'admin:admin' \
	--data-binary '<x1:addressbook-multiget xmlns:x1="urn:ietf:params:xml:ns:carddav"><x0:prop xmlns:x0="DAV:"><x0:getcontenttype/><x0:getetag/><x0:resourcetype/><x0:displayname/><x0:owner/><x0:resourcetype/><x0:sync-token/><x0:current-user-privilege-set/><x0:getcontenttype/><x0:getetag/><x0:resourcetype/><x1:address-data/></x0:prop><x0:href xmlns:x0="DAV:">83F18885-8FF6-4839-9D10-C997D5BD9B96.vcf</x0:href><x0:href xmlns:x0="DAV:">470623A2-0FCB-4BA0-A6D2-6C25F4113C02.vcf</x0:href></x1:addressbook-multiget>'

[charismatic-claire]

@ChristophWurst Maybe every pull request should start with a comprehensive description of the "ticket" we are trying to solve here. I'll try to briefly summarize what we wanted to achieve:

• In the Contacts app, we want to add the possibility to export whole groups of contacts. Say you have 5 address books and 10 groups. What you might want is to export all members from all address books belonging to a certain group. Or maybe you want to search for a certain phrase and want to export all contacts matching that search. Or you want to manually select a number of contacts you want to export.
• By "export" in mean you want to download one single vcf file containing all the vCards of all the contacts you want to export.
• Here is what we wanna do: To achieve this, the interface of the Contacts app shall compile a REPORT request for all the wanted contacts. If you send this to the CardDAV server, it will respond with a list of all the contacts. So the transformation into one single vcf file which could be downloaded would be done on the browser side via JavaScript. This is what @skjnldsv wants to avoid here.
• That's why we need this plugin here. With a export parameter appended to the URL of the REPORT request, this plugin would transform this response into one single vcf file.
• See Currently not possible to download group of contacts for more

Does that make clear what we want and why we need this plugin on the server side? Feel free to ask, if not :-)

[georgehrke]

And please squash all commits :)

DONE :D

[ChristophWurst]

Thank you, @charismatic-claire ✌️

[ChristophWurst]

Fine, I guess 🐘

[skjnldsv]

Failure unrelated