[asterisk-dev] [Code Review] 3490: Testsuite: Ensure that repeated device states and presence states behave as expected

[Matt Jordan]

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

Ship it!


Ship It!

- Matt Jordan


On April 29, 2014, 9:20 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3490/
> -----------------------------------------------------------
> 
> (Updated April 29, 2014, 9:20 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23672
>     https://issues.asterisk.org/jira/browse/ASTERISK-23672
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml 5004 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/sipp/subscribe.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/repeat_presence_state.py PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/sipp/subscribe.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/sipp/subscribe.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/repeat_device_state.py PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/extensions.conf PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3490/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140429/46f86f55/attachment-0001.html>