[Asterisk-code-review] res_geolocation: Add option to GEOLOC_PROFILE to re-resolve info (asterisk[16])

George Joseph asteriskteam at digium.com
Thu Aug 25 13:32:52 CDT 2022


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/18989 )


Change subject: res_geolocation: Add option to GEOLOC_PROFILE to re-resolve info
......................................................................

res_geolocation: Add option to GEOLOC_PROFILE to re-resolve info

Added an 'r' option to the GEOLOC_PROFILE function to resolve all
variables before a read operation and after a Set operation.

Also added a few missing parameters to the ones allowed for
writing.

ASTERISK-30190

Change-Id: I75f541db43345509a2e86225bfa4cf8e242e5b6c
---
M doc/CHANGES-staging/res_geolocation.txt
M res/res_geolocation/geoloc_dialplan.c
M res/res_geolocation/geoloc_doc.xml
M res/res_geolocation/geoloc_eprofile.c
M res/res_geolocation/geoloc_private.h
5 files changed, 148 insertions(+), 32 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/89/18989/1

diff --git a/doc/CHANGES-staging/res_geolocation.txt b/doc/CHANGES-staging/res_geolocation.txt
index deda71c..d035888 100644
--- a/doc/CHANGES-staging/res_geolocation.txt
+++ b/doc/CHANGES-staging/res_geolocation.txt
@@ -18,3 +18,6 @@
 information isn't common with any other profiles.  This is
 mutually exclusive with setting location_reference on the
 profile.
+
+Added an 'r' option to the GEOLOC_PROFILE function to resolve all
+variables before a read operation and after a Set operation.
diff --git a/res/res_geolocation/geoloc_dialplan.c b/res/res_geolocation/geoloc_dialplan.c
index 710daa6..bbc6abf 100644
--- a/res/res_geolocation/geoloc_dialplan.c
+++ b/res/res_geolocation/geoloc_dialplan.c
@@ -25,7 +25,6 @@
 #include "asterisk/strings.h"
 #include "asterisk/utils.h"
 #include "asterisk/app.h"
-#include "asterisk/res_geolocation.h"
 #include "geoloc_private.h"
 
 static void varlist_to_str(struct ast_variable *list, struct ast_str** buf, size_t len)
@@ -37,20 +36,49 @@
 	}
 }
 
