[dev] Kronolith_Integration_ToIcalendarTest and dealing with recurrence exceptions

[Gunnar Wrobel]

Quoting Michael J Rubinsky <mrubinsk at horde.org>:

> 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?

Completely my mistake. I still had it in a private branch and believed  
I pushed it. Available in master now.

>
>> 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).

Okay, thanks! This allowed me to complete converting the test.

>
>
>> 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.

Okay, definitely somewhat more complex than I initially assumed. I  
guess I will need to fix that part in the Kolab driver to understand  
it fully and will get back to it then.

I added a new Mock driver that allowed me to circumvent generating a  
complete SQL setup.

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

Thanks for clarifying this for me. I was able to convert the complete  
Kronolith test suite now which is nice. The tests in  
Kronolith_Integration_ToIcalendarTest are still marked incomplete but  
these are just minor differences between the older test fixture and  
the current test output. All of them fail when dealing with recurrence  
exceptions. Could this be a problem with using the mock driver? Maybe  
you could have a look. "testBasicVersion1" also fails because of  
quoting in the iCalendar output. This is probably Jan's area.

Cheers,

Gunnar

>
> -- 
> mike
>
> The Horde Project (www.horde.org)
> mrubinsk at horde.org
>
> -- 
> Horde developers mailing list
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org

-- 
Core Developer
The Horde Project

e: wrobel at horde.org
t: +49 700 6245 0000
w: http://www.horde.org

pgp: 9703 43BE
tweets: http://twitter.com/pardus_de
blog: http://log.pardus.de