----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3117/#review10569 -----------------------------------------------------------
/branches/12/res/ari/resource_mailboxes.c <https://reviewboard.asterisk.org/r/3117/#comment20031> It's a bit pedantic, but if I were faced with an error response that said "Mailbox is missing", I would interpret that to mean that I omitted the required mailbox in my HTTP request, not that the mailbox did not exist on the server. The fact that I'm getting a 404 should be a hint as to what the actual problem is, but I still think the response text could be changed to be more clear. Something like "Mailbox does not exist" would work better. /branches/12/res/ari/resource_mailboxes.c <https://reviewboard.asterisk.org/r/3117/#comment20032> Same "Mailbox is missing" nit here. /branches/12/res/res_stasis_mailbox.c <https://reviewboard.asterisk.org/r/3117/#comment20033> mailbox_to_json() may return NULL if there was an error creating the JSON. Also, ast_json_array_append() can return an error. With this sort of operation, I think you should either return all or nothing. If an error occurs while creating the array, you should free everything and return NULL. - Mark Michelson On Jan. 9, 2014, 11:38 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3117/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2014, 11:38 p.m.) > > > Review request for Asterisk Developers, Matt Jordan and Mark Michelson. > > > Repository: Asterisk > > > Description > ------- > > This patch adds the ability to interface with res_external_mwi via AMI. The > following commands are implemented: > > PUT mailboxes/mailboxName > modifies mailbox state and implicitly creates new mailboxes > > GET mailboxes/mailboxName > retrieves a JSON representation of a single mailbox if it exists > > GET mailboxes > retrieves a JSON array of all mailboxes > > DELETE mailbox/mailboxName > deletes a mailbox > > Thanks to Richard's external MWI work, this is actually a very simple system. > > > Diffs > ----- > > /branches/12/rest-api/resources.json 405265 > /branches/12/rest-api/api-docs/mailboxes.json PRE-CREATION > /branches/12/res/res_stasis_mailbox.exports.in PRE-CREATION > /branches/12/res/res_stasis_mailbox.c PRE-CREATION > /branches/12/res/res_ari_mailboxes.c PRE-CREATION > /branches/12/res/ari/resource_mailboxes.c PRE-CREATION > /branches/12/res/ari/resource_mailboxes.h PRE-CREATION > /branches/12/res/ari/ari_model_validators.c 405265 > /branches/12/res/ari/ari_model_validators.h 405265 > /branches/12/res/ari.make 405265 > /branches/12/include/asterisk/stasis_app_mailbox.h PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3117/diff/ > > > Testing > ------- > > Tested unloads and reloads of the res_stasis_mailboxes module with > res_mwi_external loaded and unloaded > Tested how commands respond from resource_mailboxes when res_stasis_mailboxes > isn't loaded > Tested each of the commands with numerous parameters > Created testsuite test in https://reviewboard.asterisk.org/r/3118/ which > confirms basic operation of all the new ARI functions. > > > Thanks, > > Jonathan Rose > >
-- _____________________________________________________________________ -- 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