[Home]

Summary:ASTERISK-03412: [design discussion] store/retrieve messages on/from IMAP server
Reporter:rtctel (rtctel)Labels:
Date Opened:2005-02-01 11:06:01.000-0600Date Closed:2011-06-07 14:05:20
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) vm_and_storage_apis.png
Description:Our company is going to add IMAP storage support to app_voicemail

****** ADDITIONAL INFORMATION ******

We would like to get some comments on how Digium would like to see this done (so that you would accept the patch into CVS).

I heard a rumor (from bkw?) that Digium was looking at adding this in the near future and would like to coordinate our developers efforts with theirs if that is the case.

Comments:By: rtctel (rtctel) 2005-02-01 11:18:48.000-0600

To fill in a few more details...

We plan on using Cyrus IMAP, which will let us proxy auth as one user (ie. asteriskvm) and drop mail into any users mailbox without storing the list of users passwords on the asterisk server.

Some sort of storage abstraction layer might be appropriate, so that other vm storage methods could be added more easily.

The developer should be adding his initial comments (with more app_voicemail specifics) to this bug shortly.
By: Jeffrey C. Ollie (jcollie) 2005-02-01 14:33:01.000-0600

I was looking at separating the voicemail system from it's storage a few months ago, but never got very far.  I think that this is really a logical next step in the voicemail system development.
By: Mark Spencer (markster) 2005-02-01 21:18:04.000-0600

If you look at CVS head, you'll see the ODBC_STORAGE stuff...  That's certainly *far* from perfect but might give you some ideas.  The general concept is to try to simply create retreive(), store() and a few other macros that can save and retrieve messages from a given backend (in the case of ODBC_STORAGE, the actual database).
By: Brian West (bkw918) 2005-02-02 09:49:16.000-0600

Maybe that whole ODBC_STORAGE needs to be brought into the res_config framework.  That way you can register differnt storage methods in a similar way we register diffrent config handlers?  Just an idea.  Then you can write a storage interface with ease!

bkw
By: Brian West (bkw918) 2005-02-02 10:20:30.000-0600

maybe res_storage?

bkw
By: Anthony Minessale (anthm) 2005-02-02 10:29:34.000-0600

kinda like ?

static int mystore_put(char *file, unsigned int flags) {
  ...
}
static int mystore_get(char *file, unsigned int flags) {
  ...
}

struct storage_obj {
 char *name;
 void *get_func;
 void *put_func;
};

static struct storage_obj mystore = {
    name: "mystore",
    get_func: mystore_get,
    put_func: mystore_put,
};

ast_register_storage(&mystore);

I composed this in a little text box in 5 min so please no critisim on the code =)
By: Kevin P. Fleming (kpfleming) 2005-02-02 12:50:13.000-0600

Yes, anthm's idea is on the right track. However, we need to dissociate the actual filesystem path for a file from its "logical" path. As it stands right now with ODBC storage in app_voicemail, if you recompile (or reconfigure) Asterisk with a different AST_SPOOL_DIR than you previously used, you will not be able to access any of your voicemail in ODBC storage, because it was stored using the previous path as the key.

I would suggest something like what ASTDB uses: a "family" name, a "key" name, and then an (optional) path within the key. That way the res_storage_spool driver that stores stuff in AST_SPOOL_DIR can construct its paths as it sees fit, but the res_storage_odbc driver could actually have separate tables for each "family"/"key" combo, with rows identified by the path within the key. Really, though, this needs some sort of "mapping" config like res_config uses, so that any given family/key combo can be mapped to a res_storage driver at run time.
By: Kevin P. Fleming (kpfleming) 2005-02-02 13:26:10.000-0600

Here's a more fleshed-out example of what's in my head:

-- storage.h --

struct storage_path {
 char *family;
 char *key;
 char *item;
};

struct storage_obj;
struct storage_obj_list;
struct storage_engine;

struct storage_obj *storage_open(struct storage_path *path);
int storage_close(struct storage_obj *obj);
struct storage_obj_list *storage_find(struct storage_path *path);
int storage_delete(struct storage_path *path);
int storage_copy(struct storage_path *from, struct storage_path *to);

