----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4316/#review14263 -----------------------------------------------------------
Ship it! The code changes look good to me. The only additional comments I have are 1) CHANGES should be updated to indicate that the Transfer() dialplan function for PJSIP can now refer to endpoints. CHANGES should also be updated to indicate the addition of the /redirect ARI resource. 2) Since a new backwards-compatible ARI resource was added, the ARI minor version needs to be bumped. - Mark Michelson On Jan. 21, 2015, 3:39 a.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4316/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 3:39 a.m.) > > > Review request for Asterisk Developers and Joshua Colp. > > > Bugs: ASTERISK-24015 and ASTERISK-24703 > https://issues.asterisk.org/jira/browse/ASTERISK-24015 > https://issues.asterisk.org/jira/browse/ASTERISK-24703 > > > Repository: Asterisk > > > Description > ------- > > This patch adds a new feature to ARI to redirect a channel to another server, > and fixes a few bugs in PJSIP's handling of the Transfer dialplan > application/ARI redirect capability. > > *New Feature* > A new operation has been added to the ARI channels resource, redirect. With > this, a channel in a Stasis application can be redirected to another endpoint > of the same underlying channel technology. > > - Preemptive question: why 'redirect', and not 'transfer'? Mostly because > 'transfer' was always kind of a bad name. If the channel isn't answered, we > aren't transferring, we're forwarding. If it is answered, the type of > transfer being performed is somewhat vague - is it blind? Is it attended? > 'redirect' - while also a slightly loaded term - is a bit more generic and > yet descriptive of what is happening: we're redirecting the channel to > somewhere else. Answered, not answered, it doesn't matter: your channel is no > good here! > > *Bug fixes* > In the process of writing this new feature, two bugs were fixed in the PJSIP > stack: > (1) The existing .transfer channel callback had the limitation that it could > only transfer channels to a SIP URI, i.e., you had to pass > 'PJSIP/sip:foo@my_provider.com' to the dialplan application. While this is > still supported, it is somewhat unintuitive - particularly in a world full of > endpoints. As such, we now also support specifying the PJSIP endpoint to > transfer to. > (2) res_pjsip_multihomed was, unfortunately, trying to 'help' a 302 redirect > by updating its Contact header. Alas, that resulted in the forwarding > destination set by the dialplan application/ARI resource/whatever being > rewritten with very incorrect information. Hence, we now don't bother > updating an outgoing response if it is a 302. Since this took a looong time > to find, some additional debug statements have been added to those modules > that update the Contact headers. > > > Diffs > ----- > > /branches/13/rest-api/api-docs/channels.json 430839 > /branches/13/res/stasis/control.c 430839 > /branches/13/res/res_pjsip_transport_websocket.c 430839 > /branches/13/res/res_pjsip_nat.c 430839 > /branches/13/res/res_pjsip_multihomed.c 430839 > /branches/13/res/res_ari_channels.c 430839 > /branches/13/res/ari/resource_channels.c 430839 > /branches/13/res/ari/resource_channels.h 430839 > /branches/13/include/asterisk/stasis_app.h 430839 > /branches/13/channels/chan_pjsip.c 430839 > > Diff: https://reviewboard.asterisk.org/r/4316/diff/ > > > Testing > ------- > > Tests were written both for the PJSIP stack as well as the new ARI operation. > See https://reviewboard.asterisk.org/r/4352. > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- 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
