Calendar:Architecture

This document describes a proposal for a new architecture of the calendar content model.

Architectural review

Currently the calendar component is built around the calIItemBase interface, events and todos derive from it and add only their specific functionality. Different providers implement the calICalendar interface, which is basically the authoritive instance keeping the storage and the calIItemBase objects in sync. Anyone can register a calIObserver at a calendar (calICalendar) instance to get notified of changes to the model.

Instances of the calIItemBase interface currently have strong value semantics, which means that in order to modify an item one needs first to clone() it, then modify the cloned object, and finally saving the modifcation calling the calendar instance.

The following section will consist of a short review of the current interfaces showing up the relationship among the main interfaces. We won't touch every aspect and it won't go into much detail since this is irrelevant to the proposal of the new architecture of the model.

Let's start with the calICalendar interface (only the interesting methods):

  void addObserver( in calIObserver observer );
  void removeObserver( in calIObserver observer );

  void addItem( in calIItemBase aItem, in calIOperationListener aListener );
  void adoptItem( in calIItemBase aItem, in calIOperationListener aListener );
  void modifyItem( in calIItemBase aNewItem, in calIItemBase aOldItem, 
                   in calIOperationListener aListener );
  void deleteItem( in calIItemBase aItem, in calIOperationListener aListener );

  void getItem( in string aId, in calIOperationListener aListener );
  void getItems( in unsigned long aItemFilter, in unsigned long aCount, 
                 in calIDateTime aRangeStart, in calIDateTime aRangeEndEx,
                 in calIOperationListener aListener );

Clients can add, modify or delete existing items. This is the first group of methods which generally belongs to the item management. Clients can also register as an calIObserver to get notified of changes (addObserver(), removeObserver()). The last important group is getItem() and getItems() which allows clients to query for items, filtering by specific attributes (e.g. only events, no todos) and time range.

The next important interface is calIItemBase, again only the most relevant aspects:

  attribute AUTF8String id;
  attribute AUTF8String title;

  attribute calIRecurrenceInfo recurrenceInfo;
  attribute calIDateTime recurrenceStartDate;

  void getAttendees(out PRUint32 count,
                    [array,size_is(count),retval] out calIAttendee attendees);
  calIAttendee getAttendeeById(in AUTF8String id);
  void removeAttendee(in calIAttendee attendee);
  void addAttendee(in calIAttendee attendee);
  void removeAllAttendees();

 readonly attribute boolean isMutable;
 void makeImmutable();
 calIItemBase clone();

  attribute calICalendar calendar;

  calIItemBase createProxy();
  attribute calIItemBase parentItem;
  attribute calIDateTime recurrenceId;

  void getOccurrencesBetween(in calIDateTime aStartDate, in calIDateTime aEndDate,
                             out PRUint32 aCount,
                             [array,size_is(aCount),retval] out calIItemBase aOccurrences);

The first group of attributes and methods belong to the properties of the item (title, attendees, recurrences, etc.). The second part is made of the isMutable-attribute and the clone()-method. An item is generally readonly, meaning it is immutable. Writing attributes of an immutable item will throw an exception. Clients need first to clone() the item before modifying. The last section is dedicated to recurrence handling. Items may recur, meaning they have a recurrenceInfo set (interace calIRecurrenceInfo, calRecurrenceInfo.js) which means that this event may occur multiple times. One can talk of a folded item, because it inherently represents a set of items. Single Occurrences are internally modelled as so called proxy items, because they belong to a specific master item (their parent item). In contrast to ordinary items, a proxy refers to its parentItem and has a recurrenceId (iCal RECURRENCE-ID) set which uniquely identifies the occurrence, i.e. its specific start date. Proxy items are most commnonly created from scratch upon calling getOccurrencesBetween() on a parent's calIRecurrenceInfo (that's what the views do all the time). Mosten often, because a proxy item can also be a defined exception item. An exception overrides a specific occurrence of a recurring item, being "different from its parent". E.g., you may define a recurring event, say 5 times, which comes down to an (folded)

  { parent event, recurrence-info }
→ { proxy event, RECURRENCE-ID0 }
→ { proxy event, RECURRENCE-ID1 }
...

If you modify the 2nd occurrence, e.g. its title, you are defining an exception item:

  { parent event, recurrence-info }
