<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/3628/">https://reviewboard.asterisk.org/r/3628/</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.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated June 19, 2014, 8:29 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">Addressed Matt's review feedback.</pre>
</td>
</tr>
</table>
<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-23865">ASTERISK-23865</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 is step one towards implementing resource lists in Asterisk. Once resource list subscriptions are implemented, an ast_sip_subscription may not directly correspond to an underly pjsip_evsub structure. Currently, subscription handlers will gladly try to reach underneath the ast_sip_subscription to get at the PJSIP-specific structures. This set of changes seeks to abstract the pubsub API further so that subscription handlers do not have any interaction with PJSIP.
The changes outlined at https://wiki.asterisk.org/wiki/display/AST/PJSIP+Subscription+Abstraction+Plan provide a fairly accurate overview of what this review request entails. Some differences are:
* I was not able to completely separate subscribers and notifiers into separate structures. Instead, they are now separate sub-structures within the ast_sip_subscription_handler struct. The reason for this is that PJSIP is not flexible enough to register separate entities for clients and servers.
* The ast_sip_subscription_accept(), ast_sip_subscription_reject(), and ast_sip_subscription_terminate() calls ended up not being needed.
* All functions on the page that have a body parameter as a const char * are implemented to use a pjsip_msg_body instead. This is a case where the underlying PJSIP structure does not need to be abstracted, and having a parsed structure rather than a text blob will benefit everyone.
* One thing that is not on that page, but that you will find in this review, is that we now store the SUBSCRIBE request that starts a subscription on the subscription dialog. This allows subscription handlers to be able to retrieve the values of certain headers from the SUBSCRIBE if they wish. This is necessary for the exten_state handler currently since it wants the content of the User-Agent header.</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;">Since no new functionality has been added, I just ran the current gamut of testsuite tests (tests/channels/pjsip/subscriptions). They all pass.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/res/res_pjsip_xpidf_body_generator.c <span style="color: grey">(416761)</span></li>
<li>/trunk/res/res_pjsip_pubsub.exports.in <span style="color: grey">(416761)</span></li>
<li>/trunk/res/res_pjsip_pubsub.c <span style="color: grey">(416761)</span></li>
<li>/trunk/res/res_pjsip_pidf_body_generator.c <span style="color: grey">(416761)</span></li>
<li>/trunk/res/res_pjsip_mwi.c <span style="color: grey">(416761)</span></li>
<li>/trunk/res/res_pjsip_exten_state.c <span style="color: grey">(416761)</span></li>
<li>/trunk/include/asterisk/res_pjsip_pubsub.h <span style="color: grey">(416761)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3628/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>