[asterisk-dev] [Code Review] 3601: accountcode: Slightly change accountcode propagation.

[rmudgett]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3601/
-----------------------------------------------------------

(Updated June 30, 2014, 6:24 p.m.)


Review request for Asterisk Developers.


Changes
-------

* Changed accountcode/peeraccount code propagation to be closer to the original propagation.  The calling channel's accountcode/peeraccount values overwrite the outgoing channels values.  This allows the tests/cdr/cdr_properties/blind-transfer-accountcode to pass even though the reason it failed before was because of an intentional change in behavior.

* Reworked the refactored ast_bridge_channel_update_accountcodes() helper functions to take ast_channel pointers instead of ast_bridge_channel pointers since they only needed to be ast_channel pointesr.


Bugs: AFS-65
    https://issues.asterisk.org/jira/browse/AFS-65


Repository: Asterisk


Description (updated)
-------

Accountcode propagation:

The current behavior is to simply set the accountcode of an outgoing
channel to the accountcode of the channel initiating the call.  It was
done this way a long time ago to allow the accountcode set on the SIP/100
channel to be propagated to a local channel so the dialplan execution on
the Local;2 channel would have the SIP/100 accountcode available.

SIP/100 -> Local;1/Local;2 -> SIP/200

Propagating the SIP/100 accountcode to the local channels is very useful.
Without any dialplan manipulation, all channels in this call would have
the same accountcode.

Using dialplan, you can set a different accountcode on the SIP/200 channel
either by setting the accountcode on the Local;2 channel or by the Dial
application's b(pre-dial), M(macro) or U(gosub) options, or by the
FollowMe application's b(pre-dial) option, or by the Queue application's
macro or gosub options.  Before Asterisk v12, the altered accountcode on
SIP/200 will remain until the local channels optimize out and the
accountcode would change to the SIP/100 accountcode.

Asterisk v1.8 attempted to add peeraccount support but ultimately had to
punt on the support.  The peeraccount support was rendered useless because
of how the CDR code needed to unconditionally force the caller's
accountcode onto the peer channel's accountcode.  The CEL events were thus
intentionally made to always use the channel's accountcode as the
peeraccount value.

With the arrival of Asterisk v12, the situation has improved so
peeraccount support can be made to work.  Using the indicated example, the
the accountcode values become as follows when the peeraccount is set on
SIP/100 before calling SIP/200:

SIP/100 ---> Local;1 ---- Local;2 ---> SIP/200
acct: 100 \/ acct: 200 \/ acct: 100 \/ acct: 200
peer: 200 /\ peer: 100 /\ peer: 200 /\ peer: 100

If a channel already has an accountcode it can only change by the
following explicit user actions:

1) A channel originate method that can specify an accountcode to use.

2) The calling channel propagating its non-empty peeraccount or its
non-empty accountcode if the peeraccount was empty to the outgoing
channel's accountcode before initiating the dial.  (e.g., Dial, FollowMe,
and Queue)

3) Dialplan using CHANNEL(accountcode).

4) Dialplan using CHANNEL(peeraccount) on the other end of a local
channel pair.

If a channel does not have an accountcode it can get one from the
following places:

1) The channel driver's configuration at channel creation.

2) Explicit user action as already indicated.

3) Entering a basic or stasis-mixing bridge from a peer channel's
peeraccount value.

You can specify the accountcode for an outgoing channel by setting the
CHANNEL(peeraccount) before using the Dial, FollowMe, and Queue
applications.

Accountcode and peeraccount values propagate to an outgoing channel before
dialing.  Accountcodes also propagate when channels enter or leave a basic
or stasis-mixing bridge.  The peeraccount value only makes sense for
channels in two party mixing bridges.

* Made peeraccount functional by changing accountcode propagation as
described above.

* Fixed CEL extracting the wrong ie value for the peeraccount.  This was
done intentionally in Asterisk v1.8 when that version had to punt on
peeraccount.

* Fixed a few places dealing with accountcodes that were reading from
channels without the lock held.


NOTE: This patch is against v12 because that was the original target for
the patch.  However, because the peeraccount changes were more extensive
than anticipated and resulted in a larger behavior change it will only be
checked into trunk.


NOTE: The following fixes will be extracted from this review to become
part of the ASTERISK-23852 patch.  The changes are in bridge_channel.h,
bridge_channel.c, and bridge_basic.c.

* Fixed the basic bridge sub-class to update peeraccount codes when the
number of channels in the bridge drops back down to two parties.

* Refactored ast_bridge_channel_update_accountcodes() to handle channels
joining/leaving the bridge.

* Fixed the basic bridge sub-class to not call the base bridge class pull
method twice.


Diffs (updated)
-----

  /branches/12/res/parking/parking_bridge_features.c 417700 
  /branches/12/main/pbx.c 417700 
  /branches/12/main/dial.c 417700 
  /branches/12/main/core_unreal.c 417700 
  /branches/12/main/channel.c 417700 
  /branches/12/main/cel.c 417700 
  /branches/12/main/bridge_channel.c 417700 
  /branches/12/main/bridge_basic.c 417700 
  /branches/12/main/bridge.c 417700 
  /branches/12/include/asterisk/channel.h 417700 
  /branches/12/include/asterisk/bridge_channel.h 417700 
  /branches/12/apps/app_queue.c 417700 
  /branches/12/apps/app_followme.c 417700 
  /branches/12/apps/app_dial.c 417700 
  /branches/12/UPGRADE.txt 417700 

Diff: https://reviewboard.asterisk.org/r/3601/diff/


Testing (updated)
-------

* tests/cdr/console_dial-sip_transfer passes.
* tests/cdr/cdr_properties/blind-transfer-accountcode passes.

* Set the accountcode on the calling channel and let the channel driver
supply or not the accountcode for the outgoing channel.  The acccountcode
on the calling channel forces itself on the outgoing channel.  This is the
same as previous behavior.

* Set the accountcode and peeraccount code on the calling channel and let
the channel driver supply or not the acccountcode for the outgoing
channel.  The outgoing channel's accountcode and peeraccount got the
calling channel's peeraccount and accountcode values respectively.

* Let dialplan set accountcodes on channels and performed a DTMF attended
transfer to create a three party bridge.  When the transferrer channel
left the three party bridge, the remaining two channels got the
peeraccount updated to the other party.  Unfortunately, you only see the
updated peeraccount values in the CEL log when the channels leave the
bridge.

Throughout each of these tests, the CEL log had the expected peeraccount
value.  Some interpolation is needed in the CEL log for complicated
accountcode manipulation because there aren't enough events to indicate
when the account codes change.  Note the peeraccount value is meaningless
if the bridge contains more than two parties.


Thanks,

rmudgett

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140630/ec086a97/attachment.html>