[Asterisk-code-review] message & stasis/messaging: make text message variables work in ARI (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Wed Mar 4 06:10:15 CST 2020
Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13840 )
Change subject: message & stasis/messaging: make text message variables work in ARI
......................................................................
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
---
A doc/UPGRADE-staging/ari_messaging.txt
M include/asterisk/message.h
M main/message.c
M res/ari/ari_model_validators.c
M res/ari/ari_model_validators.h
M res/stasis/messaging.c
M rest-api/api-docs/endpoints.json
7 files changed, 65 insertions(+), 108 deletions(-)
Approvals:
George Joseph: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
diff --git a/doc/UPGRADE-staging/ari_messaging.txt b/doc/UPGRADE-staging/ari_messaging.txt
new file mode 100644
index 0000000..199a8a2
--- /dev/null
+++ b/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.
diff --git a/include/asterisk/message.h b/include/asterisk/message.h
index 826fa0a..f5b7a75 100644
--- a/include/asterisk/message.h
+++ b/include/asterisk/message.h
@@ -397,6 +397,19 @@
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
*/
diff --git a/main/message.c b/main/message.c
index 39b8d87..dc81b4e 100644
--- a/main/message.c
+++ b/main/message.c
@@ -627,7 +627,9 @@
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 @@
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 @@
return 0;
}
- if (data->send) {
+ if (data->send == send) {
*name = data->name;
*value = data->value;
}
@@ -656,6 +658,17 @@
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);
diff --git a/res/ari/ari_model_validators.c b/res/ari/ari_model_validators.c
index 8910bbb..0bbbb19 100644
--- a/res/ari/ari_model_validators.c
+++ b/res/ari/ari_model_validators.c
@@ -939,9 +939,8 @@
} 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 @@
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;
diff --git a/res/ari/ari_model_validators.h b/res/ari/ari_model_validators.h
index f9285b4..e34c2e4 100644
--- a/res/ari/ari_model_validators.h
+++ b/res/ari/ari_model_validators.h
@@ -388,24 +388,6 @@
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.
*
* Caller identification
@@ -1497,10 +1479,7 @@
* - 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)
diff --git a/res/stasis/messaging.c b/res/stasis/messaging.c
index a7716b8..78d6494 100644
--- a/res/stasis/messaging.c
+++ b/res/stasis/messaging.c
@@ -262,23 +262,20 @@
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);
diff --git a/rest-api/api-docs/endpoints.json b/rest-api/api-docs/endpoints.json
index 264c0eb..1f77d37 100644
--- a/rest-api/api-docs/endpoints.json
+++ b/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
}
}
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13840
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ia6051c01a53b30cf7edef84c27df4ed4479b8b6f
Gerrit-Change-Number: 13840
Gerrit-PatchSet: 3
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.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/20200304/e8009494/attachment-0001.html>
More information about the asterisk-code-review
mailing list