[Home]

Summary:ASTERISK-06329: [branch][post 1.4] Make storage of voicemail and greetings slightly more abstract
Reporter:Andrew Moise (chops)Labels:
Date Opened:2006-02-15 21:06:56.000-0600Date Closed:2011-06-07 14:10:39
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 2006-02-15-storage-abstraction.patch
( 1) 2006-02-18-storage-abstraction.patch
Description:The storage of voicemails and greetings can happen either on disk or in an ODBC database.  Currently, the choice is based on a compiler symbol and a block of #define statements, which are not entirely clear (e.g. the macro which renames a file takes 8 arguments, which are the letters a through h).

This patch creates a struct vm_storage, which is several function pointers which describe the storage interface.  This interface is implemented for ODBC and /var/spool storage, so that existing functionality is exactly preserved, except that the code uses function pointers instead of macros, which is marginally clearer.

Benefits of this include the possibility of splitting ODBC access into a separate file (or even a separate module), and the possibility of choosing ODBC or file-based access at runtime.  Choosing ODBC or not at runtime is actually trivial with this patch, but I don't think it's a good idea, because the two storage abstractions are not cleanly separated.  AFAICS, if you have voicemails stored in /var/spool, and then you switch to ODBC access to try it out, some of your existing voicemails may be destroyed by your process of experimentation.  I think we should fix that before making it easy to switch.  That problem was around before this patch.

This patch doesn't change the existing interface to any given storage function, which means that (since less of the complexity is hidden in macros), some functions got quite a bit sillier.

This patch is on top of SVN 10241 with my previous mailbox terminology patch; see http://bugs.digium.com/view.php?id=6503 for that patch.

This is a small part of my voicemail refactoring; see http://www.voip-info.org/wiki/view/Asterisk+Voicemail+Redesign for more.  I'm posting this here to hear feedback on it; though I'd be happy to see it committed, I'm expecting to have to listen to some criticism before this stuff starts getting committed :-).
Comments:By: Andrew Moise (chops) 2006-02-18 13:20:07.000-0600

My previous refactoring patch got applied in a modified form, so I've uploaded a new patch against SVN 10353.  This one (the 2006-02-18 one) doesn't require any previous patches in order to apply.
By: Olle Johansson (oej) 2006-03-09 14:26:31.000-0600

Please mail to the asterisk-dev mailing list. We need to get some work done in voicemail again, especially dynamic menus. The bug tracker is, as you've seen, not a very good forum to start a discussion.

Keep up the good work!
By: Olle Johansson (oej) 2006-03-10 02:08:57.000-0600

Opened branch "voicemail-ng" and committed this patch.
By: Andrew Moise (chops) 2006-03-10 14:09:00.000-0600

I did mail the asterisk-dev mailing list; my message is at http://threebit.net/mail-archive/asterisk-dev/msg00596.html .  I also added a summary to the wiki at http://www.voip-info.org/wiki/view/Asterisk+Voicemail+Redesign .  Neither of those generated any discussion, and Kevin told me that my patches couldn't be applied until after an alternative IMAP synchronization patch had been applied (which would presumably change app_voicemail.c quite a bit).

I stopped working on this at that point, since it didn't seem that I was accomplishing anything.
By: Olle Johansson (oej) 2006-03-10 14:18:57.000-0600

I was not aware of that IMAp stuff. Will check it up and see what's going on. Thanks for all good work so far! Hopefully we can continue from this or another base code soon.
By: Serge Vecher (serge-v) 2006-05-22 11:53:10

chops: is 6504 a duplicate of 6512?
By: Andrew Moise (chops) 2006-05-26 08:41:49

6504 isn't a duplicate of 6512.  6504 deals with external storage of voicemails (whether in files or in a database), whereas 6512 deals with the internal abstraction of voicemails (it uses an abstract structure and access functions, rather than the current practice of directly manipulating the in-memory and on-disk structures in every place where they're used).

As I said, I'm not planning on doing any more work on this.  The deadline for the bounty I was interested in has expired, and my understanding is that there's an IMAP patch which is currently slated for inclusion instead of my work (http://svn.digium.com/svn/asterisk/team/jrothenberger/asterisk-imap/).  In any case, these refactoring patches won't apply any more after that IMAP work is applied, because asterisk-imap modifies the same sections of code without... well... quite as judicious a cleanup effort :-).
By: jmls (jmls) 2006-10-31 12:27:36.000-0600

is this issue still valid / wanted ?
By: jmls (jmls) 2006-11-20 11:45:16.000-0600

ping. housekeeping
By: Serge Vecher (serge-v) 2006-11-21 09:12:34.000-0600

I think this work is still valid/useful. The question here is how to proceed, as chops has indicated that he developed this for the bounty and is not interested in following through with updates until the code is merged into trunk. I think this either needs to be picked up by another developer and closed and be lost in the dustbin of history.
By: Serge Vecher (serge-v) 2006-11-29 10:22:41.000-0600

jason: is this work useful/complementary/duplicate/related to your astse effort?
By: Jason Parker (jparker) 2006-11-29 10:32:45.000-0600

well...

yes, it is related, but unfortunately, the way it's written, it won't be quite useful in the new ast_storage code (much of it would be removed/changed, to support arbitrary storage methods).

At this point, in order for this patch to make it into trunk, it would need to be extended to support IMAP, however, the way IMAP works is a bit different than the way ODBC worked, so it will be a bit more difficult, and perhaps not feasible.

Do we want to suspend and revisit this in the future?  I have a feeling chops isn't going to want to update the patch for IMAP.
By: Olle Johansson (oej) 2007-02-11 12:45:28.000-0600

qwell: Doesn't seem like the reporter answers. Are we ready to close this, since the patch is outdated and we're working on an alternative solution?

Chops: Last call to reply....
By: Andrew Moise (chops) 2007-02-11 12:54:37.000-0600

I'm still here, but it doesn't seem that there's much to say.  As I said, I'm not going to do any more work on the voicemail app.

Whether to close my bug or keep the patch around for some theoretical future usefulness is entirely up to you guys; I'm okay either way.
By: Dwayne Hubbard (dhubbard) 2007-03-06 10:19:47.000-0600

ast_storage is on the way.  thanks for your time and effort.  :-)
By: Dwayne Hubbard (dhubbard) 2007-03-06 18:08:04.000-0600

accidental re-opening of this issue.
By: Digium Subversion (svnbot) 2008-01-15 17:21:56.000-0600

Repository: asterisk
Revision: 12489

_U  team/oej/voicemail-ng/
U   team/oej/voicemail-ng/apps/app_voicemail.c

------------------------------------------------------------------------
r12489 | oej | 2008-01-15 17:21:56 -0600 (Tue, 15 Jan 2008) | 2 lines

Patch from issue ASTERISK-6329 - storage abstraction

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=12489