[asterisk-dev] [Code Review] 2639: Update events to use Swagger 1.2 subtyping, and related aftermath

opticron reviewboard at asterisk.org
Fri Jun 21 18:01:57 CDT 2013 

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


Overall, this looks great!


/team/dlee/ari-url-shuffle/main/stasis_channels.c
<https://reviewboard.asterisk.org/r/2639/#comment17590>

    It would be  better to use the O (capital o) specifier instead of using ast_json_ref and o.



/team/dlee/ari-url-shuffle/main/stasis_channels.c
<https://reviewboard.asterisk.org/r/2639/#comment17591>

    Idem.



/team/dlee/ari-url-shuffle/rest-api-templates/ari_model.c.mustache
<https://reviewboard.asterisk.org/r/2639/#comment17592>

    This is an error, and so should return 0 or set res to 0.



/team/dlee/ari-url-shuffle/rest-api-templates/stasis_json_resource.h.mustache
<https://reviewboard.asterisk.org/r/2639/#comment17593>

    This mustache template should be removed as well.



/team/dlee/ari-url-shuffle/rest-api/api-docs/channels.json
<https://reviewboard.asterisk.org/r/2639/#comment17594>

    s/The/the/


- opticron


On June 21, 2013, 4:21 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2639/
> -----------------------------------------------------------
> 
> (Updated June 21, 2013, 4:21 p.m.)
> 
> 
> Review request for Asterisk Developers and opticron.
> 
> 
> Bugs: ASTERISK-21885
>     https://issues.asterisk.org/jira/browse/ASTERISK-21885
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch started with the simple idea of changing the /events data
> model to be more sane. The original model would send out events like:
> 
>     { "stasis_start": { "args": [], "channel": { ... } } }
> 
> The event discriminator was the field name instead of being a value in
> the object, due to limitations in how Swagger 1.1 could model objects.
> While technically sufficient in communicating event information, it was
> really difficult to deal with in terms of client side JSON handling.
> 
> This patch takes advantage of a proposed extension[1] to Swagger which
> allows type variance through the use of a discriminator field. This had
> a domino effect that made this a surprisingly large patch.
> 
>  [1]: https://groups.google.com/d/msg/wordnik-api/EC3rGajE0os/ey_5dBI_jWcJ
> 
> In changing the models, I also had to change the swagger_model.py
> processor so it can handle the type discriminator and subtyping. I took
> that a big step forward, and using that information to generate an
> ari_model module, which can validate a JSON object against the Swagger
> model.
> 
> The REST and WebSocket generators were changed to take advantage of the
> validators. If compiled with AST_DEVMODE enabled, JSON objects that
> don't match their corresponding models will not be sent out. For REST
> API calls, a 500 Internal Server response is sent. For WebSockets, the
> invalid JSON message is replaced with an error message.
> 
> Since this took over about half of the job of the existing JSON
> generators, and the .to_json virtual function on messages took over the
> other half, I reluctantly removed the generators.
> 
> The validators turned up all sorts of errors and inconsistencies in our
> data models, and the code. These were cleaned up, with checks in the
> code generator avoid some of the consistency problems in the future.
> 
>  * The model for a channel snapshot was trimmed down to match the
>    information sent via AMI. Many of the field being sent were not
>    useful in the general case.
> 
>  * The model for a bridge snapshot was updated to be more consistent
>    with the other ARI models.
> 
> Another impact of introducing subtyping was that the swagger-codegen
> documentation generator was insufficient (at least until it catches up
> with Swagger 1.2). I wanted it to be easier to generate docs for the API
> anyways, so I ported the wiki pages to use the Asterisk Swagger
> generator. In the process, I was able to clean up many of the model
> links, which would occasionally give inconsistent results on the wiki. I
> also added error responses to the wiki docs, making the wiki
> documentation more complete.
> 
> Finally, since Stasis-HTTP will now be named Asterisk REST Interface
> (ARI), any new functions and files I created carry the ari_ prefix. I
> changed a few stasis_http references to ari where it was non-intrusive
> and made sense.
> 
> This patch relies on the work in /team/dlee/ari-url-shuffle[2].
> 
>  [2]: https://reviewboard.asterisk.org/r/2621/
> 
> 
> Diffs
> -----
> 
>   /team/dlee/ari-url-shuffle/Makefile 392488 
>   /team/dlee/ari-url-shuffle/include/asterisk/json.h 392488 
>   /team/dlee/ari-url-shuffle/include/asterisk/stasis_http.h 392488 
>   /team/dlee/ari-url-shuffle/main/json.c 392488 
>   /team/dlee/ari-url-shuffle/main/stasis_bridging.c 392488 
>   /team/dlee/ari-url-shuffle/main/stasis_channels.c 392488 
>   /team/dlee/ari-url-shuffle/main/stasis_endpoints.c 392488 
>   /team/dlee/ari-url-shuffle/res/Makefile 392488 
>   /team/dlee/ari-url-shuffle/res/res_ari_model.c PRE-CREATION 
>   /team/dlee/ari-url-shuffle/res/res_ari_model.exports.in PRE-CREATION 
>   /team/dlee/ari-url-shuffle/res/res_stasis.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_asterisk.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_bridges.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_channels.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_endpoints.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_events.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_playback.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_recordings.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_http_sounds.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_asterisk.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_asterisk.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_bridges.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_bridges.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_channels.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_channels.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_endpoints.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_endpoints.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_events.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_events.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_playback.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_playback.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_recordings.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_recordings.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_sounds.c 392488 
>   /team/dlee/ari-url-shuffle/res/res_stasis_json_sounds.exports.in 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_http/ari_model.h PRE-CREATION 
>   /team/dlee/ari-url-shuffle/res/stasis_http/ari_model.c PRE-CREATION 
>   /team/dlee/ari-url-shuffle/res/stasis_http/ari_websockets.c 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_http/resource_recordings.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_http/resource_recordings.c 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_asterisk.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_bridges.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_channels.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_endpoints.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_events.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_playback.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_recordings.h 392488 
>   /team/dlee/ari-url-shuffle/res/stasis_json/resource_sounds.h 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/api.wiki.mustache PRE-CREATION 
>   /team/dlee/ari-url-shuffle/rest-api-templates/ari_model.c.mustache PRE-CREATION 
>   /team/dlee/ari-url-shuffle/rest-api-templates/ari_model.h.mustache PRE-CREATION 
>   /team/dlee/ari-url-shuffle/rest-api-templates/asterisk_processor.py 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/make_ari_stubs.py PRE-CREATION 
>   /team/dlee/ari-url-shuffle/rest-api-templates/make_stasis_http_stubs.py 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/models.wiki.mustache PRE-CREATION 
>   /team/dlee/ari-url-shuffle/rest-api-templates/res_stasis_http_resource.c.mustache 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/res_stasis_json_resource.c.mustache 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/res_stasis_json_resource.exports.mustache 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/stasis_json_resource.h.mustache 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/swagger_model.py 392488 
>   /team/dlee/ari-url-shuffle/rest-api-templates/transform.py 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/asterisk.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/bridges.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/channels.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/endpoints.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/events.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/playback.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/recordings.json 392488 
>   /team/dlee/ari-url-shuffle/rest-api/api-docs/sounds.json 392488 
>   /team/dlee/ari-url-shuffle/tests/test_ari_model.c PRE-CREATION 
>   /team/dlee/ari-url-shuffle/tests/test_res_stasis.c 392488 
>   /team/dlee/ari-url-shuffle/tests/test_stasis_channels.c 392488 
> 
> Diff: https://reviewboard.asterisk.org/r/2639/diff/
> 
> 
> Testing
> -------
> 
> Unit tests for validator.
> 
> Created some channels, put them in Stasis(), using the ARI /bridges API to bridge them together. And it worked.
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130621/dd985099/attachment-0001.htm>

More information about the asterisk-dev mailing list