[asterisk-dev] [Code Review] 2862: ARI: Add subscription support

[Matt Jordan]

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



/branches/12/include/asterisk/endpoints.h
<https://reviewboard.asterisk.org/r/2862/#comment18956>

    zero?



/branches/12/include/asterisk/stasis_app.h
<https://reviewboard.asterisk.org/r/2862/#comment18957>

    Doxygen this one too



/branches/12/main/endpoints.c
<https://reviewboard.asterisk.org/r/2862/#comment18958>

    See Richard's template for comparison callbacks:
    
    https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=25919686



/branches/12/res/ari/resource_applications.c
<https://reviewboard.asterisk.org/r/2862/#comment18962>

    I'd check that event_source_count isn't negative as well, since it is used for memory allocation purposes. Passing -1 here will get interpreted as something rather large.



/branches/12/res/ari/resource_applications.c
<https://reviewboard.asterisk.org/r/2862/#comment18963>

    Do you want to validate args->application_name here as well?
    
    if (ast_strlen_zero(args->application_name)) {
        // send 400
    }
    
    That way stasis_app_subscribe doesn't have to be quite as paranoid.



/branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/2862/#comment18964>

    A WARNING message here may be appropriate.



/branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/2862/#comment18965>

    I'd promote this to a useful WARNING message.



/branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/2862/#comment18966>

    A WARNING here as well would be nice.


- Matt Jordan


On Sept. 17, 2013, 4:08 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2862/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2013, 4:08 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22451
>     https://issues.asterisk.org/jira/browse/ASTERISK-22451
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds an /applications API to ARI, allowing explicit management of
> Stasis applications.
> 
>  * GET /applications - list current applications
>  * GET /applications/{applicationName} - get details of a specific application
>  * POST /applications/{applicationName}/subscription - explicitly subscribe to
>    a channel, bridge or endpoint
>  * DELETE /applications/{applicationName}/subscription - explicitly unsubscribe
>    from a channel, bridge or endpoint
> 
> Subscriptions work by a reference counting mechanism: if you subscript to an
> event source X number of times, you must unsubscribe X number of times to stop
> receiveing events for that event source.
> 
> 
> Diffs
> -----
> 
>   /branches/12/include/asterisk/endpoints.h 399236 
>   /branches/12/include/asterisk/stasis_app.h 399236 
>   /branches/12/main/asterisk.c 399236 
>   /branches/12/main/endpoints.c 399236 
>   /branches/12/main/json.c 399236 
>   /branches/12/res/ari.make 399236 
>   /branches/12/res/ari/ari_model_validators.h 399236 
>   /branches/12/res/ari/ari_model_validators.c 399236 
>   /branches/12/res/ari/resource_applications.h PRE-CREATION 
>   /branches/12/res/ari/resource_applications.c PRE-CREATION 
>   /branches/12/res/ari/resource_endpoints.h 399236 
>   /branches/12/res/res_ari_applications.c PRE-CREATION 
>   /branches/12/res/res_ari_model.c 399236 
>   /branches/12/res/res_stasis.c 399236 
>   /branches/12/res/stasis/app.h 399236 
>   /branches/12/res/stasis/app.c 399236 
>   /branches/12/rest-api-templates/ari_model_validators.h.mustache 399236 
>   /branches/12/rest-api/api-docs/applications.json PRE-CREATION 
>   /branches/12/rest-api/api-docs/endpoints.json 399236 
>   /branches/12/rest-api/api-docs/events.json 399236 
>   /branches/12/rest-api/resources.json 399236 
> 
> Diff: https://reviewboard.asterisk.org/r/2862/diff/
> 
> 
> Testing
> -------
> 
> Testsuite tests (https://reviewboard.asterisk.org/r/2863/)
> 
> I hit a roadblock testing the endpoint subscription code, so that was tested
> manually using wscat and Swagger-UI.
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130926/15b89d71/attachment-0002.html>