Calendar:Architecture: Difference between revisions

major rework, part2
No edit summary
(major rework, part2)
Line 70: Line 70:
Both, parent and proxy items share the same interface, i.e. <code>calIItemBase</code>. In case clients need to know if an object refers to a parent or a proxy, they can check if <code>(item.parentItem == item)</code>, or in other words, a parent item refers to itself via its <code>parentItem</code> attribute.
Both, parent and proxy items share the same interface, i.e. <code>calIItemBase</code>. In case clients need to know if an object refers to a parent or a proxy, they can check if <code>(item.parentItem == item)</code>, or in other words, a parent item refers to itself via its <code>parentItem</code> 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 a new <code>parentItem</code>).
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 <code>parentItem</code>).


Since parent and proxy items implement the same interface, proxy items can also be passed to methods of <code>calICalendar</code> or any other method that expects a <code>calIItemBase</code>. A method receiving a <code>calIItemBase</code> always needs to be aware of the context, whether a parent or proxy item is passed to it, being robust.
Since parent and proxy items implement the same interface, proxy items can also be passed to methods of <code>calICalendar</code> or any other method that expects a <code>calIItemBase</code>. A method receiving a <code>calIItemBase</code> always needs to be aware of the context, whether a parent or proxy item is passed to it, being robust.
Line 77: Line 77:
Upon unfolding a recurring event, the view will most probably call <code>calIRecurrenceInfo</code>'s <code>getOccurrencesBetween()</code> 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 <code>clone()</code> the instance, modify the attributes and pass the new item to the calendar, calling <code>modifyItem()</code>. The calendar will sync the incoming items with the underlying storage and send a notification via <code>onModifyItem()</code>. This mechanism ensures the client receives the modified item. Again please note that the item was <code>clone()</code>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 <code>generation</code>. 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.
Upon unfolding a recurring event, the view will most probably call <code>calIRecurrenceInfo</code>'s <code>getOccurrencesBetween()</code> 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 <code>clone()</code> the instance, modify the attributes and pass the new item to the calendar, calling <code>modifyItem()</code>. The calendar will sync the incoming items with the underlying storage and send a notification via <code>onModifyItem()</code>. This mechanism ensures the client receives the modified item. Again please note that the item was <code>clone()</code>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 <code>generation</code>. 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:
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
* 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.
* This approach makes it easy to implement a generic undo/redo functionality. An Undo/Redo ring can hold (deep) item copies of all modifications.
Line 89: Line 89:


== New proposal ==
== New proposal ==
While working on issues on the blocker list for the lightning 0.1 release we discovered that there were several of them related to the above mentioned disadvantages of the current architecture. Thus we thought about some solution to overcome those disadvantages while keeping the advantages while keeping the changes to the system to a minimum. What follows is the current state of this 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.
One aspect that we didn't mention in the preceeding paragraphs was that modifyItem() expects two arguments, the new version of an item and the old version of the item. But since parentItems and proxyItems have the same interface, clients are free to pass either of the to modifyItem(). But some providers expect only a certain kind (either parent or proxy), and what's even worse is that clients are stressed more or less, depending whether you pass a parentItem or a proxyItem. Generally what's needed by providers are the following informations:
What follows is the current state of this proposal.


One aspect that we didn't mention so far in the preceeding paragraphs is, that <code>modifyItem()</code> 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 <code>modifyItem()</code> is called with <code>(parent, proxy)</code>, i.e. when a specific occurrence is deleted (see <code>calendar-management.js, deleteOccurrence()</code>).  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 <code>(parent, parent)</code> or <code>(proxy, proxy)</code>).  A more straightforward approach would probably be to call <code>deleteItem( proxyToBeDeleted )</code>.
The calendar implementation can then modify the corresponding parent item, entering an iCal <code>EXDATE</code> for that occurrence.
Generally what's needed by providers is the following information:
* Which item is about to be modified?
* Which item is about to be modified?
* What are the actual modifications (the difference)?
* What are the actual modifications (the difference)?


This is just an observation of a drawback of the current interface of calICalendar, but it's just a part of the solution but anyway fits nicely into the rest of the proposal.
This is just an observation of a drawback of the current <code>calICalendar</code>, but it's just a part of the solution but anyway fits nicely into the rest of the proposal.
 
To address the problem of the 'dangling references' we need to get rid of the unnecessary cloning, which obviously is what makes the references dangling. If we don't clone an instance but modifiy 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.
 
Assume that items will still be readonly, it should not be possible to directly modify the attributes of an item. The idea is that clients need to get a socalled transaction ticket to allow such an operation. This transaction has the following general properties:


* transactions are supposed to be extremely short operations
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.
* transaction allow to only record the specific changes
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.
* transactions ensure that the sync with the storage is a truly atomic operation.


So in a nutshell we still don't allow clients to directly modify items, but instead forcing a deep copy of the item we provide an intermediate layer which is able to record the modifications that can be applied in a transparent and safe way.
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:


