[Home]

Summary:ASTERISK-12875: [patch] Addition of a Mailbox id facility to allow shared mailboxes to work
Reporter:Howard Wilkinson (howardwilkinson)Labels:
Date Opened:2008-10-11 06:27:21Date Closed:2009-02-05 15:06:14.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail/IMAP
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-1.4.21.2-appvoicemail-sharedimap-mbxid.patch
Description:When a user is set up to share a mailbox using IMAP as the backing store then messages posted into the mailbox are only visible to that user. For example
with 2 users having the following voice mailbox definitions:

6000 => 6000,Shared mailbox,shared@example.com,,|imapuser=sharedmail
6001 => 6001,Shared mailbox,shared@example.com,,|imapuser=sharedmail

the voicemail is stored in a common folder and can be accessed via a mail client as common data but each user receives just the messages that were left for them.

I have implemented a patch against 1.4.21.2 which adds a new option 'imapmbxid' that allows all users to see all messages left for that mbxid.

This is used as follows.

6000 => 6000,Shared mailbox,shared@example.com,,|imapuser=sharedmail,imapmbxid=6000
6001 => 6001,Shared mailbox,shared@example.com,,|imapuser=sharedmail,imapmbxid=6000

and then when retrieving records the users are presented with all of the records.


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

The patch alters the header that is written into the mailbox and search for to find matching records. 'X-Asterisk-VM-Extension'

This patch aggravates a bug in the asterisk code where the mail_ and imap_ code is not multi-threadable. I have posted a patch that stop this bug from crashing asterisk and I am working on a slight improvement for this to make the code logical sound.
Comments:By: Howard Wilkinson (howardwilkinson) 2008-10-11 06:28:52

Apply patch 0013653 before this one to stop random crashes inside the UW imap library.
By: Leif Madsen (lmadsen) 2008-10-11 08:56:29

Hrmmm... is there maybe a better option name we can use that makes the purpose a little more obvious? I know that mbxid probably describes what is going on, but maybe we can call it something like... imapsharedvm ?
By: Mark Michelson (mmichelson) 2008-10-20 17:07:34

Thanks for the patch. I've taken a look at it and from a technical standpoint, it is well done. There are two problems with the patch, though, and neither are really that big.

1. There are several places where you go against Asterisk's coding guidelines. For instance the first line of the patch is indented using 8 spaces instead of a tab. The second line of the second hunk of the patch also includes 8 spaces instead of a tab. The other thing that leapt out at me is the lack of spaces around the '?' and ':' in your ternary conditionals.

2. Instead of using the comparison vmu->imapmbxid[0] != '\0' please use !ast_strlen_zero(vmu->imapmbxid).

I agree with blitzrage about making the name of the option better imply what it is used for. I'm not going to try to suggest another name since I'm definitely no UI expert and I'd probably just come up with something worse :)
By: Digium Subversion (svnbot) 2009-02-05 14:47:52.000-0600

Repository: asterisk
Revision: 173696

U   branches/1.4/apps/app_voicemail.c

------------------------------------------------------------------------
r173696 | jpeeler | 2009-02-05 14:47:51 -0600 (Thu, 05 Feb 2009) | 12 lines

Add new configuration option to make shared IMAP mailboxes function as expected.

The new option is "imapvmshareid" which is an ID to tag multiple mailboxes
using the same IMAP storage location to function as one mailbox. This allows
all messages to be retrieved for any user in the group. The patch alters the
'X-Asterisk-VM-Extension' header that is responsible for matching voicemails
for a given user.

(closes issue ASTERISK-12875)
Reported by: howardwilkinson


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

http://svn.digium.com/view/asterisk?view=rev&revision=173696
By: Digium Subversion (svnbot) 2009-02-05 15:00:26.000-0600

Repository: asterisk
Revision: 173697

_U  trunk/
U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r173697 | jpeeler | 2009-02-05 15:00:26 -0600 (Thu, 05 Feb 2009) | 18 lines

Merged revisions 173696 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r173696 | jpeeler | 2009-02-05 14:47:51 -0600 (Thu, 05 Feb 2009) | 12 lines
 
 Add new configuration option to make shared IMAP mailboxes function as expected.
 
 The new option is "imapvmshareid" which is an ID to tag multiple mailboxes
 using the same IMAP storage location to function as one mailbox. This allows
 all messages to be retrieved for any user in the group. The patch alters the
 'X-Asterisk-VM-Extension' header that is responsible for matching voicemails
 for a given user.
 
 (closes issue ASTERISK-12875)
 Reported by: howardwilkinson
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=173697
By: Digium Subversion (svnbot) 2009-02-05 15:04:58.000-0600

Repository: asterisk
Revision: 173698

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_voicemail.c

------------------------------------------------------------------------
r173698 | jpeeler | 2009-02-05 15:04:57 -0600 (Thu, 05 Feb 2009) | 25 lines

Merged revisions 173697 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r173697 | jpeeler | 2009-02-05 15:00:26 -0600 (Thu, 05 Feb 2009) | 18 lines
 
 Merged revisions 173696 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r173696 | jpeeler | 2009-02-05 14:47:51 -0600 (Thu, 05 Feb 2009) | 12 lines
   
   Add new configuration option to make shared IMAP mailboxes function as expected.
   
   The new option is "imapvmshareid" which is an ID to tag multiple mailboxes
   using the same IMAP storage location to function as one mailbox. This allows
   all messages to be retrieved for any user in the group. The patch alters the
   'X-Asterisk-VM-Extension' header that is responsible for matching voicemails
   for a given user.
   
   (closes issue ASTERISK-12875)
   Reported by: howardwilkinson
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=173698
By: Digium Subversion (svnbot) 2009-02-05 15:06:13.000-0600

Repository: asterisk
Revision: 173699

_U  branches/1.6.1/
U   branches/1.6.1/apps/app_voicemail.c

------------------------------------------------------------------------
r173699 | jpeeler | 2009-02-05 15:06:13 -0600 (Thu, 05 Feb 2009) | 25 lines

Merged revisions 173697 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r173697 | jpeeler | 2009-02-05 15:00:26 -0600 (Thu, 05 Feb 2009) | 18 lines
 
 Merged revisions 173696 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r173696 | jpeeler | 2009-02-05 14:47:51 -0600 (Thu, 05 Feb 2009) | 12 lines
   
   Add new configuration option to make shared IMAP mailboxes function as expected.
   
   The new option is "imapvmshareid" which is an ID to tag multiple mailboxes
   using the same IMAP storage location to function as one mailbox. This allows
   all messages to be retrieved for any user in the group. The patch alters the
   'X-Asterisk-VM-Extension' header that is responsible for matching voicemails
   for a given user.
   
   (closes issue ASTERISK-12875)
   Reported by: howardwilkinson
 ........
................

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

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