[asterisk-dev] [Code Review] 3361: rest_api/bridges/subscription: Update test to accomodate r410528
Jonathan Rose
reviewboard at asterisk.org
Fri Mar 21 14:13:14 CDT 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3361/#review11317
-----------------------------------------------------------
"The last verification broke because we now ensure that
applications who have channels in a bridge are now subscribed
implicitly to the bridge while their channel is in it. You can
now no longer unsubscribe from the implicit subscription
created by your channel: you may decrement the subscription
count, but it won't be sufficient to nuke out the subscription
that ARI has created for you.
I actually think this behaviour change is a good thing: If you
have a channel in an application, and it is doing "things",
you should always be notified of the "things" it is doing.
Even if you claim you don't want to know - otherwise, how do
you know when you can DELETE the channel? Or do something
else crazy to it?"
----------------------------------------------------
So one thing I think we need to look into is protecting these
types of subscriptions from being destroyed by ARI unsubscribe
commands. It seems to me like delete subscription should only
work on subscriptions that were explicitly created by ARI. Of
course, internally right now it's just stored as a sort of
reference count of 'interested' members, but we could fairly
easily turn that into two distinct types of interested parties
so that you could 'delete' the subscriptions you made without
being able to take out any arbitrary subscription.
- Jonathan Rose
On March 14, 2014, 4:23 p.m., Matt Jordan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3361/
> -----------------------------------------------------------
>
> (Updated March 14, 2014, 4:23 p.m.)
>
>
> Review request for Asterisk Developers, David Lee, Joshua Colp, and kmoore.
>
>
> Repository: testsuite
>
>
> Description
> -------
>
> The change in r410528 (https://reviewboard.asterisk.org/r/3336/) introduced a subtle behaviour change that broke the rest_api/bridges/subscription test. Previously, the test did the following:
>
> * Create a Stasis application 'testsuite'
> * Create a Stasis application 'bridge-watching-app'
> * POST a channel to application 'testsuite'
> * POST a bridge
> * Subscribe application 'bridge-watching-app' to the bridge
> * Add the previously posted channel to the bridge
> ** Verify that the app 'testsuite' sees the bridge update (as its channel is in the bridge)
> ** Verify that the app 'bridge-watching-app' sees the bridge update (as it was subscribed)
> * Unsubscribe the app 'testsuite' from the bridge
> * Remove the channel from the bridge
> ** Verify that the app 'bridge-watching-app' see the bridge update
> ** Verify that the app 'testsuite' does NOT see the bridge update <------------ THIS IS NOW BROKEN
>
> The last verification broke because we now ensure that applications who have channels in a bridge are now subscribed implicitly to the bridge while their channel is in it. You can now no longer unsubscribe from the implicit subscription created by your channel: you may decrement the subscription count, but it won't be sufficient to nuke out the subscription that ARI has created for you.
>
> I actually think this behaviour change is a good thing: If you have a channel in an application, and it is doing "things", you should always be notified of the "things" it is doing. Even if you claim you don't want to know - otherwise, how do you know when you can DELETE the channel? Or do something else crazy to it?
>
> This test has now been updated so that the thing unsubscribed is the 'bridge-watching-app'. This still verifies subscribes/unsubscribes - but it does so where the entity subscribing for the bridge updates never had a 'stake' in the state of the bridge in the first place.
>
>
> Diffs
> -----
>
> /asterisk/trunk/tests/rest_api/applications/subscribe-bridge/test-config.yaml 4844
> /asterisk/trunk/tests/rest_api/applications/subscribe-bridge/subscribe_bridge.py 4844
>
> Diff: https://reviewboard.asterisk.org/r/3361/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Matt Jordan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140321/6a534de2/attachment.html>
More information about the asterisk-dev
mailing list