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

Matt Jordan reviewboard at asterisk.org
Wed Jun 25 18:16:25 CDT 2014 

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


Well, this ended up being a lot more complex than we thought. Ugh.

First, let's make sure that the existing tests that hit accountcodes still function. Verify:
* tests/cdr/console_dial-sip_transfer
* tests/cdr/cdr_properties/blind-transfer-accountcode

Both of these use accountcode propagation in their tests, and - in the case of blind-transfer-accountcode - also check for propagation across Local channels.

This will need some tests updated in the testsuite to verify this functionality. While we could overload the existing bridging tests, there's enough propagation rules in here that it would be worthwhile having a few tests that just make sure that accountcodes get bounced across channels.

I'd suggest tests that cover the following scenarios:
* A channel with an accountcode (and only an accountcode) entering into a bridge with a channel that does/does not have an accountcode as well
* A channel with an accountcode and a peeraccount entering into a bridge with a channel that has an accountcode, and does/does not have a peeraccount as well
* Propagation of an accountcode down a chain of 2 non-optimizing Local channels




/branches/12/main/bridge_basic.c
<https://reviewboard.asterisk.org/r/3601/#comment22500>

    Why did these personalities lose the ast_bridge_base_v_table?
    
    This means, for example, that the 'normal' personality will no longer get a vtable method for pulling, nor will it get a vtable method for bridge destruction.



/branches/12/main/event.c
<https://reviewboard.asterisk.org/r/3601/#comment22501>

    For the tweaks in this section that are unrelated to the accountcode changes, please commit them on a separate patch.
    
    They don't need to be put up for review again.
    
    Note that changes in datatypes in event.h are typically dangerous - interactions with other Asterisk systems will break (badly) if we change the data types here. Luckily, CEL and Security events are not shared across corosync, so this should be okay.


- Matt Jordan


On June 23, 2014, 3:56 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3601/
> -----------------------------------------------------------
> 
> (Updated June 23, 2014, 3:56 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-65
>     https://issues.asterisk.org/jira/browse/AFS-65
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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.
> However, a side effect of this is it will overwrite any accountcode set by
> the SIP channel driver configuration for the SIP/200 channel with the
> accountcode from the Local;2 channel.  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.  The altered accountcode on SIP/200 will remain
> until the local channels optimized out when 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 goal is to have the accountcode values become as follows:
> 
> 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 explicit
> user command by the following methods:
> 
> 1) A channel originate method that can specify an accountcode to use.
> 
> 2) Dialplan using CHANNEL(accountcode).
> 
> 3) 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) Before dialing from the calling channel's peeraccount value; or as a
> legacy fallback the calling channel's accountcode.
> 
> 3) Explicit user command as already indicated.
> 
> 4) Entering a bridge from a peer channel's peeraccount value.
> 
> You can specify the accountcode for an outgoing channel that does not have
> one 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
> bridge.  The peeraccount value only makes sense for channels in two party
> bridges.
> 
> * Made peeraccount functional by changing accountcode propagation as
> described above.
> 
> * 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 to not call the base pull method twice.
> 
> * 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 some incorrect CEL event ie types.  These seem to only be used
> by the unit tests now.
> 
> * 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.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/parking/parking_bridge_features.c 417163 
>   /branches/12/main/pbx.c 417163 
>   /branches/12/main/event.c 417163 
>   /branches/12/main/dial.c 417163 
>   /branches/12/main/core_unreal.c 417163 
>   /branches/12/main/channel.c 417163 
>   /branches/12/main/cel.c 417163 
>   /branches/12/main/bridge_channel.c 417163 
>   /branches/12/main/bridge_basic.c 417163 
>   /branches/12/main/bridge.c 417163 
>   /branches/12/include/asterisk/channel.h 417163 
>   /branches/12/include/asterisk/bridge_channel.h 417163 
>   /branches/12/apps/app_queue.c 417163 
>   /branches/12/apps/app_followme.c 417163 
>   /branches/12/apps/app_dial.c 417163 
>   /branches/12/UPGRADE.txt 417163 
> 
> Diff: https://reviewboard.asterisk.org/r/3601/diff/
> 
> 
> Testing
> -------
> 
> Set the accountcode on the initial channel and let the channel driver supply or not the accountcode for the other channel.
> Without the channel driver supplying the accountcode, the outgoing channel got the same accountcode as the initial channel.
> With the channel driver supplying the accountcode, the outgoing channel kept its accountcode.
> 
> Let the channel driver supply the accountcode for all endpoint channels.
> Performed a DTMF attended transfer with consultation and creation of 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.
> 
> Throughout each of these tests, the CEL log had the expected peeraccount value.  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/20140625/f043d54d/attachment-0001.html>

More information about the asterisk-dev mailing list