message & stasis/messaging: make text message variables work in ARI

When a text message was received any associated variable was not written to
the ARI TextMessageReceived event. This occurred because Asterisk only wrote
out "send" variables. However, even those "send" variables would fail ARI
validation due to a TextMessageVariable formatting bug.

Since it seems the TextMessageReceived event has never been able to include
actual variables it was decided to remove the TextMessageVariable object type
from ARI, and simply return a JSON object of key/value pairs for variables.
This aligns more with how the ARI sendMessage handles variables, and other
places in ARI.

ASTERISK-28755 #close

Change-Id: Ia6051c01a53b30cf7edef84c27df4ed4479b8b6f

doc/UPGRADE-staging/ari_messaging.txt

@@ -0,0 +1,26 @@
+Subject: ARI
+Subject: res_stasis
+
+The "TextMessageReceived" event used to include a list of "TextMessageVariable"
+objects as part of its output. Due to a couple of bugs in Asterisk a list of
+received variables was never included even if ones were available. However,
+variables set to send would be (which they should have not been), but would
+fail validation due to the bad formatting.
+
+So basically there was no way to get a "TextMessageReceived" event with
+variables. Due to this the API has changed. The "TextMessageVariable" object
+no longer exists. "TextMessageReceived" now returns a JSON object of key/value
+pairs. So for instance instead of a list of "TextMessageVariable" objects:
+
+[ TextMessageVariable, TextMessageVariable, TextMessageVariable]
+
+where a TextMessageVariable was supposed to be:
+
+{ "key": "<var name>", "value":, "<var value>" }
+
+The output is now just:
+
+{ "<var name>": "<var value>" }
+
+This aligns more with how variables are specified when sending a message, as
+well as other variable lists in ARI.

include/asterisk/message.h

@@ -396,6 +396,19 @@ struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg
  */
 int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value);
 
+/*!
+ * \brief Get the next variable name and value that was set on a received message
+ * \param msg The message with the variables
+ * \param iter An iterator created with ast_msg_var_iterator_init
+ * \param name A pointer to the name result pointer
+ * \param value A pointer to the value result pointer
+ *
+ * \retval 0 No more entries
+ * \retval 1 Valid entry
+ */
+int ast_msg_var_iterator_next_received(const struct ast_msg *msg,
+	struct ast_msg_var_iterator *iter, const char **name, const char **value);
+
 /*!
  * \brief Destroy a message variable iterator
  * \param iter Iterator to be destroyed

main/message.c

@@ -627,7 +627,9 @@ struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg
 	return iter;
 }
 
-int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value)
+static int ast_msg_var_iterator_get_next(const struct ast_msg *msg,
+	struct ast_msg_var_iterator *iter, const char **name, const char **value,
+	unsigned int send)
 {
 	struct msg_data *data;
 
@@ -635,8 +637,8 @@ int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iter
 		return 0;
 	}
 
-	/* Skip any that aren't marked for sending out */
-	while ((data = ao2_iterator_next(&iter->iter)) && !data->send) {
+	/* Skip any that we're told to */
+	while ((data = ao2_iterator_next(&iter->iter)) && (data->send != send)) {
 		ao2_ref(data, -1);
 	}
 
@@ -644,7 +646,7 @@ int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iter
 		return 0;
 	}
 
-	if (data->send) {
+	if (data->send == send) {
 		*name = data->name;
 		*value = data->value;
 	}
@@ -656,6 +658,17 @@ int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iter
 	return 1;
 }
 
