[Asterisk-code-review] ExternalMedia: Change return object from ExternalMedia to Channel (...asterisk[17])

George Joseph asteriskteam at digium.com
Mon Oct 21 13:53:11 CDT 2019


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/13077 )

Change subject: ExternalMedia:  Change return object from ExternalMedia to Channel
......................................................................

ExternalMedia:  Change return object from ExternalMedia to Channel

When we created the External Media addition to ARI we created an
ExternalMedia object to be returned from the channels/externalMedia
REST endpoint.  This object contained the channel object that was
created plus local_address and local_port attributes (which are
also in the Channel variables).  At the time, we thought that
creating an ExternalMedia object would give us more flexibility
in the future but as we created the sample speech to text
application, we discovered that it doesn't work so well with ARI
client libraries that a) don't have the ExternalMedia object
defined and/or b) can't promote the embedded channel structure
to a first-class Channel object.

This change causes the channels/externalMedia REST endpoint to
return a Channel object (like channels/create and channels/originate)
instead of the ExternalMedia object.

Change-Id: If280094debd35102cf21e0a31a5e0846fec14af9
---
M res/ari/ari_model_validators.c
M res/ari/ari_model_validators.h
M res/ari/resource_channels.c
M res/res_ari_channels.c
M rest-api/api-docs/channels.json
5 files changed, 3 insertions(+), 136 deletions(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/res/ari/ari_model_validators.c b/res/ari/ari_model_validators.c
index 3d63f53..8910bbb 100644
--- a/res/ari/ari_model_validators.c
+++ b/res/ari/ari_model_validators.c
@@ -1385,62 +1385,6 @@
 	return ast_ari_validate_dialplan_cep;
 }
 
