Open Bug 1731115 Opened 3 years ago Updated 1 year ago

consider only supporting cached calendars

Categories

(Calendar :: General, task)

Tracking

(Not tracked)

People

(Reporter: mkmelin, Unassigned)

Details

Cached calendars ("Offline support") gives way better perceived performance. We should consider using that for all calendars.

Should this bug be re-titled to something like "Consider removing option to disable offline support"? I saw it and was a little confused because I know the caldav provider does support caching (not sure about the ics one).

That is the suggestion, but I think the current title describes it better. Double negatives are confusing.

I could pick this up at some point depending on the direction intended. It will be quite a bit of work and I will more or less redesign the whole calendar back end. What we have now is too convoluted and fragile. I would store all calendars in sqlite so we could use its API for more advanced queries and have a background sync mechanism for the network calendars.

Has there been any discussion about this bug elsewhere?

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)

Not much discussion yet that I'm aware.

It does sound like your proposal is perhaps mixing in too many pieces. I would suggest first figuring out what/if there is anything blocking us from setting all calendars as having offline support. If not, then we migrate them all to have offline support, and maybe keep "online" ones available behind a pref for some time.

Refactoring could at a later stage do what's needed to address other needs, and not have to think about online-offline while doing so.

Flags: needinfo?(mkmelin+mozilla)

Caching is done by using the calCachedCalendar with an embedded caldav calendar and storage calendar. The calCachedCalendar calls into both of these conditionally to provide the caching support. It currently uses a mix of calIOperationListeners, observers, promises and QueryInterface() calls making debugging issues tricky at best.

I would suggest first figuring out what/if there is anything blocking us from setting all calendars as having offline support.

Well, it's probably only the ics calendar that needs to have offline support and that calendar is complicated in its own way. It's probably not a good idea to cache an ics file on the file system without permission anyway. If the file is edited by an external program then we would overwrite those changes (or prompt the user about the conflict).

Memory calendar needs no caching and we could get probably away with always caching the caldav calendars provided the synchronization bugs are sorted out.

You're coming at this from a different angle. Magnus just wants to remove the option to not cache a calendar and I think that's what this bug should be about.

I would suggest first figuring out what/if there is anything blocking us from setting all calendars as having offline support.

I think all that would be required would be to set the .cache.enabled property on each calendar (or just change the code to ignore it) before starting up the calendar manager (i.e. restarting Thunderbird). That should set each calendar up as cached and fill the cache. FWIW you can make an already initialised uncached calendar cached but in terms of migration it's probably easiest to set the property at start-up.

I could pick this up at some point depending on the direction intended. It will be quite a bit of work and I will more or less redesign the whole calendar back end. What we have now is too convoluted and fragile. I would store all calendars in sqlite so we could use its API for more advanced queries and have a background sync mechanism for the network calendars.

The offline storage is already in SQLite, as each network calendar has its own storage calendar.

Since we're talking about it anyway, here's how it all fits together, as I understand it. You probably already know this but I'm writing it out here for my own benefit. I don't disagree that it's complicated, like most things in the calendar.

  • There's the four real calendar types: caldav, ics, memory, and storage. Each of these can be created with Cc["@mozilla.org/calendar/calendar;1?type=memory"].createInstance(Ci.calICalendar); and like that have no cache.
  • The CalDAV calendar without caching has an internal memory calendar (CalDavCalendar.mOfflineStorage).
  • The ICS calendar always has an internal memory calendar (CalICSCalendar.mMemoryCalendar). It throws this away and creates a new one every time the ICS file is read from the network or disk.
  • With caching, if you get the calendar from the calendar manager, you'll actually get a calCachedCalendar. This holds onto the real calendar (calCachedCalendar.mCachedCalendar) and a storage calendar (calCachedCalendar.mUncachedCalendar). The cached calendar keeps track of what happens when offline and records it in the storage calendar until online, when it handles updating the network calendar.

I think I would rearrange this in an inheritance chain, so that we don't have to keep referring to other objects. Storage would be the base class, caching would inherit from it, and ICS/CalDAV calendars inherit from the caching class. We (kind of) already do this in the address book with CardDAVDirectory/SQLiteDirectory (although CardDAVDirectory handles the caching as well as the network).

Flags: needinfo?(geoff)
You need to log in before you can comment on or make changes to this bug.