Review Board 1.7.16


Calendaring API for Asterisk

Review Request #58 - Created Nov. 12, 2008 and submitted

Terry Wilson
Reviewers
asterisk-dev
Asterisk
Calendaring integration for Asterisk--currently supporting iCalendar files (hosted over HTTP/HTTPS), CalDAV, and Microsoft Exchange.  Features available: dialplan functions for querying and writing (if the calendar technology supports it like CalDAV and Exchange do) calendar event information, a device state provider for phone BLF support, and calendar event notification through executing dialplan apps or context/exten.

The overall structure is that the res_* calendar modules register as a calendar tech to main/calendar.c, and after all modules are loaded, main/calendar.c parses calendar.conf and calls the calendar_load callback for each register calendar tech based on the "type" field for that calendar in calendars.conf.  Currently each calendar runs in its own thread downloading and refreshing the remote calendar data as necessary as to avoid serializing downloads.  

Eventually it would probably be a good idea for me to implement a thread pool and honor a maximum number of downloads per host:port as well as a global maximum number of simultaneous downloads--perhaps with a min/max refresh value so I could randomize the time for refreshing a bit so as not to be refreshing all of the calendars at once.  Also, there was a request to add support for the dialplan functions to query calendars that aren't in calendar.conf i.e. CALENDAR_QUERY(ical/http://www.google.com/calendar/ical/nkt5atdq7cdbes3ehdfpendpnc%40group.calendar.google.com/public/basic.ics,${EPOCH},$[${EPOCH} + 3600]), etc.

Currently I don't have any support for querying/setting attendees, mostly because it is a list of results whereas all of the others are individual fields.  Writing gets especially ugly for them because of the current format of CALENDAR_WRITE(calendar,${HASHKEYS(calendar)})=${HASH(calendar)}.  I suppose I could add a ...,${HAHSKEYS(attendees)})=...,${HASH(attendees)} to the end...it is just getting a little ugly.
I have tested all three calendar modules by adding, deleting, and moving events and verifying that notifications occur.  I have tested writing to Exchange, and Zimbra and Google Calendar through CalDAV.  I have run with MALOC_DEBUG looking for leaks and dropped references to astobj2 objects. 
Review request changed
Updated (March 17, 2009, 7:16 a.m.)
Updating to reflect changes in response to Russell's review.
Ship it!
Posted (May 22, 2009, 6:46 a.m.)
After this last set of comments, I say we merge this bad boy into trunk!  We can continue to work on things there if needed.

Nice work, Terry!
/configs/calendar.conf.sample (Diff revision 7)
 
 
 
 
If you would like the functionality enabled by default, then I would just make it that way in the code, so it's that way even if you don't have a config file.

However, is "enabled" really necessary?  Wouldn't the definition of calendars in this file imply that it is enabled?
/main/calendar.c (Diff revision 7)
 
 
This is actually a static number of buckets, not a maximum.
/main/calendar.c (Diff revision 7)
 
 
I'm a little bit concerned with this part.  A thread is created and given a pointer to the cal object.  However, the reference count does not reflect that another thread now holds a reference to it, meaning that it is theoretically possible that it could disappear when you don't want it to.

Also, if you change this, be careful not to leak a reference in the case that the thread fails to start.
/main/calendar.c (Diff revision 7)
 
 
I don't think the cast is necessary here.
/main/calendar.c (Diff revision 7)
 
 
 
This is a superfluous return.
/main/calendar.c (Diff revision 7)
 
 
An error will be logged for you if this happens.
/main/calendar.c (Diff revision 7)
 
 
An error will be logged for you.
/main/calendar.c (Diff revision 7)
 
 
An error will be logged for you.

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.