-int ast_ari_validate_external_media(struct ast_json *json)
-{
-	int res = 1;
-	struct ast_json_iter *iter;
-	int has_channel = 0;
-
-	for (iter = ast_json_object_iter(json); iter; iter = ast_json_object_iter_next(json, iter)) {
-		if (strcmp("channel", ast_json_object_iter_key(iter)) == 0) {
-			int prop_is_valid;
-			has_channel = 1;
-			prop_is_valid = ast_ari_validate_channel(
-				ast_json_object_iter_value(iter));
-			if (!prop_is_valid) {
-				ast_log(LOG_ERROR, "ARI ExternalMedia field channel failed validation\n");
-				res = 0;
-			}
-		} else
-		if (strcmp("local_address", ast_json_object_iter_key(iter)) == 0) {
-			int prop_is_valid;
-			prop_is_valid = ast_ari_validate_string(
-				ast_json_object_iter_value(iter));
-			if (!prop_is_valid) {
-				ast_log(LOG_ERROR, "ARI ExternalMedia field local_address failed validation\n");
-				res = 0;
-			}
-		} else
-		if (strcmp("local_port", ast_json_object_iter_key(iter)) == 0) {
-			int prop_is_valid;
-			prop_is_valid = ast_ari_validate_int(
-				ast_json_object_iter_value(iter));
-			if (!prop_is_valid) {
-				ast_log(LOG_ERROR, "ARI ExternalMedia field local_port failed validation\n");
-				res = 0;
-			}
-		} else
-		{
-			ast_log(LOG_ERROR,
-				"ARI ExternalMedia has undocumented field %s\n",
-				ast_json_object_iter_key(iter));
-			res = 0;
-		}
-	}
-
-	if (!has_channel) {
-		ast_log(LOG_ERROR, "ARI ExternalMedia missing required field channel\n");
-		res = 0;
-	}
-
-	return res;
-}
-
-ari_validator ast_ari_validate_external_media_fn(void)
-{
-	return ast_ari_validate_external_media;
-}
-
 int ast_ari_validate_rtpstat(struct ast_json *json)
 {
 	int res = 1;
diff --git a/res/ari/ari_model_validators.h b/res/ari/ari_model_validators.h
index 53a8573..f9285b4 100644
--- a/res/ari/ari_model_validators.h
+++ b/res/ari/ari_model_validators.h
@@ -478,24 +478,6 @@
 ari_validator ast_ari_validate_dialplan_cep_fn(void);
 
 /*!
- * \brief Validator for ExternalMedia.
- *
- * ExternalMedia session.
- *
- * \param json JSON object to validate.
- * \returns True (non-zero) if valid.
- * \returns False (zero) if invalid.
- */
-int ast_ari_validate_external_media(struct ast_json *json);
-
-/*!
- * \brief Function pointer to ast_ari_validate_external_media().
- *
- * See \ref ast_ari_model_validators.h for more details.
- */
-ari_validator ast_ari_validate_external_media_fn(void);
-
-/*!
  * \brief Validator for RTPstat.
  *
  * A statistics of a RTP.
@@ -1540,10 +1522,6 @@
  * - context: string (required)
  * - exten: string (required)
  * - priority: long (required)
- * ExternalMedia
- * - channel: Channel (required)
- * - local_address: string
- * - local_port: int
  * RTPstat
  * - channel_uniqueid: string (required)
  * - local_maxjitter: double
diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c
index 0fd2d69..81a902c 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -2064,7 +2064,6 @@
 	size_t endpoint_len;
 	char *endpoint;
 	struct ast_channel *chan;
-	struct ast_json *json_chan;
 	struct varshead *vars;
 
 	endpoint_len = strlen("UnicastRTP/") + strlen(args->external_host) + 1;
@@ -2093,43 +2092,10 @@
 		return;
 	}
 
-	/*
-	 * At this point, response->message contains a channel object so we
-	 * need to save it then create a new ExternalMedia object and put the
-	 * channel in it.
-	 */
-	json_chan = response->message;
-	response->message = ast_json_object_create();
-	if (!response->message) {
-		ast_channel_unref(chan);
-		ast_json_unref(json_chan);
-		ast_ari_response_alloc_failed(response);
-		return;
-	}
-
-	ast_json_object_set(response->message, "channel", json_chan);
-	/*
-	 * At the time the channel snapshot was taken the channel variables might
-	 * not have been set so we try to grab them directly from the channel.
-	 */
 	ast_channel_lock(chan);
 	vars = ast_channel_varshead(chan);
 	if (vars && !AST_LIST_EMPTY(vars)) {
-		struct ast_var_t *variables;
-
-		/* Put them all on the channel object */
-		ast_json_object_set(json_chan, "channelvars", ast_json_channel_vars(vars));
-		/* Grab out the local address and port */
-		AST_LIST_TRAVERSE(vars, variables, entries) {
-			if (!strcmp("UNICASTRTP_LOCAL_ADDRESS", ast_var_name(variables))) {
-				ast_json_object_set(response->message, "local_address",
-					ast_json_string_create(ast_var_value(variables)));
-			}
-			else if (!strcmp("UNICASTRTP_LOCAL_PORT", ast_var_name(variables))) {
-				ast_json_object_set(response->message, "local_port",
-					ast_json_integer_create(strtol(ast_var_value(variables), NULL, 10)));
-			}
-		}
+		ast_json_object_set(response->message, "channelvars", ast_json_channel_vars(vars));
 	}
 	ast_channel_unlock(chan);
 	ast_channel_unref(chan);
diff --git a/res/res_ari_channels.c b/res/res_ari_channels.c
index 27d9deb..f938e14 100644
--- a/res/res_ari_channels.c
+++ b/res/res_ari_channels.c
@@ -2918,7 +2918,7 @@
 		break;
 	default:
 		if (200 <= code && code <= 299) {
-			is_valid = ast_ari_validate_external_media(
+			is_valid = ast_ari_validate_channel(
 				response->message);
 		} else {
 			ast_log(LOG_ERROR, "Invalid error response %d for /channels/externalMedia\n", code);
diff --git a/rest-api/api-docs/channels.json b/rest-api/api-docs/channels.json
index 53e6e9d..94afb27 100644
--- a/rest-api/api-docs/channels.json
+++ b/rest-api/api-docs/channels.json
@@ -1765,7 +1765,7 @@
 					"summary": "Start an External Media session.",
 					"notes": "Create a channel to an External Media source/sink.",
 					"nickname": "externalMedia",
-					"responseClass": "ExternalMedia",
+					"responseClass": "Channel",
 					"parameters": [
 						{
 							"name": "channelId",
@@ -2166,27 +2166,6 @@
 					"description": "Channel variables"
 				}
 			}
-		},
-		"ExternalMedia": {
-			"id": "ExternalMedia",
-			"description": "ExternalMedia session.",
-			"properties": {
-				"channel": {
-					"required": true,
-					"type": "Channel",
-					"description": "The Asterisk channel representing the external media"
-				},
-				"local_address": {
-					"required": false,
-					"type": "string",
-					"description": "The local ip address used" 
-				},
-				"local_port": {
-					"required": false,
-					"type": "int",
-					"description": "The local ip port used" 
-				}
-			}
 		}
 	}
 }

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13077
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: If280094debd35102cf21e0a31a5e0846fec14af9
Gerrit-Change-Number: 13077
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191021/c8ebffd9/attachment-0001.html>


More information about the asterisk-code-review mailing list