350
edits
Calendar:Architecture: Difference between revisions
reply
(→jminta's first-glance comments: - replies to replies) |
(reply) |
||
| Line 148: | Line 148: | ||
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. | 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 <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)?". | 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)?". Providers need to notify observers in commit order. | ||
Providers need to notify observers in commit order | |||
===Pros/Cons=== | ===Pros/Cons=== | ||
| Line 174: | Line 172: | ||
* My biggest concern is that there are certain instances where transaction information is not meaningful without having knowledge of the original object. The current implementation avoids the problem by giving listeners both the original and new objects, which they can compare in any way they see fit. Consider the following (somewhat artificial) scenario: | * My biggest concern is that there are certain instances where transaction information is not meaningful without having knowledge of the original object. The current implementation avoids the problem by giving listeners both the original and new objects, which they can compare in any way they see fit. Consider the following (somewhat artificial) scenario: | ||
** A user is currently viewing today in the Day View. Via the agenda-tree, unifinder, or some other method, the user edits an event currently schedule for 4pm-5pm tomorrow. He modifies the event to begin at 4pm '''yesterday'''. Now we have a transaction that contains only a new startDate. The Day View, upon receiving this transaction, has no way of knowing whether or not such changes affect it, because it has no knowledge of the original event, and hence, no knowledge of the start time. | ** A user is currently viewing today in the Day View. Via the agenda-tree, unifinder, or some other method, the user edits an event currently schedule for 4pm-5pm tomorrow. He modifies the event to begin at 4pm '''yesterday'''. Now we have a transaction that contains only a new startDate. The Day View, upon receiving this transaction, has no way of knowing whether or not such changes affect it, because it has no knowledge of the original event, and hence, no knowledge of the start time. | ||
** A more concrete example might be the alarmService, which only cares about changes to objects with alarm information. If a user changes the start-time, the service will need to retrieve the object to learn if it had an alarm on it, and then determine whether that alarm is affected by the change. '''Re: If we understand your concern correctly, you are talking about how to find out differences between the old and the new item. That's why we have just added a proposal about a different [[#Notifications|Notification]].''' '''jminta says:''' My general concern isn't just notifications (although this section helps a lot), but rather that a set of modified attributes doesn't always tell the whole story. In simple terms: The day-view always needs to know start *and* end in order to make a decision on whether it should update. The alarmService needs to know start *and* alarmOffset. Transactions oftentimes will only provide 1 of these pieces of information, and it isn't clear how to obtain the other. | ** A more concrete example might be the alarmService, which only cares about changes to objects with alarm information. If a user changes the start-time, the service will need to retrieve the object to learn if it had an alarm on it, and then determine whether that alarm is affected by the change. '''Re: If we understand your concern correctly, you are talking about how to find out differences between the old and the new item. That's why we have just added a proposal about a different [[#Notifications|Notification]].''' '''jminta says:''' My general concern isn't just notifications (although this section helps a lot), but rather that a set of modified attributes doesn't always tell the whole story. In simple terms: The day-view always needs to know start *and* end in order to make a decision on whether it should update. The alarmService needs to know start *and* alarmOffset. Transactions oftentimes will only provide 1 of these pieces of information, and it isn't clear how to obtain the other. '''Re: Can't see a problem here. The notification { item, change-set } gives the observer all information it needs. The (already committed, up-to-date) item, and the set of attributes which have changed. If the alarm service needs the alarm-offset, read it out. The item is up-to-date. This also relates to one of your concerns later in this article about the event dialog: You need not use the change-set (i.e. <code>calICommittedTransaction</code>. You can always right away read out all item attributes, even only a single one has changed. But the change-set can be used to optimize this. And, to assure that the change-set is actually correct in some point in time, the providers need to assure that notifications are sent in order they have been committed.''' | ||
* 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: | * 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: | ||
| Line 180: | Line 178: | ||
** 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 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. | ** 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. | ||
** adding an attendee - If a transaction contains a single attendee, how can a listener determine whether this is a modifcation of a previous attendee or an addition of a new one? '''Re: As stated above, we currently think it is sufficient, that the granularity is about attributes of an item. In this case, you would receive two different sets of attendees and have to diff out by yourself; well, as it is now.''' '''jminta says:''' Right now you don't have to diff them out, you can just ignore the old set and use the new one, since it's assured to be complete. Diffing them out is much harder. | ** adding an attendee - If a transaction contains a single attendee, how can a listener determine whether this is a modifcation of a previous attendee or an addition of a new one? '''Re: As stated above, we currently think it is sufficient, that the granularity is about attributes of an item. In this case, you would receive two different sets of attendees and have to diff out by yourself; well, as it is now.''' '''jminta says:''' Right now you don't have to diff them out, you can just ignore the old set and use the new one, since it's assured to be complete. Diffing them out is much harder. '''Re: Since the item is up-to-date, you can do so with the new approach, ignoring the change-set.''' | ||
* I'm not entirely clear how this new model would change (if at all) our current occurrences model. As I understand the proposal, if I want to modify a single occurrence, I would ask for a transaction ticket on that occurrence. Committing that transaction would require the item's calendar to properly update the parentItem so it contains a proper set of occurrences. '''Re: Yes.''' | * I'm not entirely clear how this new model would change (if at all) our current occurrences model. As I understand the proposal, if I want to modify a single occurrence, I would ask for a transaction ticket on that occurrence. Committing that transaction would require the item's calendar to properly update the parentItem so it contains a proper set of occurrences. '''Re: Yes.''' | ||
| Line 189: | Line 187: | ||
* 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. | * 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 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. | * 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 <code>onModifyItem</code>''' | ||
* 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 <code>item.title = "new title"</code>. '''Re: Items will be immutable in the new approach. So this is no problem.''' '''jminta says:''' Good. Just be certain. | * 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 <code>item.title = "new title"</code>. '''Re: Items will be immutable in the new approach. So this is no problem.''' '''jminta says:''' Good. Just be certain. | ||
* Finally, <code>calITransaction</code> will run into problems in truly implementing <code>calIItemBase</code>. Certain properties of <code>calIItemBase</code> depend on certain other properties being present. In the atomic model, this is not assured. The most critical example of this is <code>icalString</code>. 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 <code>commitTransaction()</code> (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. <code>icalString</code> for <code>calIItemBase</code> or <code>duration</code> for <code>calIEvent</code>. If you implement an interface, consumers ought to be able to count on being able to use the properties of that interface. | * Finally, <code>calITransaction</code> will run into problems in truly implementing <code>calIItemBase</code>. Certain properties of <code>calIItemBase</code> depend on certain other properties being present. In the atomic model, this is not assured. The most critical example of this is <code>icalString</code>. 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 <code>commitTransaction()</code> (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. <code>icalString</code> for <code>calIItemBase</code> or <code>duration</code> for <code>calIEvent</code>. 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. <code>onModifyItem( upToDateItem, commitedChangeSet )</code>. 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'' (→<code>getModifiedAttributes()</code>), calling on <code>calIEvent/calITodo</code> in a type-safe manner. Calling on any other attribute leads to an exception. An alternative approach would be some kind of <code>nsIProperties</code>, which IMO does do this job any better. Opinions?''' | ||
==== Big Picture ==== | ==== Big Picture ==== | ||