[Asterisk-code-review] res sorcery config: Allow configuration section to be used b... (asterisk[13])

George Joseph asteriskteam at digium.com
Wed Jul 18 14:47:01 CDT 2018


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

Change subject: res_sorcery_config: Allow configuration section to be used based on name.
......................................................................

res_sorcery_config: Allow configuration section to be used based on name.

A problem I've seen countless times is a global or system section
for PJSIP not getting applied. This is inevitably the result of
the "type=" line missing. This change alleviates that problem.

The ability to specify an explicit section name has been
added to res_sorcery_config. If the configured section
name matches this and there are no unknown things configured
the section is taken as being for the given type.

Both the PJSIP "global" and "system" types now support this
so you can just name your section "global" or "system" and it
will be matched and used, even without a "type=" line.

ASTERISK-27972

Change-Id: Ie22723663c1ddd24f869af8c9b4c1b59e2476893
---
M res/res_pjsip.c
M res/res_pjsip/config_global.c
M res/res_pjsip/config_system.c
M res/res_sorcery_config.c
4 files changed, 100 insertions(+), 11 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, approved



diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index c440454..e087564 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -1661,7 +1661,7 @@
 					</description>
 				</configOption>
 				<configOption name="type">
-					<synopsis>Must be of type 'system'.</synopsis>
+					<synopsis>Must be of type 'system' UNLESS the object name is 'system'.</synopsis>
 				</configOption>
 			</configObject>
 			<configObject name="global">
@@ -1708,7 +1708,7 @@
 					twice the unidentified_request_period are pruned.</synopsis>
 				</configOption>
 				<configOption name="type">
-					<synopsis>Must be of type 'global'.</synopsis>
+					<synopsis>Must be of type 'global' UNLESS the object name is 'global'.</synopsis>
 				</configOption>
 				<configOption name="user_agent" default="Asterisk <Asterisk Version>">
 					<synopsis>Value used in User-Agent header for SIP requests and Server header for SIP responses.</synopsis>
diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c
index fc1227d..2a80312 100644
--- a/res/res_pjsip/config_global.c
+++ b/res/res_pjsip/config_global.c
@@ -491,7 +491,7 @@
 	snprintf(default_useragent, sizeof(default_useragent), "%s %s",
 		DEFAULT_USERAGENT_PREFIX, ast_get_version());
 
-	ast_sorcery_apply_default(sorcery, "global", "config", "pjsip.conf,criteria=type=global");
+	ast_sorcery_apply_default(sorcery, "global", "config", "pjsip.conf,criteria=type=global,single_object=yes,explicit_name=global");
 
 	if (ast_sorcery_object_register(sorcery, "global", global_alloc, NULL, global_apply)) {
 		return -1;
diff --git a/res/res_pjsip/config_system.c b/res/res_pjsip/config_system.c
index 65e4e2c..2ff0a65 100644
--- a/res/res_pjsip/config_system.c
+++ b/res/res_pjsip/config_system.c
@@ -175,7 +175,7 @@
 		return -1;
 	}
 
-	ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system");
+	ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system,single_object=yes,explicit_name=system");
 
 	if (ast_sorcery_object_register_no_reload(system_sorcery, "system", system_alloc, NULL, system_apply)) {
 		ast_log(LOG_ERROR, "Failed to register with sorcery (is res_sorcery_config loaded?)\n");
diff --git a/res/res_sorcery_config.c b/res/res_sorcery_config.c
index b02000e..a458d91 100644
--- a/res/res_sorcery_config.c
+++ b/res/res_sorcery_config.c
@@ -52,12 +52,18 @@
 	/*! \brief Any specific variable criteria for considering a defined category for this object */
 	struct ast_variable *criteria;
 
+	/*! \brief An explicit name for the configuration section, with it there can be only one */
+	char *explicit_name;
+
 	/*! \brief Number of buckets to use for objects */
 	unsigned int buckets;
 
 	/*! \brief Enable file level integrity instead of object level */
 	unsigned int file_integrity:1;
 
+	/*! \brief Enable enforcement of a single configuration object of this type */
+	unsigned int single_object:1;
+
 	/*! \brief Filename of the configuration file */
 	char filename[];
 };
@@ -115,6 +121,7 @@
 	ao2_global_obj_release(config->objects);
 	ast_rwlock_destroy(&config->objects.lock);
 	ast_variables_destroy(config->criteria);
+	ast_free(config->explicit_name);
 }
 
 static int sorcery_config_fields_cmp(void *obj, void *arg, int flags)
@@ -239,12 +246,66 @@
 	ao2_callback(config_objects, OBJ_NODATA | OBJ_MULTIPLE, sorcery_config_fields_cmp, &params);
 }
 
