MT#65224 renumber voicemail msgnum on folder change and delete

Asterisk uses its built-in ODBC voicemail storage against
kamailio.voicemail_spool, which expects msgnum to be contiguous
0..N-1 within each (mailboxuser, dir) folder. Asterisk maintains
that invariant itself, but the panel rewrote dir (or deleted a
row) without ever touching msgnum, leaving gaps and duplicates
that make asterisk mis-play and mis-count the messages.

Add two helpers to Utils::Subscriber reproducing the asterisk
semantics:

* renumber_voicemail_folder() compacts a single folder to 0..N-1
  ordered by (msgnum, id). It skips greetings, which live in the
  same table with msgnum -1.
* move_voicemail_to_folder() appends the message at the end of
  the target folder the way asterisk does (last_msg_index + 1),
  then compacts both the source and the target folder.

Both run inside a transaction, so the web UI paths that have no
transaction guard of their own are covered as well.

Use them in all four affected paths:

* PATCH/PUT /api/voicemails/{id} (Role::API::Voicemails)
* DELETE /api/voicemails/{id} (Controller::API::VoicemailsItem)
* mark_voicemail_read() for the CSC/admin listen action
* delete_voicemail() in the subscriber web UI

The compaction is deliberately self-healing, so it also repairs
folders already corrupted by the previous behaviour.

Change-Id: I665de1966bc37c1069ff56fb30c99da85f224f71
master
Marco Capetta 1 week ago
parent 22623e7c2f
commit d01cff87ba

@ -125,9 +125,15 @@ sub DELETE :Allow {
my $cli = $item->mailboxuser->provisioning_voip_subscriber->username;
my $uuid = $item->mailboxuser->provisioning_voip_subscriber->uuid;
my $mailboxuser = $item->get_column('mailboxuser');
my $dir = $item->dir;
$item->delete;
NGCP::Panel::Utils::Subscriber::renumber_voicemail_folder(
c => $c, mailboxuser => $mailboxuser, dir => $dir,
);
NGCP::Panel::Utils::Subscriber::vmnotify(c => $c, cli => $cli, uuid => $uuid);
$guard->commit;

@ -4328,6 +4328,8 @@ sub delete_voicemail :Chained('voicemail') :PathPart('delete') :Args(0) {
my $file = $c->stash->{voicemail};
my $cli = $file->mailboxuser->provisioning_voip_subscriber->username;
my $uuid = $file->mailboxuser->provisioning_voip_subscriber->uuid;
my $mailboxuser = $file->get_column('mailboxuser');
my $dir = $file->dir;
$c->detach('/denied_page')
if(($c->user->roles eq "admin" || $c->user->roles eq "reseller" ||
@ -4335,6 +4337,9 @@ sub delete_voicemail :Chained('voicemail') :PathPart('delete') :Args(0) {
try {
$c->stash->{voicemail}->delete;
NGCP::Panel::Utils::Subscriber::renumber_voicemail_folder(
c => $c, mailboxuser => $mailboxuser, dir => $dir,
);
NGCP::Panel::Utils::Subscriber::vmnotify(c => $c, cli => $cli, uuid => $uuid);
NGCP::Panel::Utils::Message::info(
c => $c,

@ -117,17 +117,17 @@ sub update_item {
);
my $f = $resource->{folder};
my $upresource = {};
$upresource->{dir} = $item->dir;
my $dir_old = $item->dir;
$upresource->{dir} =~ s/\/[^\/]+$/\/$f/;
my $dir_new = $dir_old;
$dir_new =~ s/\/[^\/]+$/\/$f/;
$item->update($upresource);
if ($dir_old ne $dir_new) {
NGCP::Panel::Utils::Subscriber::move_voicemail_to_folder(
c => $c, voicemail => $item, dir_new => $dir_new,
);
my $cli = $item->mailboxuser->provisioning_voip_subscriber->username;
my $uuid = $item->mailboxuser->provisioning_voip_subscriber->uuid;
if ($dir_old ne $upresource->{dir}) {
my $cli = $item->mailboxuser->provisioning_voip_subscriber->username;
my $uuid = $item->mailboxuser->provisioning_voip_subscriber->uuid;
NGCP::Panel::Utils::Subscriber::vmnotify(c => $c, cli => $cli, uuid => $uuid);
}

@ -2870,6 +2870,60 @@ sub vmnotify {
return;
}
sub renumber_voicemail_folder {
my (%params) = @_;
my ($c, $mailboxuser, $dir) = @params{qw(c mailboxuser dir)};
#msgnum must be contiguous 0..N-1 per folder, as asterisk's odbc
#voicemail storage expects it
my $vm_rs = $c->model('DB')->resultset('voicemail_spool')->search({
mailboxuser => $mailboxuser,
dir => $dir,
msgnum => { '>=' => 0 },
},{
order_by => [{ -asc => 'msgnum' }, { -asc => 'id' }],
});
return $c->model('DB')->txn_do(sub {
my $msgnum = 0;
for my $msg ($vm_rs->all) {
$msg->update({ msgnum => $msgnum }) if $msg->msgnum != $msgnum;
$msgnum++;
}
return $msgnum;
});
}
sub move_voicemail_to_folder {
my (%params) = @_;
my ($c, $voicemail, $dir_new) = @params{qw(c voicemail dir_new)};
my $dir_old = $voicemail->dir;
return if $dir_old eq $dir_new;
my $mailboxuser = $voicemail->get_column('mailboxuser');
$c->model('DB')->txn_do(sub {
#append at the end of the target folder like asterisk does (last_msg_index + 1)
my $msgnum_max = $c->model('DB')->resultset('voicemail_spool')->search({
mailboxuser => $mailboxuser,
dir => $dir_new,
msgnum => { '>=' => 0 },
})->get_column('msgnum')->max;
$voicemail->update({
dir => $dir_new,
msgnum => defined $msgnum_max ? $msgnum_max + 1 : 0,
});
#compact both folders
renumber_voicemail_folder(c => $c, mailboxuser => $mailboxuser, dir => $dir_old);
renumber_voicemail_folder(c => $c, mailboxuser => $mailboxuser, dir => $dir_new);
});
return;
}
sub mark_voicemail_read {
my (%params) = @_;
@ -2877,7 +2931,7 @@ sub mark_voicemail_read {
my $voicemail = $params{voicemail};
my $dir = $voicemail->dir;
$dir =~ s/INBOX$/Old/;
$voicemail->update({ dir => $dir });
move_voicemail_to_folder(c => $c, voicemail => $voicemail, dir_new => $dir);
return;
}

Loading…
Cancel
Save