Re: RFC mailbox interface

[Jeffrey Stedfast <fejj stampede org>]

On Fri, 2001-12-07 at 02:49, Kenneth Haley wrote:
> > 
> > I don't like this structure at all really. First, it should probably 
> > be
> > ref-counted, this'd save you a few interfaces that you designed below.
> > Secondly, if you are going to have a Message object, it should really 
> > do
> > it the Right Way (tm) and not stop at just the headers you tossed in
> > there along with a gpointer data. A better approach would be to 
> > design a
> 
> The gpointer data is for message/mailbox specific implemetation data.  
> Things like the file name for a maildir msg, file position in an mbox 
> file, etc.  The data stored in there will be defined by the 
> implemetation.  As for the header feilds, those are in there for 
> balsa's convienience when threading and generating the list view.

You'd be better off making this struct generic and then sub-classing it
then.

Also, if the headers defined are to be used for message-list generation,
you'd probably be better off having a _MessageInfo linked list
containing that data instead. Otherwise you have to parse the entire
mailbox before you can display anything. This is a Bad Thing (tm).

What you actually want is 2 (or more?) files - a mailbox and a summary
file which has all your pre-parsed message headers in it (and probably
message offsets too). This will allow MUCH faster results.

> 
> > Message object that would represent the mime hierarchy. Also, you 
> > could
> > make it so that you could request *any* message header. This is 
> > probably
> 
> You can already request any header.

how? I didn't see any way to do that...

> 
> > most easily done using a hash table or something. A better way to
> > represent the message body than "gpointer data" would be to have a 
> > tree
> > of mime parts, each mime part would then have it's own header hash 
> > table
> > as well (mime parts are kinda like mini-messages).
> > To ease appending a message to a folder, you should probably also make
> > it so that you can do Message::to_string() or something on it.
> 
> That's what getAll is for, besides that should be done by copyMessage 
> since their could be a faster way.

Okay...

[snip]
> > 
> > >         //deletes all messages in folder
> > >         gboolean (*empty)(Folder*);
> > 
> > maybe call this expunge for consistency's sake (IMAP calls it
> > EXPUNGE).
> 
> I'll have to check but isn't that just for messages marked for 
> deletion?  The above would be used for Empty Trash, etc.

Yes, but there is no reason to have a function specifically to empty a
folder when it could just as easily be done by marking every message for
deletion and then expunging deleted messages. You could always have an
optimization for this if you wanted to, but I doubt it will make much
difference really.

> 
> > >         //only updates list if folder is open?
> > >         Stats (*check)(Folder*);
> > >
> > >         //possibly creates Message list
> > >         //required for most functions
> > >         Stats (*open)(Folder*);
> > 
> > this won't work (you can't return a struct), I guess you could return 
> > a
> > struct pointer tho...
> 
> Yes you can pass and return structs in ANSI C.  I just checked.

ah, cool.

> 
> > >         //possibly free Message list
> > >         void (*close)(Folder*);
> > >
> > >         Message* (*getMessage)(Folder*, int msg);
> > >         // -1 fail, 0 success w/ folder closed, >0 msg index w/
> > folder
> > open
> > >         int (*addMessage)(Folder*, char data[], Flags flags);
> > 
> > I propose: int (*addMessage)(Folder*, Message*, Flags flags);
> > 
> > if you go with my recommendation that a Message not contain flags, but
> > if you choose to have the Message struct contains flags, then I
> > suggest:
> > 
> > int (*addMessage)(Folder*, Message*);
> 
> Wrong usage of addMessage.  It is used for messages that are created by 
> the user or are retreived through POP3.  In either case there is only 
> text data, the Message struct is created by the various mailbox 
> implemetations.

Think again. What if you wanted to take a message from one mailbox and
deliver it to another folder on a different system (local -> imap or
vise versa)? You'd have to use the "Add" interface if the folders were
not on the same system. You don't want the mbox implementation know how
to access imap for example, so you'd *have* to use the "Add" interface.

[snip]
> 
> > >         int (*alterMessage)(Folder*, Message *msg, const char
> > data[]);
> > 
> > why would you ever want to alter a message?
> 
> Think draft messages.  There are also people that do edit received 
> messages, ussually cutting out useless bit from newsletters and digests.
> 

I guess...

[snip]
> > 
> > >         char* (*getBody)(Folder*, Message *msg);
> > 
> > same as above.
> > 
> > >         char* (*getAll)(Folder*, Message *msg);
> > 
> > and again...
> 
> Off hand I agree that those and perhaps others should be attached to 
> the message.  Their was some question about this earlier.  I intend to 
> look at this again.

getAll() and getBody() are really pretty awful interfaces... I guess
getAll() could pass based on your comments earlier, but getBody is
pretty bad.

These interfaces really don't allow for the flexibility needed in the UI
to display the message to the user. How can I display a message
attachment with these interfaces? What if I want to display the
text/html subpart of the multipart/alternative rather than the
text/plain part?

What exactly does getBody() return anyway? does it contain a MIME parser
and try to guess which is the message body part? If so, what if it's
wrong? If it instead returns the raw data immediately following the
header, what use is that to me? I can't parse a MIME message with just
that bit, I also need the message headers - which means that I should be
using getAll()??

While I'm at it, let me stress the importance of having the MIME parser
at the mailbox level. First off, as I stated earlier for performance
reasons you really need to pre-parse all the headers so you can save
things like references, recipients, subject, from, etc in a
summary/index file. Secondly, it makes for wonderfully useful interfaces
on the Folder. When I ask a folder for a message, it can give me the
pre-parsed message object tree and I can then use that much more
efficiently in the UI to display the message to the user.

> 
> > >         //using copy-in allows for vfolders (ref copy)
> > >         int (*copyMessage)(Folder*, Message *src);
> > 
> > if Message was ref-counted, you wouldn't need this. Again, I don't 
> > feel
> > this belongs on a folder object - it's an operation on a message.
> 
> Yes you would need this, it (ussually) physicaly copies the message.

eek! why!? What if, say, I have a 10 meg message...now I'm gonna have 20
megs in ram!? This is why refcounting exists...please use it!

[snip]
> 
> > what is getHeaders() supposed to do? I'm confused...
> 
> It allows you to get specific headers.  Think of it as the following 
> IMAP command:
> FETCH msguid (BODY[HEADER.FEILDS (hdrs)])

Oh, so this is how you are supposed to fetch message headers. I still
think it should be an interface on the message object.

the above command would be useful in generating a folder summary/index
though - but it should be an internal thing - it's not really all that
useful for the front-end.

> 
> > getBodyStructure() should probably be an internal method for IMAP only
> > folder types and not a method on a generic folder object. I don't see
> > any real reason to have this anyway...
> > 
> > getBodyParts() should be a Message method and not a method on a folder
> 
> These are for retreiveing only specific parts of messages from IMAP 
> servers.  This can't be handled by the library since it is user 
> dependant.

How is it user-dependent? Still, I think it should be a method on the
message.

Jeff

-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj@ximian.com  - www.ximian.com