ssize_t storage_read(struct storage_obj *obj, void *buf, size_t count);
ssize_t storage_write(struct storage_obj *obj, const void *buf, size_t count);

-- storage_engine.h --

struct storage_engine {
 char *name;
 struct storage_obj *(*open)(struct storage_path *path);
 int (*close)(struct storage_obj *obj);
 struct storage_obj_list *(*find)(struct storage_path *path);
 int (*delete)(struct storage_path *path);
 int (*copy)(struct storage_path *from, struct storage_path *to);
 struct storage_engine *next;
};

int storage_engine_register(struct storage_engine *eng);
int storage_engine_deregister(struct storage_engine *eng);
void *storage_get_private(struct storage_obj *obj);
void storage_set_private(struct storage_obj *obj, void *private);

-- storage.c --

struct storage_obj {
 struct storage_path *path; (copied from opener)
 struct storage_engine *eng; (direct reference)
 void *pvt; (private data used by the engine)
};

struct storage_obj_list {
 struct storage_obj *item;
 struct storage_obj_list *next;
}

Notes:

- it's important to implement read/write directly, so that res_storage modules that can provide direct access to the data without copying it out of their own storage are able to do so (unlike current ODBC storage in app_voicemail)

- need some method for navigating the storage_obj_list; the linked-list macros may work here

- storage_find() would take a storage_path containing Unix-standard glob names for matching; res_storage modules would have to translate those to whatever wildcard syntax their storage mechanism support (or use glob() to match them directly, if needed)

- we may also want to build in flock()-type functionality as well, which can be mapped to database record/row locks or whatever, as needed.

- we should support file paths like "storage:/family/key/item" so that Playback, Background, Record, app_monitor, etc. can all create/manipulate items in storage

Just think of the possibilities: res_storage_spool, res_storage_odbc, res_storage_imap, res_storage_mirror (stacks on top of other modules), res_storage_shadow (stacks on top of another module and merges them together, putting new entries into the top layer)...
By: Mark Spencer (markster) 2005-02-02 13:33:53.000-0600

And unfortunately none of this takes into account the indexing, callerid and other parameters required for voicemail which should logically reside in the same place as the voicemail data itself :)

The whole reason that ODBC_STORAGE is not REALTIME_STORAGE is because the nature of the files in voicemail, with their indexing, does not lend itself to a general abstraction that is useful outside of voicemail.  In the absense of any further ideas or improvements, I would suggest we keep this abstraction just to voicemail, but if someone else comes up with good, non-hoaky idea for doing it more generally, I will certainly keep an open mind.
By: Jeffrey C. Ollie (jcollie) 2005-02-02 13:46:31.000-0600

I too would like to see the VM storage abstracted out (in fact I had actually started coding something up but got distracted and never got back to it).  However, I believe that the voicemail front end should not have to construct database keys/urls like kpfleming & anthm are proposing.

The voicemail front end should only know about contexts, mailboxes, folders, and message ids.  It's then up to the back end voicemail storage system to mangle the context/mailbox/folder/messageid into whatever it needs to retrieve the information that the front end has requested.

Here's a look at how I see the VM front end interacting with the back end.  Not all of the details are there yet but I hope that everone can see what I'm getting at.

struct ast_vm_storage {
 char *name;
 
 struct ast_vm_mailbox *(find_user)(const char *mailbox,
    const char *context);
};

ast_vm_register_context(const char *context,
struct ast_vm_storage *storage);

struct ast_vm_mailbox {
 struct ast_vm_storage *storage;
 char *mailbox;
 char *context;

 int (*authenticate)(struct ast_vm_mailbox *mailbox,
     const char *password);

 /*
  * Returns count of messages in folder.  Returns -1 if folder
  * does not exist.
  */
 unsigned int (*message_count)(struct ast_vm_mailbox *mailbox,
const char *folder);

 /*
  * Retrieve a message from the folder.  Returns NULL if the specified message
  * cannot be found.
  */

