Revisiting the War on RDF, especially folder lookup

Joshua Cranmer pidgeot18 at gmail.com
Wed Jan 23 01:50:45 UTC 2013


Okay, I hate top-posting, but I think it works best in this context.

I'm not sure if you were proposing this or not, but there is one API 
aspect that I want to avoid if at all possible. I do not want there to 
exist an interface where the methods are only usable at certain times. 
By this, I mean that if I were to get an nsIMsgFolder object somehow, 
then I should be able to call any method (say, addMessageToFolder) 
without having to first call a method (say, createStorageIfMissing). 
This is not true of our API at present: nsIMsgDBHdr definitely violates 
this when applying filters, and I think "proto-folder objects" can exist 
(particularly for IMAP). In particular, this implies that if I have a 
folder object, then it appears to have the necessary backing storage.

On 1/22/2013 1:51 PM, Kent James wrote:
> As it turns out, just last week I wrote a bit of code surrounding the 
> Archive functionality for ExQuilla, which faces many of the issues 
> that you describe here. So this topic is very familiar to me at the 
> moment (and just another example of the eerie history that rkent and 
> jcranmer are almost always working on the same issues at the same 
> time). I'll make some comments up front, and some comments inline.
>
> There are several general classes of issues that I have faced many 
> times in working with the folder code. Let me mention them first.
>
> 1) Confusion between the creation of the folder object, and the folder 
> storage - and the related question of who does initializations. Yo 
> already mentioned the initial startup as an issue which is related to 
> this.

I personally want folder objects to be equivalent to folder storage, as 
mentioned above.

>
> 2) Handling the URIs of special folders. We have a number of folders 
> that have special URIs, like "trash" or "junk". Keeping those straight 
> is tricky. It would be good if the handling of those was centralized. 
> (This is your "normalized to specific case-sensitive variants").

I really dislike magical special URIs, but I think the cat is out of the 
bag here.

> 3) Mixed sync/async models. Since many folder operations are sync for 
> local folders and async for IMAP, much of the code has parallel paths 
> depending on whether a folder is async or not (see for example 
> processNextBatch in mailWIndowOverlay.js, also see bug 832034 
> <https://bugzilla.mozilla.org/show_bug.cgi?id=832034>). A related 
> issue is the type of async handling that this implies. The most common 
> model in mailnews code is to listen for a folder event, like 
> nsIMsgFolderListener.folderAdded.  But this has no error handling 
> builtin, and there is no reliable method that always gets called 
> regardless of the result. In my own code, I usually end up creating a 
> class like CreateSubfolderWithListener that has a reliable listener 
> that is guaranteed to be called, and serves as a reliable termination 
> point for async processing.
>
> I would propose that calls that may be async be replaced by calls that 
> are always async, or that return whether async processing is needed - 
> and the async termination  (such as nsIURLListener.OnStopRunningURL) 
> is guaranteed to be called after the return from the main routine.

Normally, I would recommend making everything async and being done with 
it (even local hard-drive operations aren't necessarily synchronous, 
especially under very heavy disk load). The problem I have with that is 
so much of our codebase is in C++, which, combined with how XPCOM API 
ends up working, makes doing asynchronous operations painful. Suppose we 
want to delete a message under the move-to-trash model. In synchronous 
flow, this looks like so:
function deleteMessage(msg) {
   let folder = getTrashFolder();
   folder.addMessage(msg, this);
   this.reallyDeleteMessage(msg);
}

In an async world, you'd need to do:
function deleteMessage(msg) {
   getTrashFolder(function (folder) {
     folder.addMessage(msg, this, function () {
       this.reallyDeleteMessage(msg);
     });
   });
}

That API assumes simple functional callbacks; in practice, we have 
things like nsIIURLListener which means we need to either construct 
different objects with specific method names or we build one object 
which effectively switches in its callback based on its current state. 
I'm purposefully showing this in a JS syntax, since the XPCOM C++ model 
for this is extremely painful. A promise-based API has been proposed 
(see <https://wiki.mozilla.org/RFC/TaskDependencies> for what the API 
would look like, although the Add-on SDK more usefully has 
nsIPromise::then return an nsIPromise.

It is possible to have a mixed sync/async world by making the sync 
operations be internally async (prematurely presuming success) but 
making all operations accumulate in a pending buffer queue. This API is 
easier on consumers, but it is definitely a world of pain for implementers.

> I think that you are already introducing confusion here between the 
> "folderObject" and the "folderStorage". I really don't think you 
> should mix those in the way that you are proposing here. 
> "forceCreation" should refer to the folderObject only, so that 
> getFolderObjectForURI can safely be sync. Follow that call with 
> nsIMsgFolder::CreateStorageIfMissing (an async operation) if you need 
> the storage.

Perhaps, then, this API would be better:
interface nsIFolderLookupService {
   nsIMsgFolder getFolderObject(in AUTF8String URI);
   void requestFolderCreation(in AUTF8String URI, in nsIUrlListener 
callback);
}

The first method would only return if the folder's storage presently 
exists and is usable. The second method would check if the folder object 
exists; if it does not, it (recursively) tries to create it and make the 
storage. When it is finally done, it calls the callback.

> My alternate proposal would be:
>
> interface nsIMsgIncomingServer {
>   nsIMsgFolder getFolderObjectForURI(in AUTF8String folderURI, 
> [optional] in bool forceCreation);
> }
>
> and use the existing nsIMsgFolder.createStorageIfMissing(in 
> nsIUrlListener) method (though possibly with a bool return to indicate 
> that an async process was started to create the folder storage). But 
> note that forceCreation simply means create the folderObject, not the 
> folderStorage or its reflection in a nsIMsgDatabase file. That way 
> getFolderObjectForURI is always sync. (I'm not sure that the existing 
> imap createStorageIfMissing will recursively create missing parents).

One stand I do want to take is that there should be a global-ish service 
for going from a URI string to a folder, instead of asking users to try 
to find a server first and get the folder from that server. I'm not sure 
if you're dropping that part or not.
>
> I guess I am proposing that the folder lookup service is instead a 
> server lookup service which would be an nsIMsgMessageService method. 
> Servers are hard references owned by the message service. Then the 
> server would lookup the folder, caching them as soft links. There 
> would also be utility methods that already exist, such as the js 
> getFolderForURI and the C++ GetExistingFolder.

Any reason you'd have the server keep a cache instead of an external 
service?

> I am punting the issue of initialization which is a whole nuther 
> topic. We also need to deal with the questions of opening the message 
> database. It would be fairly rare to get the folder without also 
> wanting to get the database. Perhaps we need an async method to open a 
> folder from the URI, which will also ensure that the database is open 
> and up to date. We are moving toward having opening the database be 
> async, so there should be a single async method to get and open the 
> folder and its database.

This could be in principle another method on the service.

-- 
Joshua Cranmer
News submodule owner
DXR coauthor

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/tb-planning/attachments/20130122/6f7a965b/attachment.html>


More information about the tb-planning mailing list