→ { proxy event, RECURRENCE-ID0 }
→ { exception proxy event of parent's recurrence info, RECURRENCE-ID1 }

Exception proxy items are stored and managed at the calIRecurrenceInfo instance of a parent. Both, parent and proxy items share the same interface, i.e. calIItemBase. In case clients need to know if an object refers to a parent or a proxy, they can check if (item.parentItem == item), or in other words, a parent item refers to itself via its parentItem attribute.

Within this context, cloning an occurrence may become quite complex, because a proxy's parent as well as all of its exceptions need to be cloned, too (because they need to refer to the proper parentItem).

Since parent and proxy items implement the same interface, proxy items can also be passed to methods of calICalendar or any other method that expects a calIItemBase. A method receiving a calIItemBase always needs to be aware of the context, whether a parent or proxy item is passed to it, being robust.

The typical call chain can easily be sketched with a simple example. Let's assume some client (e.g. a view) is registered as an observer at an calendar instance. It creates a new event, fills its attributes and tells the calendar about the new event, calling addItem(). The view gets notified through the appropriate onAddItem() method. It is worth noting that the item passed as argument to onAddItem() is no longer the item which was originally created by the view, since it gets clone()d on its way thru the calendar. Upon unfolding a recurring event, the view will most probably call calIRecurrenceInfo's getOccurrencesBetween() on the (parent) item, receiving a set of proxy items for the specific occurrences of the folded parent item. If the client needs to modify the parent item or any proxy item, it needs to clone() the instance, modify the attributes and pass the new item to the calendar, calling modifyItem(). The calendar will sync the incoming items with the underlying storage and send a notification via onModifyItem(). This mechanism ensures the client receives the modified item. Again please note that the item was clone()d at least twice, once to make the original modification and at least one more time in the calendar during the sync process, e.g. the storage calendar modifies the item's generation. After the client has received its notification, it needs to get rid of its now outdated references and again queries for the now current occurrences to display the modifed state of the item.

Pros/Cons

The advantages of this approach are as follows:

• If you clone an item, you know you have the sole reference to that object, if you don't, someone else might change it without you knowing
• This approach makes it easy to implement a generic undo/redo functionality. An Undo/Redo ring can hold (deep) item copies of all modifications.
• Failures/rollbacks during the sync with the storage are handled easily, because modifications are always performed on temporary copies.

The disadvantages are as follows:

• clone()ing costs an enormous amount of resources, since we constantly create and delete objects. We need to even always copy objects that don't need to be copied in case we just want to modify a single attribute.
• This approach opens the door for all sorts of dangling references issues, since it's easy to keep outdated version of items (→ current view problems).
• It's tedious to compare items, since two instances of an item can refer to the same event/todo ("diff"-ing two items is expensive). This hurts provider implementors.

New proposal

While working on issues on the blocker list for the lightning 0.1 release, we discovered that several of them related to the above mentioned disadvantages. So we thought about a solution to overcome those disadvantages while keeping the advantages, but keeping impact to existing code as low as possible. What follows is the current state of this proposal.

One aspect that we didn't mention so far in the preceeding paragraphs is, that modifyItem() expects two arguments, the new version of an item and the old version of the item. In the current implementation, there is a special situation when modifyItem() is called with (parent, proxy), i.e. when a specific occurrence is deleted (see calendar-management.js, deleteOccurrence()). From the provider implementors point of view, this is kind of confusing, because one really expects the same class of objects being passed in (either (parent, parent) or (proxy, proxy)). A more straightforward approach would probably be to call deleteItem( proxyToBeDeleted ). The calendar implementation can then modify the corresponding parent item, entering an iCal EXDATE for that occurrence. Generally what's needed by providers is the following information:

• Which item is about to be modified?
• What are the actual modifications (the difference)?

This is just an observation of a drawback of the current calICalendar, but it's just a part of the solution but anyway fits nicely into the rest of the proposal.

To address the problem of dangling references to outdated items, the general goal is to just deal with one unique item object all over its lifetime. Moreover avoids any unnecessary cloning. If we don't clone an instance but modify it in place, we don't have invalid references. Thus basically we want to go from value semantics to reference semantics. The trick is to keep the advantages of the original system which is basically a unique interface for both occurrences (proxies) and ordinary events, and makes implementing the views easy.

Assume that items still are immutable, and every modification can only be performed by the calendar instance to which it belongs. The idea is that clients need to get a so called transaction ticket to issue a modification. The transaction ticket shares the same interface with its target item. A common modification would look like this:

var transaction = item.calendar.getTransaction(item);
transaction.title = "new title";
item.calendar.commitTransaction(transaction);

An intelligent transaction ticket will only record changes that are to be done on the target item. The target item won't get modified until commitTransaction() is called on the calendar.

This way we get rid of cloning an item multiple times when modifying, because currently items are cloned (at least) two times:

1. The client code clones to record its modifications to the item.
2. The (storage) calendar clones the item again to modify the item's generation.

But the foremost big advantage of this proposal is the ability to get rid of the possibility that the views hold an outdated reference to an item. Using a transaction system for modifications, there's no way of having an item which doesn't reflect the current state of the provider, each item/proxy exists only once. As soon as a transaction has been committed by the provider, the changes will be reflect by the the one and only instance of an item. Notifications will be sent like before, so views still have to reflect item changes in XUL, of course. The provider will/can ensure that committing a transaction is an atomic operation.

Typically, a transaction tends to be short in time, lowering the risk of a race/conflict with a concurrent transaction being committed. When a conflict/inconsistency is encountered by the committing calendar provider, the transaction needs to be rejected and an error must be reported. Conflicts may come for different reasons:

• The item (proxy, parent) has been deleted.
• A concurrent transaction has written the same item attribute
• A concurrent transaction has written a different item attribute, but the item's constraints, e.g. { DTSTART <= DTEND }, would be violated.

Undo/Redo

The previous approach can handle undo/redo easily because it keeps old items and jump back to a previous generation if requested. Since with the transaction system in place we don't provide copies of the items this approach is no longer possible. But undo/redo is essentially nothing more than a history log of changes, which fits into the transaction mechanism. Upon commit(), the transaction system needs to store the last value of an attribute to be modified, before before committing the new one. That way, we could implement a rollback mechanism for items. Because each commit() transits the item from one consistent state into another, this won't violate data integrity. However, the number possible rollbacks needs to be configurable. xxx todo: where? jminta: probably use nsITransactionManager::maxTransactionCount

Notifications

It becomes obvious that the current notification scheme cannot be supported anymore, i.e. onModifyItem(), because there is no old item anymore. Easing the oberver implementations, it makes sense to provide information about what attributes have changed. Then, the observing instance does not need to update all reflected information. This perfectly embraces the history log described above. The new onModifyItem() would look like this:

// objects of this can be queried for either calIEvent/calITodo:
interface calICommittedTransaction : nsISupports {
    void getModifiedAttributes(
        out PRUint32 count, [array, size_is(count), retval] out string keys );
};
void onModifyItem( in calIItemBase item, in calICommittedTransaction diff );

Implementation Approach

Parent items need to know which proxies are in use, and return the same proxy object everytime when asked for occurrences: constraint is that only one unique item object is published to the views/... at a time. Only when an item has become inaccessable, a new item can be published by the calendar. So the parent item needs to hold weak references on its created proxies. Holding hard references to created proxies does not scale, because proxies would only vanish when the parent and all proxies vanish. E.g. when you are switching your calendar view towards future, looking at a for ever recurring event, you create an open number of proxies.

Transaction objects will implement the calIItemBase interface, they'll appear as regular items to clients, internally they'll just record what attributes have been set, and are to be modified. Upon construction transaction, objects are basically empty, they don't modify the item upon commit(). Clients that are setting attributes will flesh out the transaction object which actual content; which attributes are to be modified, no more, no less. Answere to "What are the actual modifications (the difference)?". Providers need to notify observers in commit order.

Pros/Cons

Since we request transaction objects from the very instance we want to modify, this is the answer to the question "which item is about to be modifed?". Only if we finally commit changes, the provider will be sync'ed and all observers will get notified. This works for parent as well as for proxy items. Clients can transparently create a transaction for a proxy item, change attributes at will, and commit those changes. Even if some other client has a reference to the same proxy, those changes will be correctly reflected and notified.

The necessary interface changes can be kept to a minimum. Basically we need to get rid of modifyItem() and adoptItem() in calICalendar, adding the aboce sketched:

  calIItemBase getTransaction( in calIItemBase item );
  void commit( in calIItemBase item );

Additionally, onModifyItem() would only take one parameter, i.e. the item. Cloning should be removed from the current calIItemBase, because it conflicts with this new mimic. xxx todo, what more?

The whole transaction handling could be implemented for all provider types.

The sketched transaction system will elegantly solve the disadvantages of the original architecture with a minimum of changes. Error handling remains as is, in case of a commit conflict.

Comments are welcome

As this is just a proposal for an architectural shift of the calendar core, it is open for discussion. Feel free to comment on this.

jminta's first-glance comments

I've removed the majority of my old comments, because, for the most part, they have been cleared up by further explanation of the notification system. The big issue I was misunderstanding here was what was being passed to onModifyItem. Since onModifyItem gets a transaction-object that contains all of the old-field values that changed as well as the new object, it has enough information to handle the cases I was concerned about.

New issues

I've had some more time to ruminate on this model, now that I have a better understanding of the notification system. Some new concerns that come to mind.

• Shared calendars - This system works fairly well for changes that a user makes within a system. However, when an outside user changes a shared calendar, coming up with a calICommittedTransaction for that set of changes is highly non-trivial. In fact, it's likely to be even more expensive than the 'deep-copy' that clone() requires now, because what we now need is a 'deep-comparison'. For enterprise calendaring, this may have a performance impact.
• Each calICalendar, under this model, is going to be required to construct one and only one calIItemBase for each item in its store, as requested. Therefore, when subsequent getItems calls are made, it is going to have to consult its list of previous dispatch items, to determine whether it was previously constructed. Not only could this search be a performance hit, but it keeping arrays/hash-tables of objects sent out could impact memory-usage.

Old issues

This concern is still valid, I believe. It concerns issues with the actual editing transaction, not the commitedTransaction. Trying to delete properties and objects from an empty calIItemBase (as calITransaction is defined to be) will be problematic. Some examples, and the previous discussion concerning them follow:

• The variety of transactions that can take place is extremely broad, and it is not clear that all such changes can be easily represented. Some types of changes the may prove difficult to embody in a transaction:
  • deleteProperty("X-PROP1"). Re: The granularity of changes are the attributes of an item. If ical properties have been changed in a transaction, all depending attributes have to be marked as modified. If you reinitiliaze an item using the icalString attribute, the whole item, i.e. all of its attributes are marked as being modified. jminta says: I don't think I was clear here. My issue is that if a calITransaction is empty, you can't simply call deleteProperty("FOO") on it (like you could on a clone), because it doesn't have the property. Furthermore, how would you distinguish between a property that has been deleted and a property whose value is simply blank?
  • changing an event into a task - would the transaction implement calIToDo or calIEvent? This requires that certain code areas that only care about one of these still need to watch other transactions. Re: Why would someone ever want to switch an event into a task or vice versa? jminta says: it's actually a pretty common request, bug 277731.
  • changing the calendar of an item - could be handled with add+delete, but really, only the item's .calendar property is changing, which makes it seem like a commitTransaction() would work.

Keeping this around, just because it's a point I want to emphasize.

• The listener's currently passed in for making modifications, at least in theory, ought to be told when errors occur and what kind of error it was. This is advantageous, in my opinion, and if this new model were to be adopted, I would like to see commitTransaction accept a listener argument as well. Re: Agreed, has been omitted for brevity. jminta says: OK, as long as it's there in the final form.

This may be related to the first new point. In both cases, we're dealing with a set of largely unknown changes to an event, and trying to then diff them out to create a calITransaction.

• This model puts a lot of stress on the event-dialog. It seems to need to figure out exactly what changed, rather than being able to simply rebuild the object from scratch and re-submit it. Re: Where is the difference to the current model? Currently we are creating and writing a new event reading out the whole dialog, and with the new proposal, there would be no code change; just using a transaction object. But if we like, we need only commit changes to UI elements that have actually changed. Being efficient for the first time. jminta says: My point was that I don't see how you can *not* only submit the changed fields. Figuring out which fields changed isn't trivial. Re: The attributes that have changed are derived from the actual transaction that has been committed, which is easy. When committing, the provider has to backup those attributes and construct a change-set for onModifyItem

Keeping this around because I think it's very important. It may also not be as trivial as it sounds, given that enforcing permissions such that the provider may change objects but other code may not is not exactly easy.

• This model needs to be absolutely certain that it has iron-clad enforcement of the rule that it's impossible to change an item other than by a transaction. Otherwise, I'm very scared of how easily a novice extension writer could completely break this model by following the intuitive idea of coding item.title = "new title". Re: Items will be immutable in the new approach. So this is no problem. jminta says: Good. Just be certain.

Still not clear on this one:

• Finally, calITransaction will run into problems in truly implementing calIItemBase. Certain properties of calIItemBase depend on certain other properties being present. In the atomic model, this is not assured. The most critical example of this is icalString. Currently, the ics provider can be certain that it has all the information about an event if it calls icalString. However, calling icalString on a transaction will likely throw an error. Providers will need to be very smart in order to assure that they have all of the changes truly included in a transaction. Re: There seems to be a misunderstanding. A transaction object is not supposed to be passed around, it usually lives only for short time, minimizing the risk to fail upon commitTransaction() (becoming inconsistent). After a transaction object has been committed, it must not be used anymore, leading to an error. jminta says: My point was merely that you can't truly implement an interface if you don't support everything that interface does, ie. icalString for calIItemBase or duration for calIEvent. If you implement an interface, consumers ought to be able to count on being able to use the properties of that interface. Re: Yes, and IMO the notification approach can somehow improved, i.e. onModifyItem( upToDateItem, commitedChangeSet ). The observer needs know what attributes have changed, and the old values. Retrieving old values from the change-set object, the observer can only call for attributes marked as modified (→getModifiedAttributes()), calling on calIEvent/calITodo in a type-safe manner. Calling on any other attribute leads to an exception. An alternative approach would be some kind of nsIProperties, which IMO doesn't do this job any better. Opinions? jminta says: - I'm not sure. This could be related to the first old-issue, in that both deal with how to express these changes in the most coherent manner. My point is that calIItemBase may not be the ideal solution. nsIProperties may be possible, if it can also accept XPCOM objects as arguments, since sometimes the changed property will be an additional attendee, etc. (I don't know enough about nsIProperties to really evaluate this issue.)

I'd like to thank everyone who worked on this model. There are some good ideas here, and regardless of whether this model is adopted or not, I think discussion of the proposals here will signifcantly benefit calendar development.

comments from mvl

There are some good points in the article above, and I agree that changes might be a good thing. Hoiwever, what worries me, is that the changes are deep in the backend. I fear that implementing them in one big (crash) landing mught leave our app(s) in an unusable state for maybe a few weeks. Doesn't sounds like a good idea this soon after the previous rewrites.

So what I'm hoping that we can do, is making changes in small steps. We should try to always have a usable apps. There may be bugs, but they should be fixable in a relativly short time. That way, we can also test things in small parts.

Now, about the contents of the document. My understanding is that there are two basic problems: 1) working with occurences vs parentitems and 2) excessive cloning. I think those two problems are quite independent, and there is not one change that fixes both.

1: ocurrences and parentitems. The basic problem here seems to be that occurences and parentitems both are the same interface (calIItemBase). This leads to confusion. In real live, people have to deal with occurences. When I go to a meeting, it isn't really imporant that it happens every monday, I only really care about the fact that I have to go today, to this particular meeting. Our interfaces should reflect that. calIEvent and calITodo should be occurences of an item. They are what is visible in the views. They might be related to other items, maybe by some recurrence rule. That rule can be expressed in a calIParentItem interface. That interface does not know about item titles, startdates, locations and everything, but only knows about the recurrence rules. The items themself know about the details.

(For ics serialization, each item may have a 'ismodified' flag, meaning that it should be serialized seperatly, with a recurrance-id)

2: excessive cloning. The idea of a commit sounds like a good one. But the need for a new interface and a way to detect the changes feels a bit complicated and fragile to me. We could change the model somewhat by not committing to a calendar, but having a commit method on the item itself. Thiw would work like this: Each item has a number of setters, like for the title. Calling this setter will write the new value in some shadow variable. It won't be stored directly in the item. You need to call commit first. Then the item will tell it's calendar about the change.

Combined with problem 1, we can add a commitToParent oir commitToSet, for when you want all items to be changed.

(This part needs some more thought, ideas are welcome)