[asterisk-dev] [Code Review] 3494: ARI: Add the ability to raise an arbitrary User Event from the Applications resource

[Scott Griepentrog]

> On May 2, 2014, 8:37 a.m., Matt Jordan wrote:
> > /branches/12/main/stasis_user.c, lines 64-74
> > <https://reviewboard.asterisk.org/r/3494/diff/1/?file=58110#file58110line64>
> >
> >     Wrapping the snapshots is fine, but I think all you'll need is the type stored as an enum. That will allow something converting the snapshots into another format to know how to convert them.

The name is used to provide prefixing for multiple objects.  For example, if you add two channels, you get Channel: for the first and 2Channel: for the second.  This also allows the multi object mechanism to be a drop in replacement for the multi channel, where channels are explicitly named as they are added (FirstChannel:, SecondChannel).


> On May 2, 2014, 8:37 a.m., Matt Jordan wrote:
> > /branches/12/main/stasis_user.c, lines 110-114
> > <https://reviewboard.asterisk.org/r/3494/diff/1/?file=58110#file58110line110>
> >
> >     I'd go with a different approach for storing the various snapshots. An ao2 container, while useful if you want to look up a single particular object, is overkill when the pattern of usage is to iterate over the entire container.
> >     
> >     Generally, payloads are always consumed in whole - it's rare for anyone to only want a single part of the payload. This is particularly true for payloads with multiple objects: you generally will convert all of the objects in it to JSON, or AMI kvp, or something else.
> >     
> >     I'd just store the snapshots in a wrapped struct (like you have) that is a tuple of (type, snapshot) in a list or a vector. That has the added benefit of not requiring a custom hash function.

I don't see a way to do that without loosing the naming mechanism, which I currently believe is a necessary and desired feature.  I could still replace the hash with a list though, as it's a small quantity and currently is only iterated.


- Scott


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


On April 29, 2014, 11:15 a.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3494/
> -----------------------------------------------------------
> 
> (Updated April 29, 2014, 11:15 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22697
>     https://issues.asterisk.org/jira/browse/ASTERISK-22697
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This changes the implementation of UserEvent stasis messaging and adds additional features:
> 
> 1) Existing channel_user_event stasis messaging replaced with new multi_object_blob in stasis_user.c
> 2) Multi object blob provides ability to signal blob + any number of channel, bridge, or endpoint snapshots
> 3) Changed existing Userevent() app to use new multi object blob - but still publishes to channel
> 4) Added ARI implementation of userevent as /ari/applications/{applicationName}/user/{eventName}
> 5) ARI generated userevent messages are published to application, and also to AMI if channel present
> 
> 
> Diffs
> -----
> 
>   /branches/12/rest-api/api-docs/events.json 413115 
>   /branches/12/rest-api/api-docs/applications.json 413115 
>   /branches/12/res/stasis/app.c 413115 
>   /branches/12/res/res_stasis.c 413115 
>   /branches/12/res/res_ari_applications.c 413115 
>   /branches/12/res/ari/resource_applications.c 413115 
>   /branches/12/res/ari/resource_applications.h 413115 
>   /branches/12/res/ari/ari_model_validators.c 413115 
>   /branches/12/res/ari/ari_model_validators.h 413115 
>   /branches/12/main/stasis_user.c PRE-CREATION 
>   /branches/12/main/stasis_channels.c 413115 
>   /branches/12/main/manager_channels.c 413115 
>   /branches/12/main/asterisk.c 413115 
>   /branches/12/include/asterisk/stasis_user.h PRE-CREATION 
>   /branches/12/include/asterisk/stasis_channels.h 413115 
>   /branches/12/include/asterisk/stasis_app.h 413115 
>   /branches/12/apps/app_userevent.c 413115 
> 
> Diff: https://reviewboard.asterisk.org/r/3494/diff/
> 
> 
> Testing
> -------
> 
> Tested with valgrind to insure no invalid references.  Some leaks found which have been attributed to other code to be fixed separately.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

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