+int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value)
+{
+	return ast_msg_var_iterator_get_next(msg, iter, name, value, 1);
+}
+
+int ast_msg_var_iterator_next_received(const struct ast_msg *msg,
+	  struct ast_msg_var_iterator *iter, const char **name, const char **value)
+{
+	return ast_msg_var_iterator_get_next(msg, iter, name, value, 0);
+}
+
 void ast_msg_var_unref_current(struct ast_msg_var_iterator *iter)
 {
 	ao2_cleanup(iter->current_used);

res/ari/ari_model_validators.c

@@ -939,9 +939,8 @@ int ast_ari_validate_text_message(struct ast_json *json)
 		} else
 		if (strcmp("variables", ast_json_object_iter_key(iter)) == 0) {
 			int prop_is_valid;
-			prop_is_valid = ast_ari_validate_list(
-				ast_json_object_iter_value(iter),
-				ast_ari_validate_text_message_variable);
+			prop_is_valid = ast_ari_validate_object(
+				ast_json_object_iter_value(iter));
 			if (!prop_is_valid) {
 				ast_log(LOG_ERROR, "ARI TextMessage field variables failed validation\n");
 				res = 0;
@@ -978,60 +977,6 @@ ari_validator ast_ari_validate_text_message_fn(void)
 	return ast_ari_validate_text_message;
 }
 
-int ast_ari_validate_text_message_variable(struct ast_json *json)
-{
-	int res = 1;
-	struct ast_json_iter *iter;
-	int has_key = 0;
-	int has_value = 0;
-
-	for (iter = ast_json_object_iter(json); iter; iter = ast_json_object_iter_next(json, iter)) {
-		if (strcmp("key", ast_json_object_iter_key(iter)) == 0) {
-			int prop_is_valid;
-			has_key = 1;
-			prop_is_valid = ast_ari_validate_string(
-				ast_json_object_iter_value(iter));
-			if (!prop_is_valid) {
-				ast_log(LOG_ERROR, "ARI TextMessageVariable field key failed validation\n");
-				res = 0;
-			}
-		} else
-		if (strcmp("value", ast_json_object_iter_key(iter)) == 0) {
-			int prop_is_valid;
-			has_value = 1;
-			prop_is_valid = ast_ari_validate_string(
-				ast_json_object_iter_value(iter));
-			if (!prop_is_valid) {
-				ast_log(LOG_ERROR, "ARI TextMessageVariable field value failed validation\n");
-				res = 0;
-			}
-		} else
-		{
-			ast_log(LOG_ERROR,
-				"ARI TextMessageVariable has undocumented field %s\n",
-				ast_json_object_iter_key(iter));
-			res = 0;
-		}
-	}
-
-	if (!has_key) {
-		ast_log(LOG_ERROR, "ARI TextMessageVariable missing required field key\n");
-		res = 0;
-	}
-
-	if (!has_value) {
-		ast_log(LOG_ERROR, "ARI TextMessageVariable missing required field value\n");
-		res = 0;
-	}
-
-	return res;
-}
-
-ari_validator ast_ari_validate_text_message_variable_fn(void)
-{
-	return ast_ari_validate_text_message_variable;
-}
-
 int ast_ari_validate_caller_id(struct ast_json *json)
 {
 	int res = 1;

res/ari/ari_model_validators.h

@@ -387,24 +387,6 @@ int ast_ari_validate_text_message(struct ast_json *json);
  */
 ari_validator ast_ari_validate_text_message_fn(void);
 
-/*!
- * \brief Validator for TextMessageVariable.
- *
- * A key/value pair variable in a text message.
- *
- * \param json JSON object to validate.
- * \returns True (non-zero) if valid.
- * \returns False (zero) if invalid.
- */
-int ast_ari_validate_text_message_variable(struct ast_json *json);
-
-/*!
- * \brief Function pointer to ast_ari_validate_text_message_variable().
- *
- * See \ref ast_ari_model_validators.h for more details.
- */
-ari_validator ast_ari_validate_text_message_variable_fn(void);
-
 /*!
  * \brief Validator for CallerID.
  *
@@ -1497,10 +1479,7 @@ ari_validator ast_ari_validate_application_fn(void);
  * - body: string (required)
  * - from: string (required)
  * - to: string (required)
- * - variables: List[TextMessageVariable]
- * TextMessageVariable
- * - key: string (required)
- * - value: string (required)
+ * - variables: object
  * CallerID
  * - name: string (required)
  * - number: string (required)

res/stasis/messaging.c

@@ -262,23 +262,20 @@ static struct ast_json *msg_to_json(struct ast_msg *msg)
 		return NULL;
 	}
 
-	json_vars = ast_json_array_create();
+	json_vars = ast_json_object_create();
 	if (!json_vars) {
 		ast_msg_var_iterator_destroy(it_vars);
 		return NULL;
 	}
 
-	while (ast_msg_var_iterator_next(msg, it_vars, &name, &value)) {
-		struct ast_json *json_tuple;
-
-		json_tuple = ast_json_pack("{s: s}", name, value);
-		if (!json_tuple) {
+	while (ast_msg_var_iterator_next_received(msg, it_vars, &name, &value)) {
+		struct ast_json *json_val = ast_json_string_create(value);
+		if (!json_val || ast_json_object_set(json_vars, name, json_val)) {
 			ast_json_unref(json_vars);
 			ast_msg_var_iterator_destroy(it_vars);
 			return NULL;
 		}
 
-		ast_json_array_append(json_vars, json_tuple);
 		ast_msg_var_unref_current(it_vars);
 	}
 	ast_msg_var_iterator_destroy(it_vars);

rest-api/api-docs/endpoints.json

@@ -233,22 +233,6 @@
 				}
 			}
 		},
-		"TextMessageVariable": {
-			"id": "TextMessageVariable",
-			"description": "A key/value pair variable in a text message.",
-			"properties": {
-				"key": {
-					"type": "string",
-					"description": "A unique key identifying the variable.",
-					"required": true
-				},
-				"value": {
-					"type": "string",
-					"description": "The value of the variable.",
-					"required": true
-				}
-			}
-		},
 		"TextMessage": {
 			"id": "TextMessage",
 			"description": "A text message.",
@@ -269,8 +253,8 @@
 					"required": true
 				},
 				"variables": {
-					"type": "List[TextMessageVariable]",
-					"description": "Technology specific key/value pairs associated with the message.",
+					"type": "object",
+					"description": "Technology specific key/value pairs (JSON object) associated with the message.",
 					"required": false
 				}
 			}