sabre-io / dav

sabre/dav is a CalDAV, CardDAV and WebDAV framework for PHP
http://sabre.io
BSD 3-Clause "New" or "Revised" License
1.53k stars 346 forks source link

Support for Multiget in CalDAV ICSExportPlugin #761

Open rentmanpublic opened 8 years ago

rentmanpublic commented 8 years ago

Hi everybody,

I'm currently writing an CalDAV server which communicates with our backend through a REST-API. This is working great with CalDAV but i'm experiencing problems with the ICSExport. Since their seems to be no support for the Multiget each individual calendar gets downloaded one by one and thus resulting in an API-call each.

Is there any way to make smarter use of the 'getMultipleCalendarObjects'-method during the ICSExport?

evert commented 8 years ago

Hi!

I took a look at the actual source, and from what I can tell this is already happening. I assume you developed your own caldav backend, are you returning the calendardata key for each calendar object from that function? Which version are you running?

rentmanpublic commented 8 years ago

Hi Evert,

Thanks for your reply. Yes I created an custom caldav backend which extends the AbstractBackend similar to the PDO example. We're running sabre/dav 3.1.

The function 'getMultipleCalendarObjects' returns an array of vObjects. A codesnippet:

foreach(...)
{
$details = Rentman\FunctionFactory::calendarDetails($rf, $calendarId);
$details['calendardata'] = Rentman\FunctionFactory::makeVObject($rf,$project);
$result[] = $details;
}
return $result;

The functions doesn't seems to be called at all, when I throw an Exception or die() within it doesnt trigger. Do I have to enable anything?

evert commented 8 years ago

That's pretty odd... I'll have to investigate a bit, because the function definitely should be called for those cases.

rentmanpublic commented 8 years ago

The abstract backend supports getMultipleCalendarObjects and getCalendarObjects. When I return the calenderdata within getCalendarObjects the individual objects don't get asked and I could deliver all data in one query.

But this seems not the way to go because if I implement it like this it gets pretty expensive to return the etags in a default CalDAV configuration. Shouldn't it be the case that getCalendarObjects returns all etags and all events that have a different etag are queried by getMultipleCalendarObjects?

evert commented 8 years ago

Actually I also mis-read my own code there... getMultipleCalendarObjects is only used when you are searching for specific events. An example in the iCalendar export plugin is that it's used when you specify a time-range.

If you don't specify any filters or time-ranges, the plugin will simply do getCalendarObjects. If getCalendarObjects returns a calendardata for each item, you will not see individual requests to getCalendarObject. The drawback is, as you said, that you pre-fetch the calendar data whether you need it or not.

In reality there are actually few HTTP requests that will result in getCalendarObjects that don't also need calendardata, but there are some, such as a plain PROPFIND requests. CalDAV clients generally won't make these requests.

So you basically have two options:

  1. Return calendardata from getCalendarObjects.
  2. Only do that if you detect that this is part of a request that will need that data... this is a bit of a hack but you could for example 'warm the cache' for these types of requests. This is significantly harder though.

I don't really have a good sabre/dav answer for this, but I do think it's worth looking into. I'm keeping this ticket open, but I'm considering it a bit more of a long-term thing though...

evert commented 8 years ago

But this seems not the way to go because if I implement it like this it gets pretty expensive to return the etags in a default CalDAV configuration. Shouldn't it be the case that getCalendarObjects returns all etags and all events that have a different etag are queried by getMultipleCalendarObjects?

Re-reading this again, I think it's important to know that most modern CalDAV clients will not use this mechanism to read etags. They will use sync-collection instead, which is quite cheap (and totally worth implementing).

But I agree that those specific calls could get more expensive. I hope to solve this in the future by using generators there. It won't reduce the total amount of bytes sent from your backend to PHP, but at least all those bytes are not kept in memory at the same time.