Re: RFC mailbox interface

[Kenneth Haley <haleykd yahoo com>]

On 12.01.2001 19:47 Jeffrey Stedfast wrote:
> > Hi,
> > Here is my second attempt at the API.  It incorporates some of the
> > suggestions made here as well as a mistake I made since I'm used to
> > writing C++.  So, how's this one.
> 
> I've got a few comments, so bear with me...
> 
> >
> > typedef struct _Flags {
          unsigned READ    :1,
             NEW     :1,
             TRASH   :1,
             DRAFT   :1,
             FLAGGED :1,
             junk :27;
> > } Flags;
> 
> I guess this is ok, might be better to just do guint32 flags; and then
> have macros to set the corresponding bits, this way you could add more
> flags at a later date without breaking compatibility. Not a huge deal
> tho...this'll work.

This just seems like a more natural way to access the flags to me.

> >
> > //all feilds are read-only to clients
> > struct _Message {
> >         Folder *owner;
> >         GList *views;
> >         Flags flags;
> >
> >         //these header fields are unfolded
> >         char to[],from[],subject[],date[];
> >         char msg_id[],references[],in_reply_to[];
> >         char cc[],bcc[],reply_to[];
> >
> >         gpointer data;
> > };
> 
> 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.

> 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.

> 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.

> I kind of feel that a message shouldn't be tied to flags and whatnot,
> but whatever. I later comment on why...
> 
> >
> > typedef struct _Stats {
> >         int New;
> >         int Draft;
> >         int Flagged;
> >         int Read;
> >         int Total;
> > } Stats;
> >
> > typedef struct _Folder_Funcs {
> >         void (*lock)(Folder*);
> 
> this should have error codes, what if you can't lock the mailbox?
> I suggest:  int (*lock) (Folder*);

My mistake .

> returns -1 on error and 0 on success or something. That keeps with 
> most
> of the libc stuff, if ya want to keep with a standard (which is a good
> idea to do, otherwise you have to keep looking up return codes and 
> stuff).
> 
> >         void (*unlock)(Folder*);
> 
> this could also return error codes, but you could probably get by
> without them (for the most part, an unlock shouldn't fail...?)
> 
> >         gboolean (*sync)(Folder*);
> 
> This is fine, but should probably decide on either gboolean or int
> returns for all functions that can fail, rather than having some
> return
> -1 and some return FALSE and stuff.
> 
> I think an int return is better, this way you can have a different
> value
> for why it failed. <0 == failed, -1 = generic error, -2 = folder
> invalid, -3 = server disconnected, etc...
> 
> >         //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.

> >         //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.

> >         //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.

> Normally when you copy a message from one folder to another, you want 
> it
> to keep the flags.
> 
> >         int (*deleteMessage)(Folder*, Message *msg);
> 
> I think a better interface would be: int (*deleteMessage)(Folder*, int
> sequence_id);

Actually having both would be the best course.

> >         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.

> >         int (*setFlags)(Folder*, Message *msg, Flags flags);
> 
> If you go with my suggestion about Message*, then:
> 
> int (*setFlags)(Folder*, int sequence_id, Flags flags);
> 
> If not, then why do you even need it? you can just set it directly on
> the Message object.

Well, maybe both.  The reason for it is to allow any filesystem changes 
to be made imediately such as changing the filename of a maildir 
message.

> >         // return unfolded headers?
> >         GList (*getAllHeaders)(Folder*, Message *msg);
> 
> This method doesn't belong on a folder object, it belongs on the
> message
> object.
> 
> >         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.

> >         //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.

> >
> >         //IMAP format
> >         GList (*findMessages)(Folder*, const char srch[]);
> >         GList (*getHeaders)(Folder*, Message *msg,const char
> hdrs[]);
> >         char* (*getBodyStructure)(Folder*, Message *msg);
> >         char* (*getBodyParts)(Folder*, Message *msg, const char
> parts[]);
> 
> I don't think findMessages should be restructed to just IMAP, if you
> want vfolders this could come in handy for all mailbox types.

None of these are IMAP specific, they merely use IMAP format arguments.

> 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)])

> 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.

> > } Folder_Funcs;
> >
> > struct _Folder {
> >         Server *server;
> >         Folder_Funcs *funcs;
> >         gpointer data;
> > };
> >
> > struct _Server {
> >         Folder* (*getFolder)(Server*, const char path[]);
> >         //should this return a Folder* for the new folder?
> >         int (*newFolder)(Server*, const char path[]);
> >         //return list of available Folder paths for getFolder
> >         GList (*scanFolders)(Server*, const char path[]);
> >
> >         //remote server functions
> >
> >         gboolean (*logon)(Server*, const char user[], const char
> passwd[]);
> >         gboolean (*logoff)(Server*);
> >         //sets path and size for cache use size=0 for unlimited
> >         gboolean (*setcache)(Server*, const char path[], gsize
> size);
> >
> >         gpointer data;
> > };
> 
> I guess this ok...

-- 
K. Haley <halykd@yahoo.com>