This approach has several advantages which we're now going to highlight step by step. At the same time we provide the necessary interface changes and some implementation details.
var transaction = item.calendar.getTransaction(item);
transaction.title = "new title";
item.calendar.commitTransaction(transaction);


The first and foremost big advantage of this proposal is the ability to get rid of the possibility that clients being able to hold outdated version of item instances. With the transaction system in place, there's no way of having an item which doesn't reflect the current state of the storage, each item/proxy exists only once. As soon as a transaction has been performed by the storage, the changes will be transfered to the one and only instance of an item.
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 <code>commitTransaction()</code> is called on the calendar.


One implementation detail seems worth noting here: parentItems need to know which proxies are alive and proxies need to refer back to the parentItem. It's necessary to keep the references this way, since changes to items as well as proxies need to be reflected to all known instances. The only way to create proxies is to call getOccurrencesBetween() or some similar method provided by the calIItemBase interface. In order to hand out the appropriate proxy objects, the item needs to either hand out references to already existing proxies (which some previous call created) or create a new ones.
This way we get rid of cloning an item multiple times when modifying, because currently items are cloned (at least) two times:
#The client code clones to record its modifications to the item.
#The (storage) calendar clones the item again to modify the item's <code>generation</code>.


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. Upon construction transaction objects are basically 'empty', the don't contain anything. Clients setting new attributes will flesh out the transaction object which actual content, and more specifically only with the difference, with what is necessary to modify. So transaction objects are the answer to 'what needs to be modified?', not more, not less.
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.


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()' those changes, the storage will be synched and all intereseted clients will get notified. It it worth noting here, that this system will work for parentItems as well as proxyItems. Clients can transparently create a transaction for a proxyItem, change attributes at will, and commit() those changes. Even if some other client has a reference to the same proxyItem, those changes will be correctly reflected and notified.
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. <code>{ DTSTART <= DTEND }</code>, would be violated.


The necessary interface changes can be kept to a minimum. Basically we need to get rid of 'modifyItem()' in calICalendar. 'adoptItem' would also be needless in this scenario and could be send to the void. The following new methods would provide the entrypoint for the transaction system:
===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.


  calITransaction getTransaction( calIItemBase );
Transaction objects will implement the <code>calIItemBase</code> 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 <code>commit()</code>. 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)?".
  commit( calITransaction );


The modification of an item would look like this:
===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.


  var transaction = calendar.getTransaction(item);
The necessary interface changes can be kept to a minimum. Basically we need to get rid of <code>modifyItem()</code> and <code>adoptItem()</code> in <code>calICalendar</code>, adding the aboce sketched:
  transaction.title = "new title";
  calendar.commit(transaction);


The whole system could be implemented as some additions to the items and a single class which could handle the transaction system for all providers. Specific providers would aggregate the transaction system, thus we need to write this once and use it several times, once for each provider.
  calIItemBase getTransaction( [in] calIItemBase item );
  commit( [in] calIItemBase item );


The above sketched transaction system will elegantly solve the disadvantages of the original architecture with a minimum of changes. But what's about the failure of changes to the storage and to the generic undo/redo functionality? Of course we won't loose what were the advantages of the original approach, but the transaction system provides all of that, too.
Additionally, <code>onModifyItem()</code> would only take one parameter, i.e. the item.  Cloning should be removed from the current <code>calIItemBase</code>, because it conflicts with this new mimic. xxx todo, what more?


Let's first discuss what happens in case of a failure to perform the requested transaction. The original approach passes an optional listener object as argument to modifyItem(). The listener never gets called if the operation couldn't be performed and calendar observers play the same role, they actually never get called. Clients still have a valid view of the model, since they still have there unmodifed events. Exactly the same happens with the transaction system, either a transaction can be performed or not, changes to the items won't be reflected unless the storage operation was successful. And the commit() method could also acceppt the same listener than modifyItem() did.
The whole transaction handling could be implemented for all provider types.


Now for the tricky part called undo/redo. The previous approach can handle undo/redo because it's easy to just keep 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 exactly is what the transaction objects provide. So a naiive approach would be to keep all transaction objects along the original item and upon request for some specific attribute travel along the changes. Of course this would be not optimal, and the solution is simple. The item reflects the most up-to-date version at all times, but we keep a negative version of the transaction along with the item. This allows for a generic and elegant undo/redo functionality.
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.


While thinking for possible drawbacks of the transaction system as a whole we came up with the question 'what happens if several clients want to get a new transaction object while there are maybe others not yet committed?'. After some thinking we decided that it wouldn't be a problem to hand out several transaction objects, even if previously created ones were still not committed. But generally we would need to add a generation-stamp to each transaction object. Afterwards, during commit() the transaction handler could merge non-conflicting transactions and therefore even allow for parallel transactions but still ensure that transactions itself are truly atomic operations.
Now for 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 <code>commit()</code>, 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 <code>commit()</code> 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?


== Comments are welcome ==
== 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.
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.