+#define RESOLVE_FOR_READ(_param) \
+({ \
+	if (ast_test_flag(&opts, OPT_GEOLOC_RESOLVE)) { \
+		struct ast_variable *resolved = geoloc_eprofile_resolve_varlist( \
+			eprofile->_param, eprofile->location_variables, chan); \
+		if (!resolved) { \
+			ast_log(LOG_ERROR, "%s: Unable to resolve " #_param "\n", chan_name); \
+			pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-3"); \
+			return 0; \
+		} \
+		varlist_to_str(resolved, buf, len); \
+		ast_variables_destroy(resolved); \
+	} else { \
+		varlist_to_str(eprofile->_param, buf, len); \
+	} \
+})
+
+enum my_app_option_flags {
+	OPT_GEOLOC_RESOLVE = (1 << 0),
+};
+
+AST_APP_OPTIONS(action_options, {
+	AST_APP_OPTION('r', OPT_GEOLOC_RESOLVE),
+});
+
+
 static int geoloc_profile_read(struct ast_channel *chan,
 	const char *cmd, char *data, struct ast_str **buf, ssize_t len)
 {
 	char *parsed_data = ast_strdupa(data);
+	const char *chan_name = ast_channel_name(chan);
 	struct ast_datastore *ds;
 	struct ast_geoloc_eprofile *eprofile = NULL;
+	struct ast_flags opts = { 0, };
 
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(field);
+		AST_APP_ARG(options);
 	);
 
 	/* Check for zero arguments */
 	if (ast_strlen_zero(parsed_data)) {
-		ast_log(LOG_ERROR, "%s: Cannot call without arguments\n", cmd);
+		ast_log(LOG_ERROR, "%s: Cannot call without arguments\n", chan_name);
 		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-1");
 		return 0;
 	}
@@ -58,24 +86,33 @@
 	AST_STANDARD_APP_ARGS(args, parsed_data);
 
 	if (ast_strlen_zero(args.field)) {
-		ast_log(LOG_ERROR, "%s: Cannot call without a field to query\n", cmd);
+		ast_log(LOG_ERROR, "%s: Cannot call without a field to query\n", chan_name);
 		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-1");
 		return 0;
 	}
 
+	if (!ast_strlen_zero(args.options)) {
+		if (ast_app_parse_options(action_options, &opts, NULL, args.options)) {
+			ast_log(LOG_ERROR, "%s: Invalid options: %s\n", chan_name, args.options);
+			pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-1");
+			return 0;
+		}
+	}
+
 	ds = ast_geoloc_datastore_find(chan);
 	if (!ds) {
-		ast_log(LOG_NOTICE, "%s: There is no geoloc profile on this channel\n", cmd);
+		ast_log(LOG_NOTICE, "%s: There is no geoloc profile on this channel\n", chan_name);
 		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-2");
 		return 0;
 	}
 
 	eprofile = ast_geoloc_datastore_get_eprofile(ds, 0);
 	if (!eprofile) {
-		ast_log(LOG_NOTICE, "%s: There is no geoloc profile on this channel\n", cmd);
+		ast_log(LOG_NOTICE, "%s: There is no geoloc profile on this channel\n", chan_name);
 		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-2");
 		return 0;
 	}
+	ast_geoloc_eprofile_refresh_location(eprofile);
 
 	pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "0");
 	if (ast_strings_equal(args.field, "inheritable")) {
@@ -101,19 +138,19 @@
 	} else if (ast_strings_equal(args.field, "notes")) {
 		ast_str_append(buf, len, "%s", eprofile->notes);
 	} else if (ast_strings_equal(args.field, "location_info")) {
-		varlist_to_str(eprofile->location_info, buf, len);
+		RESOLVE_FOR_READ(location_info);
 	} else if (ast_strings_equal(args.field, "location_info_refinement")) {
-		varlist_to_str(eprofile->location_refinement, buf, len);
+		RESOLVE_FOR_READ(location_refinement);
 	} else if (ast_strings_equal(args.field, "location_variables")) {
-		varlist_to_str(eprofile->location_variables, buf, len);
+		RESOLVE_FOR_READ(location_variables);
 	} else if (ast_strings_equal(args.field, "effective_location")) {
-		varlist_to_str(eprofile->effective_location, buf, len);
+		RESOLVE_FOR_READ(effective_location);
 	} else if (ast_strings_equal(args.field, "usage_rules")) {
-		varlist_to_str(eprofile->usage_rules, buf, len);
+		RESOLVE_FOR_READ(usage_rules);
 	} else if (ast_strings_equal(args.field, "confidence")) {
 		varlist_to_str(eprofile->confidence, buf, len);
 	} else {
-		ast_log(LOG_ERROR, "%s: Field '%s' is not valid\n", cmd, args.field);
+		ast_log(LOG_ERROR, "%s: Field '%s' is not valid\n", chan_name, args.field);
 		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-3");
 	}
 
@@ -146,6 +183,22 @@
 	_ep->_field = _list; \
 })
 
+
+#define RESOLVE_FOR_WRITE(_param) \
+({ \
+if (ast_test_flag(&opts, OPT_GEOLOC_RESOLVE)) { \
+	struct ast_variable *resolved = geoloc_eprofile_resolve_varlist( \
+		eprofile->_param, eprofile->location_variables, chan); \
+	if (!resolved) { \
+		ast_log(LOG_ERROR, "%s: Unable to resolve " #_param "\n", chan_name); \
+		pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-3"); \
+		return 0; \
+	} \
+	ast_variables_destroy(eprofile->_param); \
+	eprofile->_param = resolved; \
+} \
+})
+
 static int geoloc_profile_write(struct ast_channel *chan, const char *cmd, char *data,
 	 const char *value)
 {
@@ -153,9 +206,11 @@
 	const char *chan_name = ast_channel_name(chan);
 	struct ast_datastore *ds; /* Reminder: datastores aren't ao2 objects */
 	RAII_VAR(struct ast_geoloc_eprofile *, eprofile, NULL, ao2_cleanup);
+	struct ast_flags opts = { 0, };
 
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(field);
+		AST_APP_ARG(options);
 	);
 
 	/* Check for zero arguments */
@@ -173,6 +228,14 @@
 		return 0;
 	}
 
+	if (!ast_strlen_zero(args.options)) {
+		if (ast_app_parse_options(action_options, &opts, NULL, args.options)) {
+			ast_log(LOG_ERROR, "%s: Invalid options: %s\n", chan_name, args.options);
+			pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "-1");
+			return 0;
+		}
+	}
+
 	ds = ast_geoloc_datastore_find(chan);
 	if (!ds) {
 		ds = ast_geoloc_datastore_create(ast_channel_name(chan));
@@ -203,6 +266,8 @@
 
 	if (ast_strings_equal(args.field, "inheritable")) {
 		ast_geoloc_datastore_set_inheritance(ds, ast_true(value));
+	} else if (ast_strings_equal(args.field, "id")) {
+		ast_string_field_set(eprofile, id, value);
 	} else if (ast_strings_equal(args.field, "location_reference")) {
 		struct ast_geoloc_location *loc = ast_geoloc_get_location(value);
 		ao2_cleanup(loc);
@@ -224,18 +289,25 @@
 		TEST_ENUM_VALUE(chan_name, eprofile, format, value);
 	} else if (ast_strings_equal(args.field, "pidf_element")) {
 		TEST_ENUM_VALUE(chan_name, eprofile, pidf_element, value);
-	} else if (ast_strings_equal(args.field, "location_info")) {
-		TEST_VARLIST(chan_name, eprofile, location_info, value);
 	} else if (ast_strings_equal(args.field, "location_source")) {
 		ast_string_field_set(eprofile, location_source, value);
+	} else if (ast_strings_equal(args.field, "notes")) {
+		ast_string_field_set(eprofile, notes, value);
+	} else if (ast_strings_equal(args.field, "location_info")) {
+		TEST_VARLIST(chan_name, eprofile, location_info, value);
+		RESOLVE_FOR_WRITE(location_info);
 	} else if (ast_strings_equal(args.field, "location_info_refinement")) {
 		TEST_VARLIST(chan_name, eprofile, location_refinement, value);
+		RESOLVE_FOR_WRITE(location_refinement);
 	} else if (ast_strings_equal(args.field, "location_variables")) {
 		TEST_VARLIST(chan_name, eprofile, location_variables, value);
+		RESOLVE_FOR_WRITE(location_variables);
 	} else if (ast_strings_equal(args.field, "effective_location")) {
 		TEST_VARLIST(chan_name, eprofile, effective_location, value);
+		RESOLVE_FOR_WRITE(effective_location);
 	} else if (ast_strings_equal(args.field, "usage_rules")) {
 		TEST_VARLIST(chan_name, eprofile, usage_rules, value);
+		RESOLVE_FOR_WRITE(usage_rules);
 	} else if (ast_strings_equal(args.field, "confidence")) {
 		TEST_VARLIST(chan_name, eprofile, confidence, value);
 	} else {
@@ -245,6 +317,7 @@
 	}
 
 	ast_geoloc_eprofile_refresh_location(eprofile);
+
 	pbx_builtin_setvar_helper(chan, "GEOLOCPROFILESTATUS", "0");
 
 	return 0;
diff --git a/res/res_geolocation/geoloc_doc.xml b/res/res_geolocation/geoloc_doc.xml
index 4f7cdc2..84d6db1e 100644
--- a/res/res_geolocation/geoloc_doc.xml
+++ b/res/res_geolocation/geoloc_doc.xml
@@ -7,12 +7,14 @@
 			<configObject name="location">
 				<synopsis>Location</synopsis>
 				<description>
-					<para>cffdffff</para>
+					<para>Parameters for defining a Location object</para>
 				</description>
+
 				<configOption name="type">
 					<synopsis>Must be of type 'location'.</synopsis>
 				</configOption>
-				<configOption name="format" default="">
+
+				<configOption name="format" default="none">
 					<synopsis>Location specification type</synopsis>
 					<description>
 						<enumlist>
@@ -42,7 +44,8 @@
 						</enumlist>
 					</description>
 				</configOption>
-				<configOption name="location_info" default="">
+
+				<configOption name="location_info" default="none">
 					<synopsis>Location information</synopsis>
 					<description>
 						<para>The contents of this parameter are specific to the
@@ -68,7 +71,8 @@
 						</enumlist>
 					</description>
 				</configOption>
-				<configOption name="location_source" default="">
+
+				<configOption name="location_source" default="none">
 					<synopsis>Fully qualified host name</synopsis>
 					<description>
 						<para>This parameter isn't required but if provided, RFC8787 says it MUST be a fully
@@ -77,7 +81,8 @@
 						Geolocation</literal> header.</para>
 					</description>
 				</configOption>
-				<configOption name="method" default="">
+
+				<configOption name="method" default="none">
 					<synopsis>Location determination method</synopsis>
 					<description>
 						<para>This is a rarely used field in the specification that would
@@ -94,7 +99,8 @@
 						</enumlist>
 					</description>
 				</configOption>
-				<configOption name="confidence" default="">
+
+				<configOption name="confidence" default="none">
 					<synopsis>Level of confidence</synopsis>
 					<description>
 						<para>This is a rarely used field in the specification that would
@@ -123,14 +129,16 @@
 					</see-also>
 				</configOption>
 			</configObject>
+
 			<configObject name="profile">
 				<synopsis>Profile</synopsis>
 				<description>
-					<para>cffdffff</para>
+					<para>Parameters for defining a Profile object</para>
 				</description>
 				<configOption name="type">
 					<synopsis>Must be of type 'profile'.</synopsis>
 				</configOption>
+
 				<configOption name="pidf_element" default="device">
 					<synopsis>PIDF-LO element to place this profile in</synopsis>
 					<description>
@@ -148,21 +156,25 @@
 						<ref type="link">https://www.rfc-editor.org/rfc/rfc5491.html#section-3.4</ref>
 					</see-also>
 				</configOption>
-				<configOption name="location_reference" default="">
+
+				<configOption name="location_reference" default="none">
 					<synopsis>Reference to a location object</synopsis>
 				</configOption>
-				<configOption name="location_info_refinement" default="">
+
+				<configOption name="location_info_refinement" default="none">
 					<synopsis>Reference to a location object</synopsis>
 				</configOption>
-				<configOption name="location_variables" default="">
+				<configOption name="location_variables" default="none">
 					<synopsis>Reference to a location object</synopsis>
 				</configOption>
-				<configOption name="usage_rules" default="yes">
+
+				<configOption name="usage_rules" default="empty <usage_rules> element">
 					<synopsis>location specification type</synopsis>
 					<description>
 						<para>xxxx</para>
 					</description>
 				</configOption>
+
 				<configOption name="notes" default="">
 					<synopsis>Notes to be added to the outgoing PIDF-LO document</synopsis>
 					<description>
@@ -171,11 +183,13 @@
 						outgoing PIDF-LO document.  Its usage should be pre-negotiated with
 						any recipients.</para>
 					</description>
+
 				</configOption>
-				<configOption name="allow_routing_use">
+				<configOption name="allow_routing_use" default="no">
 					<synopsis>Sets the value of the Geolocation-Routing header.</synopsis>
 				</configOption>
-				<configOption name="suppress_empty_ca_elements">
+
+				<configOption name="suppress_empty_ca_elements" default="no">
 					<synopsis>Sets if empty Civic Address elements should be suppressed
 					from the PIDF-LO document.</synopsis>
 				</configOption>
@@ -207,6 +221,7 @@
 						</enumlist>
 					</description>
 				</configOption>
+
 				<xi:include xpointer="xpointer(/docs/configInfo[@name='res_geolocation']/configFile[@name='geolocation.conf']/configObject[@name='location']/configOption[@name='format'])"/>
 				<xi:include xpointer="xpointer(/docs/configInfo[@name='res_geolocation']/configFile[@name='geolocation.conf']/configObject[@name='location']/configOption[@name='location_info'])"/>
 				<xi:include xpointer="xpointer(/docs/configInfo[@name='res_geolocation']/configFile[@name='geolocation.conf']/configObject[@name='location']/configOption[@name='confidence'])"/>
@@ -220,8 +235,8 @@
 			Get or Set a field in a geolocation profile
 		</synopsis>
 		<syntax>
-			<parameter name="field" required="true">
-				<para>The profile field to operate on. The following fields from the
+			<parameter name="parameter" required="true">
+				<para>The profile parameter to operate on. The following fields from the
 				Location and Profile objects are supported.</para>
 				<enumlist>
 					<enum name="id"/>
@@ -244,10 +259,31 @@
 				set to <literal>true</literal> or <literal>false</literal> to control
 				whether the profile will be passed to the outgoing channel.
 				</para>
+				<para>
+				</para>
+			</parameter>
+
+			<parameter name="options" required="false">
+				<optionlist>
+				<option name="r">
+					<para>Before reading or after writing the specified parameter,
+					re-resolve the <literal>effective_location</literal> and
+					<literal>usage_rules</literal> parameters using the
+					<literal>location_variables</literal> parameter and the variables
+					set on the channel in effect at the time this function is called.
+					</para>
+					<note><para>On a read operation, this does not alter the actual profile
+						in any way.  On a write operation however, the
+						<literal>effective_location</literal> and/or <literal>usage_rules</literal>
+						parameters may indeed change and those changes will be passed on
+						to any outgoing channel.
+					</para></note>
+				</option>
+				</optionlist>
 			</parameter>
 		</syntax>
 		<description><para>
-		When used to set a field on a profile, if the profile doesn't already exist, a new
+		When used to set a parameter on a profile, if the profile doesn't already exist, a new
 		one will be created automatically.
 		</para>
 		<para>
@@ -258,8 +294,8 @@
 			<enum name="0"><para>Success</para></enum>
 			<enum name="-1"><para>No or not enough parameters were supplied</para></enum>
 			<enum name="-2"><para>There was an internal error finding or creating a profile</para></enum>
-			<enum name="-3"><para>There was an issue specific to the field specified
-			(value not valid or field name not found)</para></enum>
+			<enum name="-3"><para>There was an issue specific to the parameter specified
+			(value not valid or parameter name not found, etc.)</para></enum>
 		</enumlist>
 		</description>
 	</function>
diff --git a/res/res_geolocation/geoloc_eprofile.c b/res/res_geolocation/geoloc_eprofile.c
index 1deb76e..9d6260e 100644
--- a/res/res_geolocation/geoloc_eprofile.c
+++ b/res/res_geolocation/geoloc_eprofile.c
@@ -287,7 +287,7 @@
 	return eprofile;
 }
 
-static struct ast_variable *geoloc_eprofile_resolve_varlist(struct ast_variable *source,
+struct ast_variable *geoloc_eprofile_resolve_varlist(struct ast_variable *source,
 	struct ast_variable *variables, struct ast_channel *chan)
 {
 	struct ast_variable *dest = NULL;
diff --git a/res/res_geolocation/geoloc_private.h b/res/res_geolocation/geoloc_private.h
index 910dbc5..0bd0797 100644
--- a/res/res_geolocation/geoloc_private.h
+++ b/res/res_geolocation/geoloc_private.h
@@ -155,4 +155,8 @@
 
 struct ast_sorcery *geoloc_get_sorcery(void);
 
+struct ast_variable *geoloc_eprofile_resolve_varlist(struct ast_variable *source,
+	struct ast_variable *variables, struct ast_channel *chan);
+
+
 #endif /* GEOLOC_PRIVATE_H_ */

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I75f541db43345509a2e86225bfa4cf8e242e5b6c
Gerrit-Change-Number: 18989
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220825/e77b9f52/attachment-0001.html>


More information about the asterisk-code-review mailing list