[Home]

Summary:ASTERISK-14597: [patch] Missing new-message notification for urgent messages
Reporter:Tomo Takebe (tomo1657)Labels:
Date Opened:2009-08-04 13:23:38Date Closed:2010-02-26 12:49:38.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20100224__issue15654.diff.txt
( 1) 20100225__issue15654.diff.txt
( 2) urgentnotify_v2_233576.patch
( 3) urgentnotify_v2.patch
( 4) urgentnotify.patch
Description:notify_new_message() is not called when urgent, new messages are left in a mailbox using non-IMAP file system storage.

Urgent messages are moved to the "Urgent" folder so the following condition to check INBOX for the new message (line 5312) is always false, and notify_new_message() is never called.

 if (ast_fileexists(fn, NULL, NULL)) {

A workaround would be to add an exception for urgent messages:

 if (ast_fileexists(fn, NULL, NULL) && !ast_strlen_zero(flag) && !strcmp(flag, "Urgent"))

which I have attacked a patch of.  There are other ways to approach this, though.  (Run notify_new_message() within the urgent message section or checking for dfn location instead of fn)

****** STEPS TO REPRODUCE ******

Leave an urgent message using non-IMAP storage.
Comments:By: Tomo Takebe (tomo1657) 2009-08-10 15:32:18

Please ignore the description; the issue wasn't just with "fn" checking.

Problem 1: notify_new_message() only counts and attaches new messages which are in INBOX, not Urgent.

Problem 2: By moving urgent messages to Urgent before notify_new_message(), the MWI externnotify and email attachment in the function don't work, but by moving urgent messages to Urgent after notify_new_message(), the mail count in the function will be wrong (counts the urgent message as new, non-urgent).

In IMAP this is not an issue because new and urgent messages are both in INBOX.


By: Tomo Takebe (tomo1657) 2009-08-11 13:27:01

Here's an updated patch (v2) that does the following:

1) notify_new_message() checks the flag argument to see if the new message is an Urgent message.  If so, it uses "Urgent" folder for "todir" instead of "INBOX".

2) When a message is moved to Urgent folder, the "fn" is updated to "dfn" (Urgent folder path), and "msgnum" is updated to "x" (Message number in Urgent folder) so that the correct attachment is sent from the Urgent folder instead of trying to find a non-existent message in INBOX.

3) The comment in the section "Created an Urgent message, moving file from %s to %s" wasn't useful because sfn and dfn weren't populated.  I moved it down a few lines.

The patch is against:
URL: http://svn.digium.com/svn/asterisk/branches/1.6.1
Revision: 211714
Last Changed Rev: 211586


By: Tomo Takebe (tomo1657) 2009-12-07 15:07:31.000-0600

The issue continues to exist in the latest svn trunk.  The patch has been constantly necessary in our deployment; could someone review it?

Urgent messages using file system storage do not trigger notify_new_messages() with the correct message counts.

I've updated the patch to be compatible with svn revision 233576.  (The only changes are line numbers, but hoping there will be bigger chance of being reviewed with the trunk)

To recreate issue:

1) Enable email delivery, set up externnotify path
2) leave an urgent, new message to an empty mailbox using file system storage
3) The email delivery and extennotify are not triggered since the Urgent message count is not being counted correctly

Proposed patch:
urgentnotify_v2_233576.patch for "apps/app_voicemail.c"
(Please see previous post for details on what was changed)

URL: http://svn.digium.com/svn/asterisk/trunk
Repository UUID: f38db490-d61c-443f-a65b-d21fe96a405b
Revision: 233576

Thank you!
By: Tomo Takebe (tomo1657) 2009-12-08 13:17:09.000-0600

Added to review board
https://reviewboard.asterisk.org/r/444/
By: Leif Madsen (lmadsen) 2010-01-07 11:23:00.000-0600

I've marked 16448 as a child of this issue as it needs to also be included in the fix for this issue.
By: Russell Bryant (russell) 2010-01-28 11:40:11.000-0600

Thanks a lot for the patch.  Hopefully we can get a committer on this soon to do final review and commit.  I have suspended the review request in the mean time, but we can open it back up if the committer decides it is needed once they take this on.
By: Tilghman Lesher (tilghman) 2010-02-23 02:07:03.000-0600

I have updated the patch against trunk, along with some changes to the functions, such that they act in predictable (and documented) ways.  One problem was that messagecount did not act consistently between file storage methods.  Also, in certain circumstances, an Urgent message was counted twice.
By: Digium Subversion (svnbot) 2010-02-26 12:41:58.000-0600

Repository: asterisk
Revision: 249187

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r249187 | tilghman | 2010-02-26 12:41:57 -0600 (Fri, 26 Feb 2010) | 18 lines

Cleanups to fix bugs in the VM count API functions.

- Urgent voicemails were not attached, because the attachment code looked in the wrong folder.
- Urgent voicemails were sometimes counted twice when displaying the count of new messages.
- Backends were inconsistent as to which voicemails each API counted.
- Unit tests added to verify behavior in the future.

(closes issue ASTERISK-14597)
Reported by: tomo1657
Patches:
      20100225__issue15654.diff.txt uploaded by tilghman (license 14)
Tested by: tilghman

(closes issue ASTERISK-15316)
Reported by: hevad

Review: https://reviewboard.asterisk.org/r/525/

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

http://svn.digium.com/view/asterisk?view=rev&revision=249187
By: Digium Subversion (svnbot) 2010-02-26 12:48:58.000-0600

Repository: asterisk
Revision: 249189

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

------------------------------------------------------------------------
r249189 | tilghman | 2010-02-26 12:48:58 -0600 (Fri, 26 Feb 2010) | 24 lines

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

........
 r249187 | tilghman | 2010-02-26 12:41:57 -0600 (Fri, 26 Feb 2010) | 18 lines
 
 Cleanups to fix bugs in the VM count API functions.
 
 - Urgent voicemails were not attached, because the attachment code looked in the wrong folder.
 - Urgent voicemails were sometimes counted twice when displaying the count of new messages.
 - Backends were inconsistent as to which voicemails each API counted.
 
 (closes issue ASTERISK-14597)
  Reported by: tomo1657
  Patches:
        20100225__issue15654.diff.txt uploaded by tilghman (license 14)
  Tested by: tilghman
 
 (closes issue ASTERISK-15316)
  Reported by: hevad
 
 Review: https://reviewboard.asterisk.org/r/525/
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=249189
By: Digium Subversion (svnbot) 2010-02-26 12:49:37.000-0600

Repository: asterisk
Revision: 249190

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

------------------------------------------------------------------------
r249190 | tilghman | 2010-02-26 12:49:37 -0600 (Fri, 26 Feb 2010) | 24 lines

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

........
 r249187 | tilghman | 2010-02-26 12:41:57 -0600 (Fri, 26 Feb 2010) | 18 lines
 
 Cleanups to fix bugs in the VM count API functions.
 
 - Urgent voicemails were not attached, because the attachment code looked in the wrong folder.
 - Urgent voicemails were sometimes counted twice when displaying the count of new messages.
 - Backends were inconsistent as to which voicemails each API counted.
 
 (closes issue ASTERISK-14597)
  Reported by: tomo1657
  Patches:
        20100225__issue15654.diff.txt uploaded by tilghman (license 14)
  Tested by: tilghman
 
 (closes issue ASTERISK-15316)
  Reported by: hevad
 
 Review: https://reviewboard.asterisk.org/r/525/
........

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

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