-/*! \brief Internal function which determines if criteria has been met for considering an object set applicable */
-static int sorcery_is_criteria_met(struct ast_variable *objset, struct ast_variable *criteria)
+/*! \brief Internal function which determines if a category matches based on explicit name */
+static int sorcery_is_explicit_name_met(const struct ast_sorcery *sorcery, const char *type,
+	struct ast_category *category, struct sorcery_config *config)
+{
+	struct ast_sorcery_object_type *object_type;
+	struct ast_variable *field;
+	int met = 1;
+
+	if (ast_strlen_zero(config->explicit_name) || strcmp(ast_category_get_name(category), config->explicit_name)) {
+		return 0;
+	}
+
+	object_type = ast_sorcery_get_object_type(sorcery, type);
+	if (!object_type) {
+		return 0;
+	}
+
+	/* We iterate the configured fields to see if we don't know any, if we don't then
+	 * this is likely not for the given type and we skip it. If it actually is then criteria
+	 * may pick it up in which case it would just get rejected as an invalid configuration later.
+	 */
+	for (field = ast_category_first(category); field; field = field->next) {
+		if (!ast_sorcery_is_object_field_registered(object_type, field->name)) {
+			met = 0;
+			break;
+		}
+	}
+
+	ao2_ref(object_type, -1);
+
+	return met;
+}
+
+/*! \brief Internal function which determines if a category matches based on criteria */
+static int sorcery_is_criteria_met(struct ast_category *category, struct sorcery_config *config)
 {
 	RAII_VAR(struct ast_variable *, diff, NULL, ast_variables_destroy);
 
-	return (!criteria || (!ast_sorcery_changeset_create(objset, criteria, &diff) && !diff)) ? 1 : 0;
+	if (!config->criteria) {
+		return 0;
+	}
+
+	return (!ast_sorcery_changeset_create(ast_category_first(category), config->criteria, &diff) && !diff) ? 1 : 0;
+}
+
+/*! \brief Internal function which determines if criteria has been met for considering an object set applicable */
+static int sorcery_is_configuration_met(const struct ast_sorcery *sorcery, const char *type,
+	struct ast_category *category, struct sorcery_config *config)
+{
+	if (!config->criteria && ast_strlen_zero(config->explicit_name)) {
+		/* Nothing is configured to allow specific matching, so accept it! */
+		return 1;
+	} else if (sorcery_is_explicit_name_met(sorcery, type, category, config)) {
+		return 1;
+	} else if (sorcery_is_criteria_met(category, config)) {
+		return 1;
+	} else {
+		/* Nothing explicitly matched so reject */
+		return 0;
+	}
 }
 
 static void sorcery_config_internal_load(void *data, const struct ast_sorcery *sorcery, const char *type, unsigned int reload)
@@ -271,8 +332,8 @@
 	if (!config->buckets) {
 		while ((category = ast_category_browse_filtered(cfg, NULL, category, NULL))) {
 
-			/* If given criteria has not been met skip the category, it is not applicable */
-			if (!sorcery_is_criteria_met(ast_category_first(category), config->criteria)) {
+			/* If given configuration has not been met skip the category, it is not applicable */
+			if (!sorcery_is_configuration_met(sorcery, type, category, config)) {
 				continue;
 			}
 
@@ -294,6 +355,16 @@
 		buckets = config->buckets;
 	}
 
+	/* For single object configurations there can only ever be one bucket, if there's more than the single
+	 * object requirement has been violated.
+	 */
+	if (config->single_object && buckets > 1) {
+		ast_log(LOG_ERROR, "Config file '%s' could not be loaded; configuration contains more than one object of type '%s'\n",
+			config->filename, type);
+		ast_config_destroy(cfg);
+		return;
+	}
+
 	ast_debug(2, "Using bucket size of '%d' for objects of type '%s' from '%s'\n",
 		buckets, type, config->filename);
 
@@ -310,8 +381,8 @@
 		RAII_VAR(void *, obj, NULL, ao2_cleanup);
 		id = ast_category_get_name(category);
 
-		/* If given criteria has not been met skip the category, it is not applicable */
-		if (!sorcery_is_criteria_met(ast_category_first(category), config->criteria)) {
+		/* If given configurationhas not been met skip the category, it is not applicable */
+		if (!sorcery_is_configuration_met(sorcery, type, category, config)) {
 			continue;
 		}
 
@@ -420,6 +491,24 @@
 				ao2_ref(config, -1);
 				return NULL;
 			}
+		} else if (!strcasecmp(name, "explicit_name")) {
+			ast_free(config->explicit_name);
+			config->explicit_name = ast_strdup(value);
+			if (ast_strlen_zero(config->explicit_name)) {
+				/* This is fatal since it could stop a configuration section from getting applied */
+				ast_log(LOG_ERROR, "Could not create explicit name entry of '%s' for configuration file '%s'\n",
+					value, filename);
+				ao2_ref(config, -1);
+				return NULL;
+			}
+		} else if (!strcasecmp(name, "single_object")) {
+			if (ast_strlen_zero(value)) {
+				ast_log(LOG_ERROR, "Could not set single object value for configuration file '%s' as the value is empty\n",
+					filename);
+				ao2_ref(config, -1);
+				return NULL;
+			}
+			config->single_object = ast_true(value);
 		} else {
 			ast_log(LOG_ERROR, "Unsupported option '%s' used for configuration file '%s'\n", name, filename);
 		}

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie22723663c1ddd24f869af8c9b4c1b59e2476893
Gerrit-Change-Number: 9514
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180718/0726e65d/attachment-0001.html>


More information about the asterisk-code-review mailing list