<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Okay, I hate top-posting, but I think
it works best in this context.<br>
<br>
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.<br>
<br>
On 1/22/2013 1:51 PM, Kent James wrote:<br>
</div>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">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.<br>
<br>
There are several general classes of issues that I have faced
many times in working with the folder code. Let me mention them
first.<br>
<br>
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.<br>
</div>
</blockquote>
<br>
I personally want folder objects to be equivalent to folder storage,
as mentioned above.<br>
<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite">
<div class="moz-cite-prefix"> <br>
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").<br>
</div>
</blockquote>
<br>
I really dislike magical special URIs, but I think the cat is out of
the bag here.<br>
<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite">
<div class="moz-cite-prefix"> 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 <a moz-do-not-send="true"
href="https://bugzilla.mozilla.org/show_bug.cgi?id=832034">832034</a>).
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.<br>
<br>
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.<br>
</div>
</blockquote>
<br>
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:<br>
function deleteMessage(msg) {<br>
let folder = getTrashFolder();<br>
folder.addMessage(msg, this);<br>
this.reallyDeleteMessage(msg);<br>
}<br>
<br>
In an async world, you'd need to do:<br>
function deleteMessage(msg) {<br>
getTrashFolder(function (folder) {<br>
folder.addMessage(msg, this, function () {<br>
this.reallyDeleteMessage(msg);<br>
});<br>
});<br>
}<br>
<br>
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
<a class="moz-txt-link-rfc2396E" href="https://wiki.mozilla.org/RFC/TaskDependencies"><https://wiki.mozilla.org/RFC/TaskDependencies></a> for what the
API would look like, although the Add-on SDK more usefully has
nsIPromise::then return an nsIPromise.<br>
<br>
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.<br>
<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite">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.<br>
</blockquote>
<br>
Perhaps, then, this API would be better:<br>
interface nsIFolderLookupService {<br>
nsIMsgFolder getFolderObject(in AUTF8String URI);<br>
void requestFolderCreation(in AUTF8String URI, in nsIUrlListener
callback);<br>
}<br>
<br>
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.<br>
<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite"> My
alternate proposal would be:<br>
<br>
interface nsIMsgIncomingServer {<br>
nsIMsgFolder getFolderObjectForURI(in AUTF8String folderURI,
[optional] in bool forceCreation);<br>
}<br>
<br>
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).<br>
</blockquote>
<br>
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.<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite"> <br>
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.<br>
</blockquote>
<br>
Any reason you'd have the server keep a cache instead of an external
service?<br>
<br>
<blockquote cite="mid:50FEEDB2.4050105@caspia.com" type="cite"> 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.<br>
</blockquote>
<br>
This could be in principle another method on the service.<br>
<pre class="moz-signature" cols="72">--
Joshua Cranmer
News submodule owner
DXR coauthor</pre>
</body>
</html>