<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/3490/">https://reviewboard.asterisk.org/r/3490/</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>
<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-23672">ASTERISK-23672</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
testsuite
</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;">In ASTERISK-23672, it was reported that when the presence state of an entity was changed such that the state was the same but the subtype or message differed from what was previously set, NOTIFYs were not sent to SIP subscribers. Looking at the code, it appeared that res_pjsip_exten_state was being overzealous in trying to filter out repeated state changes and that the core PBX code already performed the necessary filtering.
I made the following change to res_pjsip_exten_state.c, which I'm not placing in its own review due to its small size:
Index: res/res_pjsip_exten_state.c
===================================================================
--- res/res_pjsip_exten_state.c (revision 412578)
+++ res/res_pjsip_exten_state.c (working copy)
@@ -334,11 +334,6 @@
struct notify_task_data *task_data;
struct exten_state_subscription *exten_state_sub = data;
- if (exten_state_sub->last_exten_state == info->exten_state &&
- exten_state_sub->last_presence_state == info->presence_state) {
- return 0;
- }
-
if (!(task_data = alloc_notify_task_data(exten, exten_state_sub, info))) {
return -1;
}
I then created three testsuite tests to ensure that behavior is as we expect for it to be:
devstate_repeat: This relies on the device state for a custom device state to be "not in use" initially. A subscriber subscribes to a custom device state. We then set a device state change for the custom device state to be "not in use". Since there is no change in the device state, no NOTIFY should be sent to the subscriber.
presencestate_repeat: This sets an initial presence state for a CustomPresence entity. A SIP subscriber subscribes to the custom presence state. We then set the presence state to the exact same value again and ensure that no additional NOTIFYs are sent to the subscriber.
presencestate_repeat_okay: This sets an initial presence state for a CustomPresence entity. A SIP subscriber subscribes to the custom presence state. We then change the presence state twice. The first change keeps the same state and message and changes the subtype. The second change keeps the same state and subtype and changes the message. These should result in two additional NOTIFYs being sent to the subscriber.</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;">Prior to the diff mentioned in the description, devstate_repeat and presencestate_repeat would pass, but presencestate_repeat_okay would not. With the diff above applied, all three tests pass.</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>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml <span style="color: grey">(5004)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/sipp/subscribe.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/repeat_presence_state.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/pjsip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/sipp/subscribe.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/sipp/subscribe.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/repeat_device_state.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/pjsip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3490/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>