[asterisk-commits] dlee: trunk r397565 - in /trunk: include/asterisk/ res/ res/ari/ res/stasis/ ...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Aug 23 12:19:05 CDT 2013
Author: dlee
Date: Fri Aug 23 12:19:02 2013
New Revision: 397565
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=397565
Log:
ARI: Correct error codes for bridge operations
This patch adds error checking to ARI bridge operations, when
adding/removing channels to/from bridges.
In general, the error codes fall out as follows:
* Bridge not found - 404 Not Found
* Bridge not in Stasis - 409 Conflict
* Channel not found - 400 Bad Request
* Channel not in Stasis - 422 Unprocessable Entity
* Channel not in this bridge (on remove) - 422 Unprocessable Entity
(closes issue ASTERISK-22036)
Review: https://reviewboard.asterisk.org/r/2769/
Modified:
trunk/include/asterisk/stasis_app.h
trunk/include/asterisk/stasis_app_impl.h
trunk/res/ari/resource_bridges.c
trunk/res/res_ari_bridges.c
trunk/res/stasis/control.c
trunk/rest-api/api-docs/bridges.json
Modified: trunk/include/asterisk/stasis_app.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/stasis_app.h?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/include/asterisk/stasis_app.h (original)
+++ trunk/include/asterisk/stasis_app.h Fri Aug 23 12:19:02 2013
@@ -361,8 +361,10 @@
*
* \param control Control whose channel should be added to the bridge
* \param bridge Pointer to the bridge
- */
-void stasis_app_control_add_channel_to_bridge(
+ * \return non-zero on failure
+ * \return zero on success
+ */
+int stasis_app_control_add_channel_to_bridge(
struct stasis_app_control *control, struct ast_bridge *bridge);
/*!
@@ -370,9 +372,21 @@
*
* \param control Control whose channel should be removed from the bridge
* \param bridge Pointer to the bridge
- */
-void stasis_app_control_remove_channel_from_bridge(
+ * \return non-zero on failure
+ * \return zero on success
+ */
+int stasis_app_control_remove_channel_from_bridge(
struct stasis_app_control *control, struct ast_bridge *bridge);
+
+/*!
+ * \since 12
+ * \brief Gets the bridge currently associated with a control object.
+ *
+ * \param control Control object for the channel to query.
+ * \return Associated \ref ast_bridge.
+ * \return \c NULL if not associated with a bridge.
+ */
+struct ast_bridge *stasis_app_get_bridge(struct stasis_app_control *control);
/*!
* \brief Destroy the bridge.
Modified: trunk/include/asterisk/stasis_app_impl.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/stasis_app_impl.h?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/include/asterisk/stasis_app_impl.h (original)
+++ trunk/include/asterisk/stasis_app_impl.h Fri Aug 23 12:19:02 2013
@@ -85,14 +85,4 @@
int stasis_app_send_command_async(struct stasis_app_control *control,
stasis_app_command_cb command, void *data);
-/*!
- * \since 12
- * \brief Gets the bridge currently associated with a control object.
- *
- * \param control Control object for the channel to query.
- * \return Associated \ref ast_bridge.
- * \return \c NULL if not associated with a bridge.
- */
-struct ast_bridge *stasis_app_get_bridge(struct stasis_app_control *control);
-
#endif /* _ASTERISK_RES_STASIS_H */
Modified: trunk/res/ari/resource_bridges.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/ari/resource_bridges.c?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/res/ari/resource_bridges.c (original)
+++ trunk/res/ari/resource_bridges.c Fri Aug 23 12:19:02 2013
@@ -99,6 +99,18 @@
control = stasis_app_control_find_by_channel_id(channel_id);
if (control == NULL) {
+ /* Distinguish between 400 and 422 errors */
+ RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL,
+ ao2_cleanup);
+ snapshot = ast_channel_snapshot_get_latest(channel_id);
+ if (snapshot == NULL) {
+ ast_log(LOG_DEBUG, "Couldn't find '%s'\n", channel_id);
+ ast_ari_response_error(response, 400, "Bad Request",
+ "Channel not found");
+ return NULL;
+ }
+
+ ast_log(LOG_DEBUG, "Found non-stasis '%s'\n", channel_id);
ast_ari_response_error(response, 422, "Unprocessable Entity",
"Channel not in Stasis application");
return NULL;
@@ -145,6 +157,7 @@
list->controls[list->count] =
find_channel_control(response, channels[i]);
if (!list->controls[list->count]) {
+ /* response filled in by find_channel_control() */
return NULL;
}
++list->count;
@@ -166,7 +179,7 @@
size_t i;
if (!bridge) {
- /* Response filled in by find_bridge */
+ /* Response filled in by find_bridge() */
return;
}
@@ -200,7 +213,7 @@
size_t i;
if (!bridge) {
- /* Response filled in by find_bridge */
+ /* Response filled in by find_bridge() */
return;
}
@@ -210,17 +223,22 @@
return;
}
- /* BUGBUG this should make sure the bridge requested for removal is actually
- * the bridge the channel is in. This will be possible once the bridge uniqueid
- * is added to the channel snapshot. A 409 response should be issued if the bridge
- * uniqueids don't match */
+ /* Make sure all of the channels are in this bridge */
+ for (i = 0; i < list->count; ++i) {
+ if (stasis_app_get_bridge(list->controls[i]) != bridge) {
+ ast_log(LOG_WARNING, "Channel %s not in bridge %s\n",
+ args->channel[i], args->bridge_id);
+ ast_ari_response_error(response, 422,
+ "Unprocessable Entity",
+ "Channel not in this bridge");
+ return;
+ }
+ }
+
+ /* Now actually remove it */
for (i = 0; i < list->count; ++i) {
stasis_app_control_remove_channel_from_bridge(list->controls[i],
bridge);
- }
-
- if (response->response_code) {
- return;
}
ast_ari_response_no_content(response);
Modified: trunk/res/res_ari_bridges.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_ari_bridges.c?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/res/res_ari_bridges.c (original)
+++ trunk/res/res_ari_bridges.c Fri Aug 23 12:19:02 2013
@@ -345,9 +345,10 @@
break;
case 500: /* Internal Server Error */
case 501: /* Not Implemented */
+ case 400: /* Channel not found */
case 404: /* Bridge not found */
case 409: /* Bridge not in Stasis application */
- case 422: /* Channel not found, or not in Stasis application */
+ case 422: /* Channel not in Stasis application */
is_valid = 1;
break;
default:
@@ -444,6 +445,10 @@
break;
case 500: /* Internal Server Error */
case 501: /* Not Implemented */
+ case 400: /* Channel not found */
+ case 404: /* Bridge not found */
+ case 409: /* Bridge not in Stasis application */
+ case 422: /* Channel not in this bridge */
is_valid = 1;
break;
default:
Modified: trunk/res/stasis/control.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/stasis/control.c?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/res/stasis/control.c (original)
+++ trunk/res/stasis/control.c Fri Aug 23 12:19:02 2013
@@ -510,6 +510,9 @@
ast_bridge_after_cb_reason_string(reason));
}
+static int OK = 0;
+static int FAIL = -1;
+
static void *app_control_add_channel_to_bridge(
struct stasis_app_control *control,
struct ast_channel *chan, void *data)
@@ -542,7 +545,7 @@
bridge_after_cb_failed, control);
if (res != 0) {
ast_log(LOG_ERROR, "Error setting after-bridge callback\n");
- return NULL;
+ return &FAIL;
}
{
@@ -569,22 +572,24 @@
ast_log(LOG_ERROR, "Error adding channel to bridge\n");
ast_channel_pbx_set(chan, control->pbx);
control->pbx = NULL;
- return NULL;
+ return &FAIL;
}
ast_assert(stasis_app_get_bridge(control) == NULL);
control->bridge = bridge;
}
- return NULL;
-}
-
-void stasis_app_control_add_channel_to_bridge(
+ return &OK;
+}
+
+int stasis_app_control_add_channel_to_bridge(
struct stasis_app_control *control, struct ast_bridge *bridge)
{
+ int *res;
ast_debug(3, "%s: Sending channel add_to_bridge command\n",
stasis_app_control_get_channel_id(control));
- stasis_app_send_command_async(control,
+ res = stasis_app_send_command(control,
app_control_add_channel_to_bridge, bridge);
+ return *res;
}
static void *app_control_remove_channel_from_bridge(
@@ -594,7 +599,7 @@
struct ast_bridge *bridge = data;
if (!control) {
- return NULL;
+ return &FAIL;
}
/* We should only depart from our own bridge */
@@ -606,20 +611,22 @@
ast_log(LOG_WARNING, "%s: Not in bridge %s; not removing\n",
stasis_app_control_get_channel_id(control),
bridge->uniqueid);
- return NULL;
+ return &FAIL;
}
ast_bridge_depart(chan);
- return NULL;
-}
-
-void stasis_app_control_remove_channel_from_bridge(
+ return &OK;
+}
+
+int stasis_app_control_remove_channel_from_bridge(
struct stasis_app_control *control, struct ast_bridge *bridge)
{
+ int *res;
ast_debug(3, "%s: Sending channel remove_from_bridge command\n",
stasis_app_control_get_channel_id(control));
- stasis_app_send_command_async(control,
+ res = stasis_app_send_command(control,
app_control_remove_channel_from_bridge, bridge);
+ return *res;
}
const char *stasis_app_control_get_channel_id(
Modified: trunk/rest-api/api-docs/bridges.json
URL: http://svnview.digium.com/svn/asterisk/trunk/rest-api/api-docs/bridges.json?view=diff&rev=397565&r1=397564&r2=397565
==============================================================================
--- trunk/rest-api/api-docs/bridges.json (original)
+++ trunk/rest-api/api-docs/bridges.json Fri Aug 23 12:19:02 2013
@@ -131,6 +131,10 @@
],
"errorResponses": [
{
+ "code": 400,
+ "reason": "Channel not found"
+ },
+ {
"code": 404,
"reason": "Bridge not found"
},
@@ -140,7 +144,7 @@
},
{
"code": 422,
- "reason": "Channel not found, or not in Stasis application"
+ "reason": "Channel not in Stasis application"
}
]
}
@@ -171,6 +175,24 @@
"required": true,
"allowMultiple": true,
"dataType": "string"
+ }
+ ],
+ "errorResponses": [
+ {
+ "code": 400,
+ "reason": "Channel not found"
+ },
+ {
+ "code": 404,
+ "reason": "Bridge not found"
+ },
+ {
+ "code": 409,
+ "reason": "Bridge not in Stasis application"
+ },
+ {
+ "code": 422,
+ "reason": "Channel not in this bridge"
}
]
}
More information about the asterisk-commits
mailing list