[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] [Code Review] 3726: ari: Add message technology agnostic out of call text messagi
From: "opticron" <reviewboard () asterisk ! org>
Date: 2014-07-31 19:43:58
Message-ID: 20140731194358.17335.77781 () sonic ! digium ! api
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3726/#review12949
-----------------------------------------------------------
Ship it!
Minor nitpicks below.
/branches/12/main/message.c
<https://reviewboard.asterisk.org/r/3726/#comment23331>
Red blob.
/branches/12/tests/test_message.c
<https://reviewboard.asterisk.org/r/3726/#comment23332>
s/invalid/unknown/
/branches/12/tests/test_message.c
<https://reviewboard.asterisk.org/r/3726/#comment23333>
s/invalid/unkown/
- opticron
On July 31, 2014, 2:01 p.m., Matt Jordan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3726/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 2:01 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-23692 and ASTERISK-23969
> https://issues.asterisk.org/jira/browse/ASTERISK-23692
> https://issues.asterisk.org/jira/browse/ASTERISK-23969
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> This patch adds the ability to send and receive text messages from various \
> technology stacks in Asterisk through ARI. This includes chan_sip (sip), \
> res_pjsip_messaging (pjsip), and res_xmpp (xmpp).
> The following would send the message "Hello there" to PJSIP endpoint alice with a \
> display URI of sip:asterisk@mycooldomain.org:
> ari/endpoints/sendMessage?to=pjsip:alice&from=sip:asterisk@mycooldomain.org&body=Hello+There
>
> This is equivalent to the following as well:
>
> ari/endpoints/PJSIP/alice/sendMessage?from=sip:asterisk@mycooldomain.org&body=Hello+There
>
> Both forms are available for message technologies that allow for arbitrary \
> destinations, such as chan_sip.
> Inbound messages can now be received over ARI. An ARI application that subscribes \
> to endpoints will receive messages from those endpoints:
> {
> "type": "TextMessageReceived",
> "timestamp": "2014-07-12T22:53:13.494-0500",
> "endpoint": {
> "technology": "PJSIP",
> "resource": "alice",
> "state": "online",
> "channel_ids": []
> },
> "message": {
> "from": "\"alice\" <sip:alice@127.0.0.1>",
> "to": "pjsip:asterisk@127.0.0.1",
> "body": "Watson, come here.",
> "variables": []
> },
> "application": "testsuite"
> }
>
> A few interesting things you could do with this:
> (1) Build your own XMPP to SIP gateway (without ever touching dialplan)
> (2) Make a conferencing application with built-in text messaging (speech to text \
> would be fun with this... probably should write that too) (3) WebRTC! SIP stacks in \
> the browser can send MESSAGE requests. Why limit yourself to just making calls when \
> you can send arbitrary messages to a communications application? (Note: if you \
> can't mention WebRTC in a release, you're not trying very hard)
> The above was made possible due to some rather major changes in the message core. \
> This includes (but is not limited to):
> - Users of the message API can now register message handlers. A handler has two \
> callbacks: one to determine if the handler has a destination for the message, and \
> another to handle it.
> - All dialplan functionality of handling a message was moved into a message handler \
> provided by the message API.
> - Messages can now have the technology/endpoint associated with them. Various other \
> properties are also now more easily accessible.
> - A number of ao2 containers that weren't really needed were replaced with vectors. \
> Iteration over ao2_containers is expensive and pointless when the lifetime of \
> things is well defined and the number of things is very small.
> res_stasis now has a new file that makes up its structure, messaging. The messaging \
> functionality implements a message handler, and passes received messages that match \
> an interested endpoint over to the app for processing.
> Note that inadvertently while testing this, I reproduced ASTERISK-23969. \
> res_pjsip_messaging was incorrectly parsing out the 'to' field, such that arbitrary \
> SIP URIs mangled the endpoint lookup. This patch includes the fix for that as well. \
>
> Diffs
> -----
>
> /branches/12/tests/test_message.c PRE-CREATION
> /branches/12/rest-api/api-docs/events.json 419870
> /branches/12/rest-api/api-docs/endpoints.json 419870
> /branches/12/res/stasis/app.c 419870
> /branches/12/res/res_xmpp.c 419870
> /branches/12/res/res_stasis.c 419870
> /branches/12/res/res_pjsip_messaging.c 419870
> /branches/12/res/res_ari_endpoints.c 419870
> /branches/12/res/ari/resource_endpoints.c 419870
> /branches/12/res/ari/resource_endpoints.h 419870
> /branches/12/res/ari/resource_channels.c 419870
> /branches/12/res/ari/ari_model_validators.c 419870
> /branches/12/res/ari/ari_model_validators.h 419870
> /branches/12/main/message.c 419870
> /branches/12/main/json.c 419870
> /branches/12/include/asterisk/vector.h 419870
> /branches/12/include/asterisk/message.h 419870
> /branches/12/include/asterisk/manager.h 419870
> /branches/12/include/asterisk/json.h 419870
> /branches/12/channels/chan_sip.c 419870
>
> Diff: https://reviewboard.asterisk.org/r/3726/diff/
>
>
> Testing
> -------
>
> Unit tests were added for the message core to make sure dialplan still worked.
>
> Basic nominal tests have been added for the Asterisk Test Suite, and are up for \
> review at https://reviewboard.asterisk.org/r/3864/
>
> Thanks,
>
> Matt Jordan
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3726/">https://reviewboard.asterisk.org/r/3726/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Minor nitpicks \
below.</pre> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://reviewboard.asterisk.org/r/3726/diff/3/?file=65958#file65958line944" \
style="color: black; font-weight: bold; text-decoration: \
underline;">/branches/12/main/message.c</a> <span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">883</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="ew"><span class="tb"> </span></span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Red \
blob.</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://reviewboard.asterisk.org/r/3726/diff/3/?file=65971#file65971line316" \
style="color: black; font-weight: bold; text-decoration: \
underline;">/branches/12/tests/test_message.c</a> <span style="font-weight: \
normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">316</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span \
class="s">"</span><span class="se">\t</span><span class="s">An invalid message \
technology cannot be unregistered</span><span class="se">\n</span><span \
class="s">"</span><span class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">s/invalid/unknown/</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://reviewboard.asterisk.org/r/3726/diff/3/?file=65971#file65971line350" \
style="color: black; font-weight: bold; text-decoration: \
underline;">/branches/12/tests/test_message.c</a> <span style="font-weight: \
normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">350</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span \
class="s">"</span><span class="se">\t</span><span class="s">An invalid message \
handler cannot be unregistered</span><span class="se">\n</span><span \
class="s">"</span><span class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">s/invalid/unkown/</pre> </div>
<br />
<p>- opticron</p>
<br />
<p>On July 31st, 2014, 2:01 p.m. CDT, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated July 31, 2014, 2:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23692">ASTERISK-23692</a>, \
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23969">ASTERISK-23969</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">This patch adds the ability to send and receive text messages from \
various technology stacks in Asterisk through ARI. This includes chan_sip (sip), \
res_pjsip_messaging (pjsip), and res_xmpp (xmpp).
The following would send the message "Hello there" to PJSIP endpoint alice \
with a display URI of sip:asterisk@mycooldomain.org:
ari/endpoints/sendMessage?to=pjsip:alice&from=sip:asterisk@mycooldomain.org&body=Hello+There
This is equivalent to the following as well:
ari/endpoints/PJSIP/alice/sendMessage?from=sip:asterisk@mycooldomain.org&body=Hello+There
Both forms are available for message technologies that allow for arbitrary \
destinations, such as chan_sip.
Inbound messages can now be received over ARI. An ARI application that subscribes to \
endpoints will receive messages from those endpoints:
{
"type": "TextMessageReceived",
"timestamp": "2014-07-12T22:53:13.494-0500",
"endpoint": {
"technology": "PJSIP",
"resource": "alice",
"state": "online",
"channel_ids": []
},
"message": {
"from": "\"alice\" <sip:alice@127.0.0.1>",
"to": "pjsip:asterisk@127.0.0.1",
"body": "Watson, come here.",
"variables": []
},
"application": "testsuite"
}
A few interesting things you could do with this:
(1) Build your own XMPP to SIP gateway (without ever touching dialplan)
(2) Make a conferencing application with built-in text messaging (speech to text \
would be fun with this... probably should write that too) (3) WebRTC! SIP stacks in \
the browser can send MESSAGE requests. Why limit yourself to just making calls when \
you can send arbitrary messages to a communications application? (Note: if you \
can't mention WebRTC in a release, you're not trying very hard)
The above was made possible due to some rather major changes in the message core. \
This includes (but is not limited to):
- Users of the message API can now register message handlers. A handler has two \
callbacks: one to determine if the handler has a destination for the message, and \
another to handle it.
- All dialplan functionality of handling a message was moved into a message handler \
provided by the message API.
- Messages can now have the technology/endpoint associated with them. Various other \
properties are also now more easily accessible.
- A number of ao2 containers that weren't really needed were replaced with \
vectors. Iteration over ao2_containers is expensive and pointless when the lifetime \
of things is well defined and the number of things is very small.
res_stasis now has a new file that makes up its structure, messaging. The messaging \
functionality implements a message handler, and passes received messages that match \
an interested endpoint over to the app for processing.
Note that inadvertently while testing this, I reproduced ASTERISK-23969. \
res_pjsip_messaging was incorrectly parsing out the 'to' field, such that \
arbitrary SIP URIs mangled the endpoint lookup. This patch includes the fix for that \
as well.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Unit tests were added for the message core to make sure dialplan still \
worked.
Basic nominal tests have been added for the Asterisk Test Suite, and are up for \
review at https://reviewboard.asterisk.org/r/3864/</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/tests/test_message.c <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>/branches/12/rest-api/api-docs/events.json <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/rest-api/api-docs/endpoints.json <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/stasis/app.c <span style="color: grey">(419870)</span></li>
<li>/branches/12/res/res_xmpp.c <span style="color: grey">(419870)</span></li>
<li>/branches/12/res/res_stasis.c <span style="color: grey">(419870)</span></li>
<li>/branches/12/res/res_pjsip_messaging.c <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/res_ari_endpoints.c <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/ari/resource_endpoints.c <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/ari/resource_endpoints.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/ari/resource_channels.c <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/ari/ari_model_validators.c <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/res/ari/ari_model_validators.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/main/message.c <span style="color: grey">(419870)</span></li>
<li>/branches/12/main/json.c <span style="color: grey">(419870)</span></li>
<li>/branches/12/include/asterisk/vector.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/include/asterisk/message.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/include/asterisk/manager.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/include/asterisk/json.h <span style="color: \
grey">(419870)</span></li>
<li>/branches/12/channels/chan_sip.c <span style="color: grey">(419870)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3726/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic