1693873 - Investigate replacing calIOperationListener with the DOM Streams API

Status: RESOLVED FIXED

(Calendar :: Internal Components, enhancement)

Description

[Lasana Murray]

The calIOperationListener interface is an important interface in the calendar component.

It is used to both get results from reads, and be notified off writes completion to a calendar. It is however, a bit complicated to grasp at first, not to mention verbose.
Example:
https://searchfox.org/comm-central/source/calendar/providers/ics/CalICSCalendar.jsm#317

Use of this interface for reading seems to be based around streaming results rather than providing them all at once. Now that there is a standard streaming api coming to browsers, it's probably worth consideration.

https://developer.mozilla.org/en-US/docs/Web/API/Streams_API
https://streams.spec.whatwg.org/

I have not looked into this in much detail yet but sharing my thoughts.

Comment 1

[Lasana Murray]

WritableStream isn't ready yet but ReadableStream exists. I think we can benefit from some cleaner code if we return this instead of using callbacks. I'm going to add a method to calICalendar for querying that will return a stream.

Comment 2

[Lasana Murray]

I'm going to do some additional work here to make some of the calICalendar return promises.

Comment 3

[Lasana Murray]

Attachment: WIP: Bug 1693873 - Have calICalendar.getItem return a promise instead. r=darktrojan

Comment 4

[Lasana Murray]

Attachment: Bug 1693873 - Part 1: Have calICalendar.getItem return a Promise. r=darktrojan

Comment 5

[Lasana Murray]

Attachment: Bug 1693873 - Part 2: Convert deleteItem(), getItemOfflineFlag() and deleteOfflineItem() to Promises. r=darktrojan

Comment 6

[Lasana Murray]

Attachment: Bug 1693873 - Part 3: Convert addItem() adoptItem() to Promises. r=darktrojan

Comment 7

[Lasana Murray]

Let's get part 1 in, this bug is likely to become unwieldy the longer it takes to land.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c610b77b74e9
Part 1: Have calICalendar.getItem return a Promise. r=darktrojan

Comment 9

[Lasana Murray]

Ok this is proving exponentially more difficult than it started. The relationship between the storage calendar, cached calendar and the caldav calendar is way too complex and roundabout. I'm scaling doing my next patches.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/245a295b4e97
Part 2: Convert deleteItem(), getItemOfflineFlag() and deleteOfflineItem() to Promises. r=darktrojan

Comment 11

[Lasana Murray]

Attachment: Bug 1693873 - Part 4: Convert modifyItem() to Promises. r=darktrojan

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b3db46c32bc3
Part 3: Convert addItem() adoptItem() to Promises. r=darktrojan
https://hg.mozilla.org/comm-central/rev/cba80602d505
Part 4: Convert modifyItem() to Promises. r=darktrojan

Comment 13

[Lasana Murray]

Attachment: Bug 1693873 - Part 5: Convert getItems() to return a ReadableStream. r=darktrojan

Comment 14

[Lasana Murray]

Attachment: Bug 1693873 - Part 7: Remove calAsyncUtils.jsm. r=darktrojan

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8d5f9a37e98e
Part 5: Convert getItems() to return a ReadableStream. r=darktrojan

Comment 16

[Lasana Murray]

Attachment: Bug 1693873 - Part 6 - Make all calIOfflineStorage methods use promises. r=darktrojan.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2ad2d6a1dbc4
Part 6 - Make all calIOfflineStorage methods use promises. r=darktrojan.
https://hg.mozilla.org/comm-central/rev/15f448d138dc
Part 7: Remove calAsyncUtils.jsm. r=darktrojan

Comment 18

[[e-mail]]

Part 3: Convert addItem() adoptItem() to Promises. r=darktrojan

 // This is a needed hack to keep the cached calendar's "onAddItem" event
 // firing before the endBatch() call of the uncached calendar. It is called
 // in mUncachedCalendar's adoptItem() method.

I don't suppose you can still remember why this was necessary? I couldn't see what it was doing from the code, since all of the adoptItem() methods invoke it as the very last thing, in which case surely this caller could just perform the operation itself? I notice one comment (which seems to have become detached from the comment that it's answering) in the review which mentions a test that might be relevant; do you remember which test it was?

Comment 19

[Lasana Murray]

It's browser_caldavCalendar_cached.js the order of the onAddItem event in a batch needs to be kept. Changes to this part of the code base just landed. You might also want to check out the add-on updating guide if you have not already.

https://github.com/thundernest/developer-docs/blob/master/add-ons/updating/tb102/adapt-to-changes-in-thunderbird-92-102.md#calicalendar