[dev] Kronolith_Integration_ToIcalendarTest and dealing with recurrence exceptions

[Michael J Rubinsky]

Quoting Gunnar Wrobel <wrobel at horde.org>:

> Hi!
>
> I started converting more of our application test suites to PHPUnit  
> and I am currently focusing on Kronolith. The test mentioned above  
> ("Kronolith_Integration_ToIcalendarTest") is hard to convert from  
> the Horde 3 version because of the way recurrence exceptions are  
> handled. You can disable the "markTestIncomplete()" in the  
> "ToIcalendarTest" in order to see what I refer to. The problem is  
> caused by the section that handles recurrence exceptions in the  
> Kronolith_Event::toiCalendar() function.

I couldn't find the test that you are referring to here...am I missing  
some new branch or something?

> This sections fetches a complete driver and searches for the  
> "baseid". "baseid" however seems to work only with the SQL backend.

The information obtained by using baseid needs to work for any backend  
that both supports exceptions and can be exported to iCalendar, so we  
will need to look at this. The baseid and exceptionoriginaldate  
properties are used in other places in kronolith code as well. This  
functionality will need to be added to the other drivers that are to  
support exceptions.

> Which I would like to avoid as the amount of setup code - even if  
> done in SQLite - is much more complex and it seems unnecessary to  
> simply export an event to iCal. So I tried using a mock "Ical"  
> driver instead but that one fails because the search function is  
> unhappy about the missing stdClass->start and ->end properties.

This is a bug. Just pushed a fix that allows for empty values for  
these properties in the base driver (recently fixed in the sql driver,  
neglected to check/fix in the base driver class).


> So my impression is that this section is only meant to be valid for  
> the SQL backend and then it would be strange to find it in  
> Kronolith_Event.

No, as stated above, this needs to work for any driver that can  
support exceptions. However, after looking through the code while  
composing this email it's obvious this is currently broken for any  
driver other than SQL, so will need to be fixed.

> I assume the fact that the event needs to query the driver again is  
> an optimization to lazy load the recurrence exceptions only in case  
> they are required.

Sort of. What's happening there is this (my apologies if this is  
already obvious to you - not sure how deep into it you are currently):

$event->recurrence->getExceptions() only ever returns just the dates  
for a recurring event's exceptions. It contains no information  
_at_all_ about what the exception is. It could be that the event is  
totally skipped that day or it could be the event was moved to a  
different time on the same day, or a different day completely.

The function of the baseid field is to tie an event that represents an  
exception to the "parent" recurring event. Similarly, the  
exceptionoriginaldate field ties the exception's event object to a  
specific exception date. These were meant as client code facing  
properties. The driver should now how to use these to get the data  
they need.

In other words, if an exception is added to a recurring event with a  
UID of abc123 that has an exception added on 3/20/2011 that moves the  
event to 3/21/2011: the 3/20/2011 date would exist in the  
getExceptions() result for event abc123, a new event would be created  
for the 3/21/2011 event that would have the baseid eq to abc123, and a  
exceptionoriginaldate property set to 3/20/2011

Ok. So, what's going on in the Kronolith_Event::toiCalendar() method  
(and to a similar extent, in toASAppointment() method) is that we need  
to know more than just what date the exceptions are on. We need to  
know if the event was deleted or, if not deleted, the new event  
details. We search on the baseid property to pull all events that  
represent exceptions to the "parent" event. We then iterate through  
all those exceptions and, using the exceptionoriginaldate property of  
each of those exception events, remove this date from the list of  
known exception dates. Any dates left over after these have been  
removed must be exceptions that are deleted, not just rescheduled.

> I'd be tempted to convert that into a callback function that gets  
> set in the constructor and allows the event to query the recurrence  
> exceptions from the driver on demand.

I agree it might be reasonable to abstract this part out since some  
drivers might be able to do this in a more efficient way, though I'm  
not sure why a callback would be better instead of just a private  
method defined in each Kronolith_Event subclass that can support  
exceptions?

Regardless, we need a consistent way of representing exception  
information on the client facing side - and that's currently the role  
that baseid and exceptionoriginaldate properties play. I'm not at all  
familiar with Kolab to know how it would store recurring  
events/exceptions or obtain this information.

Hope this makes sense...almost 3am here and the brain is not quite up  
to speed :)

-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org