 struct ast_vm_message *(*get_message)(struct ast_vm_mailbox *mailbox,
const char *folder,
unsigned short msgid);
 
 /*
  * Store a message into the mailbox.
  */
 int (*put_message)(struct ast_vm_mailbox *mailbox,
    struct ast_vm_message *message);
 

 /*
  * Permanently delete any messages that have the "deleted" flag set.
  */
 int (*purge_folder)(struct ast_vm_mailbox *mailbox,
     const char *folder);

 /*
  * Functions for dealing with name/announcements.
  *
  * has_X returns FALSE/TRUE if recording exists for X.
  * get_X returns a malloc'd string that has the filename that
  *       contains the recording.
  * put_X saves a recording.
  * delete_X deletes a recording.
  */

 int (*has_name)(void);
 char *(*get_name)(void);
 int (*put_name)(const char *filename);
 int (*delete_name)(void);

 int (*has_busy_announcement)(void)
 char *(get_busy_announcement)(void);
 int (*put_busy_announcement)(const char *filename);
 int (*delete_busy_announcement)(void);

 int (*has_temporary_announcement)(const char *filename);
 char *(*get_unavailable_announcement)(void);
 int (*put_unavailable_announcement)(const char *filename);
 int (*delete_unavailable_announcement)(void);

 int (*has_temporary_announcement)(void);
 char (*get_temporary_announcement)(void);
 int (*put_temporary_announcement)(const char *filename);
 int (*delete_temporary_announcement)(void);

 /* used by various storage modules for private data */
 void private[];
};

struct ast_vm_message {
 struct ast_vm_mailbox *mailbox;
 char *folder;
 unsigned short msgid;

 char *filename; /* Filename that contains audio data.  Initially
    NULL... see load/unload_recording */
 char *calleridnum;
 char *calleridname;
 unsigned long *time_recorded; /* timetamp of when message was recorded */
 unsigned long *time_saved; /* timestamp of when message was put into folder */
 int read;  /* boolean indicator */
 int deleted; /* boolean indicator */

 int (*mark_read)(struct ast_vm_message *message);
 int (*mark_unread)(struct ast_vm_message *message);
 int (*mark_deleted)(struct ast_vm_message *message);
 int (*mark_undeleted)(struct ast_vm_message *message);

 /*
  * When you first retrieve a message from a mailbox, the filename is
  * set to NULL.  All other meta-data is filled in.  This is done in
  * case there is a backend where it is a very expensive operation to
  * get the audio data into a file so that it can be streamed out to
  * the caller.
  *
  *
  */

 int (*load_recording)(struct ast_vm_message *message);
 int (*unload_recording)(struct ast_vm_message *message);

 /*
  * Copy a message between folders in the same mailbox.
  */

 int (*copy)(struct ast_vm_message *message,
     const char *dst_folder);

 /*
  * Move a message between folders in the same mailbox.
  */

 int (*move)(struct ast_vm_message *message,
     const char *dst_folder);

 /* used by various storage modules for private data */
 void private[];
};
By: Kevin P. Fleming (kpfleming) 2005-02-02 15:42:06.000-0600

Mark,

All of this "indexing" stuff that you refer to in app_voicemail is _inside_ the message index file; what Tony and I are referring to is a layer below that, that would manage the storage and retrieval of that file, but be opaque to its contents. As it stands today, app_voicemail does not have an interface to search a mailbox by "callerid" without opening and parsing all the index files in that mailbox, so I don't see how the proposed res_storage would change that. Have I missed something?

jcollie,

Yes, I agree, that the layer that you are referring to is useful as well, but I see it as being a layer _on top_ of res_storage. However, it's use will be limited to other apps that want to peek into the voicemail storage; if there are no other apps that want to peek into there, then this abstraction layer will be used only by app_voicemail, so it has limited benefits (other than code organization and maintainability, which might be enough of a benefit). As far as app_voicemail generating keys, I was thinking that app_voicemail would use "voicemail" as its family name, "mailbox@context" as its key name, and then the folder name and message id as the item path ("INBOX/0001.txt", "INBOX/0001.msg"). Is this more complex than it should be?

