<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2462/">https://reviewboard.asterisk.org/r/2462/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21196">ASTERISK-21196</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch reworks CDRs for Asterisk 12. For more information on why and what the records should look like, please view the specification on the wiki: https://wiki.asterisk.org/wiki/display/AST/Asterisk+12+CDR+Specification
Since the Bridge API migration resulted in all CDR code in features.c no longer being applicable, and the model of masquerades during transfers now no longer occurs (mostly), the vast majority of complex CDR code sprinkled throughout Asterisk was either wrong, or would never be executed. Rather than piling the CDR code back into the Bridging API, this patch instead builds CDRs off of messages received over the Stasis-Core message bus. However, as a result, all direct manipulation of CDRs by channel drivers, applications, and the Asterisk core is now highly suspect and most likely null and void.
This patch rips most of the existing code out.
Messages received over Stasis-Core are first dispatched by top level message handlers. The handlers job is to (a) find the CDRs that are affected by the message, (b) dispatch the message to the CDRs, and (c) based on the statuses, determine if more CDRs are needed.
Each CDR has a virtual table that determines what effect the message has on it. Depending on the state of the CDR, the message may be dropped; invalid; or it may alter the state of the CDR. CDRs transition states when they need to, and the top level message handlers generally don't care what state the CDR is in.
CDRs exist in a chain. As a channel does stuff, additional CDRs may be appended to the chain, and there may be multiple active CDRs at any given moment in time.
As a result of all of this, ForkCDR, NoCDR, and ResetCDR have essentially been rewritten and simplified greatly. The need for ForkCDR is significantly less, as additional CDRs are created between all pairs of channels in multi-party and transfer situations.
A new CDR function, CDR_PROP, has been added that lets the dialplan override who is the Party A when two channels enter into a bridge together. This function also lets you disable/re-enable CDRs on a channel, which precludes the need for NoCDR and one of ResetCDR's options.
Finally, a set of unit tests have been written that cover the existing functionality.
This patch is not complete - there is still some additional work to do and testing to perform. The following needs to be covered under subsequent reviews:
* Linkedid propagation. As CEL will need this as well, this will most likely become a core concept (if possible)
* Park. A unit test exists for this, however, the holding bridge is not yet handled properly. This is rather trivial, as most of the Bridge state logic simply needs to be avoided if a holding bridge is entered.
* Call Pickup. We need the Stasis Message for this before this can be completed.
* Attended transfers into an application. We need the Stasis Message for this too.
* Quite a few BUGBUG comments in app_queue (which needs the Dial messages - see ASTERISK-21551) and the media channel (see ASTERISK-21713)
Hopefully, this patch is far enough along that the approach can be verified and major bugs can be commented on. Ideally, this would go into bridge_construction so that the Test Suite running against it can be updated.
A few open questions:
* What do we want to do with peeraccount? In general, this property on the channel needs to be set by the bridging API, but much like the BRIDGEPEER channel variable, has issues in reasonably complex scenarios. Right now, the peeraccount is passed through the CDRs by virtue of the CDR engine knowing who the channel is bridged with, but inspecting this value through a function becomes problematic since we can't return a single value in multi-party bridges. CEL will also need an answer to this question. (There is also some bridging logic that applies a channel's accountcode to who its bridged with if the other channel doesn't have an accountcode; this does need to be performed by the bridging layer)
* There is certainly room for debate with where the userfield, accountcode, and amaflags should live. Currently that is CDR, channel, channel. The channel userfield is generally unused. These could all be combined; however, that forces some parity between CEL userfield and CDR userfield that didn't used to exist.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A lot of unit testing.
This patch allows the existing Test Suite tests to be updated to cover CDRs in Asterisk 12 as well.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>team/group/bridge_construction/CHANGES <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/UPGRADE.txt <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/addons/cdr_mysql.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/addons/chan_ooh323.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_authenticate.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_cdr.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_dial.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_disa.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_dumpchan.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_followme.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_forkcdr.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_osplookup.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/apps/app_queue.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/bridges/bridge_softmix.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_adaptive_odbc.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_csv.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_custom.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_manager.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_odbc.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_pgsql.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_radius.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_syslog.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cdr/cdr_tds.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cel/cel_manager.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cel/cel_radius.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/cel/cel_tds.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_agent.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_dahdi.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_h323.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_iax2.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_local.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_mgcp.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_sip.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_skinny.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/channels/chan_unistim.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/funcs/func_callerid.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/funcs/func_cdr.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/funcs/func_channel.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/cdr.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/cel.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/channel.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/config_options.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/stasis_channels.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/strings.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/test.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/include/asterisk/time.h <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/asterisk.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/bridging.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/cdr.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/cel.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/channel.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/channel_internal_api.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/cli.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/config_options.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/features.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/manager.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/manager_channels.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/pbx.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/stasis_channels.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/test.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/main/utils.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/res/res_agi.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/res/res_config_sqlite.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/res/res_monitor.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/res/res_stasis.c <span style="color: grey">(387018)</span></li>
<li>team/group/bridge_construction/tests/test_cdr.c <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2462/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>