Also note that my proposed model above has a flaw: storage_obj_list should instead be storage_path_list (since storage_find() does not return a list of opened storage objects, but rather a list of matching objects). So:

struct storage_path_list {
 struct storage_path path;
 struct storage_engine *eng;
 struct storage_path_list *next;
}

This would allow for the list to retain a pointer to the engine that returned the results, so that they won't have to be looked up again when storage_open() is called.

Also, storage_open() needs to support a "mode" parameter, with most of the same semantics (and mode values) as the system open() function. This would allow res_storage modules to optimize their behavior based on what has been requested: if an object is opened O_RDONLY, then a res_storage_odbc may choose to stream the data directly from the database, rather than copying out into a local file and streaming from there.
By: Jeffrey C. Ollie (jcollie) 2005-02-02 17:05:05.000-0600

Kevin,

I agree that your generic storage API would be useful, especially if it could be used elsewhere in Asterisk.  However, using *purely* your storage API you would need to leave in app_voicemail a large chunk of functionality like metadata serializing/unserializing and authentication that I would like to see abstracted out.  Also, you would not be able to take advantage of the special features of your underlying storage mechanism.  For example, with an IMAP backend (like rctel would like) you could store a voice mail message as a multipart MIME message - one part would be the message metadata (probably in a specially structured text/plain part) and the other part would be the audio data.  Authentication could be performed directly against the IMAP server as well.  That way you could access the IMAP store directly with an email client (or a webmail client too) and retrieve/manage your email that way.

Anyway, I've attached a .PNG that shows how I think that the APIs might be structured so that everyone wins.

Jeff
By: Brian West (bkw918) 2005-02-03 00:07:19.000-0600

OK START YOUR EDITORS.... ready.... set.... CODE!!!!!!!!!

bkw
By: Anthony Minessale (anthm) 2005-02-03 09:14:47.000-0600

What I was getting at in my example was a way to short circuit open and close syscalls so the whole API would be able to hijacked

so like for every openstream you call you end up with your custom code mimicing the open it would do something like open a tmp file pass back the fd
then when close was called it could close the fd and transport the file to wherever you want. see the t flag I added to file.c last fall with this option on, all audio is secretly written to /var/tmp untill its closed then its moved to where you expect it.  one way would be a url-like path to trigger the storage module  IMAP:fred@bedrock.com:1234/INBOX or FTP:me@server.com:1234:/pub/sounds then you could read and write to these transports seemlessly , I'd guess there would be issues with disturbed connectivity to be addressed somehow but then a simple storageprefix= option in the conf file could move the whole works. that may not solve the index file thing in voicemail but the sad truth is a new voicemail should be developed anyway without looking at 1 line of code from the old one.


I personally already started my own (in perl mwa haha) and I have the framework to record a file and upload it right to imap and i can tell you it rocks already. Shhhhh the secret is you dont need the index file anymore because you can store all that info as headers and IMAP proto allows you to just fetch hearders on messages.

I also suggest some kind of high level IVR menu api be developed first then use that to make the new voicemail because there are soooo many if's in it you can get dizzy following them.


I'm thinkin like.....

struct ast_menu main_menu;

static int action_sayhi(struct ast_channel *chan, struct ast_menu *menu) {
 return ast_streamfile(chan, "hithere", chan->language);
}

ast_menu_add(&main_menu, '1' , action_sayhi);
ast_menu_add(&main_menu, '#', default_action_quit);

ast_menu_run(chan, &main_menu);

actions are pluggables that in turn could implement and execute more menus.
that way all the actions are in funcs and you can move them around with ease.
and some common ones are availale like default_action_quit.
plus all the execution stuff looping can be controled from a params func.

struct ast_menu_params {
unsigned int flags;
char *invalid sound;
char *wait_sound;
};

struct ast_menu_params params;

ast_menu_add_params(&main_menu,&params);

waddya think?
By: Olle Johansson (oej) 2005-03-16 13:18:49.000-0600

No discussion for a month, discussion closed. Re-open when we have patches :-)

Maybe IMAP storage (or even WebDav) fits within realtime?