[asterisk-commits] dlee: branch dlee/amqp-cdr-cel r431575 - in /team/dlee/amqp-cdr-cel: cdr/ cel...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Feb 5 19:08:10 CST 2015


Author: dlee
Date: Thu Feb  5 19:08:07 2015
New Revision: 431575

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=431575
Log:
Address review feedback.

Basically use pre_apply_config to validate config properly.

Modified:
    team/dlee/amqp-cdr-cel/cdr/cdr_amqp.c
    team/dlee/amqp-cdr-cel/cel/cel_amqp.c
    team/dlee/amqp-cdr-cel/res/amqp/config.c

Modified: team/dlee/amqp-cdr-cel/cdr/cdr_amqp.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/amqp-cdr-cel/cdr/cdr_amqp.c?view=diff&rev=431575&r1=431574&r2=431575
==============================================================================
--- team/dlee/amqp-cdr-cel/cdr/cdr_amqp.c (original)
+++ team/dlee/amqp-cdr-cel/cdr/cdr_amqp.c Thu Feb  5 19:08:07 2015
@@ -37,7 +37,7 @@
 				<configOption name="loguniqueid">
 					<synopsis>Determines whether to log the uniqueid for calls</synopsis>
 					<description>
-						<para>Defaults is no.</para>
+						<para>Default is no.</para>
 					</description>
 				</configOption>
 				<configOption name="loguserfield">
@@ -178,8 +178,38 @@
 	return ao2_bump(conf);
 }
 
+static int setup_amqp(void);
+
 CONFIG_INFO_STANDARD(cfg_info, confs, conf_alloc,
-		     .files = ACO_FILES(&conf_file));
+	.files = ACO_FILES(&conf_file),
+	.pre_apply_config = setup_amqp,
+);
+
+static int setup_amqp(void)
+{
+	struct cdr_amqp_conf *conf = aco_pending_config(&cfg_info);
+
+	if (!conf) {
+		return 0;
+	}
+
+	if (!conf->global) {
+		ast_log(LOG_ERROR, "Invalid cdr_amqp.conf\n");
+		return -1;
+	}
+
+	/* Refresh the AMQP connection */
+	ao2_cleanup(conf->global->amqp);
+	conf->global->amqp = ast_amqp_get_connection(conf->global->connection);
+
+	if (!conf->global->amqp) {
+		ast_log(LOG_ERROR, "Could not get AMQP connection %s\n",
+			conf->global->connection);
+		return -1;
+	}
+
+	return 0;
+}
 
 /*!
  * \brief CDR handler for AMQP.
@@ -202,15 +232,8 @@
 	};
 
 	conf = ao2_global_obj_ref(confs);
-	if (!conf || !conf->global) {
-		ast_log(LOG_ERROR, "Error obtaining config from cdr_amqp.conf\n");
-		return -1;
-	}
-
-	if (!conf->global->amqp) {
-		ast_log(LOG_ERROR, "No AMQP connection; discarding CDR\n");
-		return -1;
-	}
+
+	ast_assert(conf && conf->global && conf->global->amqp);
 
 	json = ast_json_pack("{"
 		/* clid, src, dst, dcontext */
@@ -301,16 +324,6 @@
 	conf = ao2_global_obj_ref(confs);
 	if (!conf || !conf->global) {
 		ast_log(LOG_ERROR, "Error obtaining config from cdr_amqp.conf\n");
-		return -1;
-	}
-
-	/* Refresh the AMQP connection */
-	ao2_cleanup(conf->global->amqp);
-	conf->global->amqp = ast_amqp_get_connection(conf->global->connection);
-
-	if (!conf->global->amqp) {
-		ast_log(LOG_ERROR, "Could not get AMQP connection %s\n",
-			conf->global->connection);
 		return -1;
 	}
 

Modified: team/dlee/amqp-cdr-cel/cel/cel_amqp.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/amqp-cdr-cel/cel/cel_amqp.c?view=diff&rev=431575&r1=431574&r2=431575
==============================================================================
--- team/dlee/amqp-cdr-cel/cel/cel_amqp.c (original)
+++ team/dlee/amqp-cdr-cel/cel/cel_amqp.c Thu Feb  5 19:08:07 2015
@@ -163,8 +163,38 @@
 	return ao2_bump(conf);
 }
 
+static int setup_amqp(void);
+
 CONFIG_INFO_STANDARD(cfg_info, confs, conf_alloc,
-		     .files = ACO_FILES(&conf_file));
+	.files = ACO_FILES(&conf_file),
+	.pre_apply_config = setup_amqp,
+);
+
+static int setup_amqp(void)
+{
+	struct cel_amqp_conf *conf = aco_pending_config(&cfg_info);
+
+	if (!conf) {
+		return 0;
+	}
+
+	if (!conf->global) {
+		ast_log(LOG_ERROR, "Invalid cdr_amqp.conf\n");
+		return -1;
+	}
+
+	/* Refresh the AMQP connection */
+	ao2_cleanup(conf->global->amqp);
+	conf->global->amqp = ast_amqp_get_connection(conf->global->connection);
+
+	if (!conf->global->amqp) {
+		ast_log(LOG_ERROR, "Could not get AMQP connection %s\n",
+			conf->global->connection);
+		return -1;
+	}
+
+	return 0;
+}
 
 /*!
  * \brief CEL handler for AMQP.
@@ -189,15 +219,8 @@
 	};
 
 	conf = ao2_global_obj_ref(confs);
-	if (!conf || !conf->global) {
-		ast_log(LOG_ERROR, "Error obtaining config from cel_amqp.conf\n");
-		return;
-	}
-
-	if (!conf->global->amqp) {
-		ast_log(LOG_ERROR, "No AMQP connection; discarding CEL\n");
-		return;
-	}
+
+	ast_assert(conf && conf->global && conf->global->amqp);
 
 	/* Extract the data from the CEL */
 	if (ast_cel_fill_record(event, &record) != 0) {

Modified: team/dlee/amqp-cdr-cel/res/amqp/config.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/amqp-cdr-cel/res/amqp/config.c?view=diff&rev=431575&r1=431574&r2=431575
==============================================================================
--- team/dlee/amqp-cdr-cel/res/amqp/config.c (original)
+++ team/dlee/amqp-cdr-cel/res/amqp/config.c Thu Feb  5 19:08:07 2015
@@ -105,8 +105,75 @@
 	return ao2_bump(conf);
 }
 
+static int validate_connections(void);
+
 CONFIG_INFO_STANDARD(cfg_info, confs, conf_alloc,
-		     .files = ACO_FILES(&conf_file));
+	.files = ACO_FILES(&conf_file),
+	.pre_apply_config = validate_connections,
+);
+
+static int validate_connection_cb(void *obj, void *arg, int flags)
+{
+        struct amqp_conf_connection *cxn_conf = obj;
+	int *validation_res = arg;
+
+	/* Copy the URL, so we can copy it non-destructively */
+	cxn_conf->parsed_url = ast_strdup(cxn_conf->url);
+	if (!cxn_conf->parsed_url) {
+		*validation_res = -1;
+		return -1;
+	}
+
+	amqp_default_connection_info(&cxn_conf->connection_info);
+	if (amqp_parse_url(cxn_conf->parsed_url, &cxn_conf->connection_info) != AMQP_STATUS_OK) {
+		ast_log(LOG_ERROR, "%s: invalid url %s\n",
+			cxn_conf->name, cxn_conf->url);
+		*validation_res = -1;
+		return -1;
+	}
+
+	/* While this could be intentional, this is probably an error */
+	if (strlen(cxn_conf->connection_info.vhost) == 0) {
+		ast_log(LOG_WARNING, "%s: vhost in url is blank\n",
+			cxn_conf->url);
+	}
+
+	if (cxn_conf->max_frame_bytes < AMQP_FRAME_MIN_SIZE) {
+		ast_log(LOG_WARNING, "%s: invalid max_frame_bytes %d\n",
+			cxn_conf->name, cxn_conf->max_frame_bytes);
+		cxn_conf->max_frame_bytes = AMQP_FRAME_MIN_SIZE;
+	}
+
+	if (cxn_conf->heartbeat_seconds < 0) {
+		ast_log(LOG_WARNING, "%s: invalid heartbeat_seconds %d\n",
+			cxn_conf->name, cxn_conf->heartbeat_seconds);
+		cxn_conf->heartbeat_seconds = 0;
+	}
+
+	return 0;
+}
+
+static int validate_connections(void)
+{
+	int validation_res = 0;
+
+	struct amqp_conf *conf = aco_pending_config(&cfg_info);
+	if (!conf) {
+		ast_log(LOG_ERROR, "Error obtaining config from amqp.conf\n");
+		return 0;
+	}
+
+	if (!conf->general->enabled) {
+		ast_log(LOG_NOTICE, "AMQP disabled\n");
+		return 0;
+	}
+
+	ast_debug(3, "Building %d AMQP connections\n",
+		ao2_container_count(conf->connections));
+	ao2_callback(conf->connections, OBJ_NODATA, validate_connection_cb, &validation_res);
+
+	return validation_res;
+}
 
 static void amqp_conf_connection_dtor(void *obj)
 {
@@ -137,11 +204,6 @@
 	}
 
 	if (ast_string_field_set(cxn_conf, name, cat) != 0) {
-		return NULL;
-	}
-
-	if (aco_set_defaults(&connection_option, "connection", cxn_conf) != 0) {
-		ast_log(LOG_ERROR, "Error setting defaults on %s\n", cat);
 		return NULL;
 	}
 
@@ -176,47 +238,8 @@
 	}
 }
 
-static int validate_connection_cb(void *obj, void *arg, int flags)
-{
-        struct amqp_conf_connection *cxn_conf = obj;
-
-	/* Copy the URL, so we can copy it non-destructively */
-	cxn_conf->parsed_url = ast_strdup(cxn_conf->url);
-	if (!cxn_conf->parsed_url) {
-		return -1;
-	}
-
-	amqp_default_connection_info(&cxn_conf->connection_info);
-	if (amqp_parse_url(cxn_conf->parsed_url, &cxn_conf->connection_info) != AMQP_STATUS_OK) {
-		ast_log(LOG_ERROR, "%s: invalid url %s\n",
-			cxn_conf->name, cxn_conf->url);
-		return -1;
-	}
-
-	if (strlen(cxn_conf->connection_info.vhost) == 0) {
-		ast_log(LOG_WARNING, "%s: vhost in url is blank\n",
-			cxn_conf->url);
-	}
-
-	if (cxn_conf->max_frame_bytes < AMQP_FRAME_MIN_SIZE) {
-		ast_log(LOG_WARNING, "%s: invalid max_frame_bytes %d\n",
-			cxn_conf->name, cxn_conf->max_frame_bytes);
-		cxn_conf->max_frame_bytes = AMQP_FRAME_MIN_SIZE;
-	}
-
-	if (cxn_conf->heartbeat_seconds < 0) {
-		ast_log(LOG_WARNING, "%s: invalid heartbeat_seconds %d\n",
-			cxn_conf->name, cxn_conf->heartbeat_seconds);
-		cxn_conf->heartbeat_seconds = 0;
-	}
-
-	return 0;
-}
-
 static int process_config(int reload)
 {
-	RAII_VAR(struct amqp_conf *, conf, NULL, ao2_cleanup);
-
 	switch (aco_process_config(&cfg_info, reload)) {
 	case ACO_PROCESS_ERROR:
 		return -1;
@@ -224,20 +247,6 @@
 	case ACO_PROCESS_UNCHANGED:
 		break;
 	}
-
-	conf = amqp_config_get();
-	if (!conf) {
-		ast_log(LOG_ERROR, "Error obtaining config from amqp.conf\n");
-	}
-
-	if (!conf->general->enabled) {
-		ast_log(LOG_NOTICE, "AMQP disabled\n");
-		return 0;
-	}
-
-	ast_debug(3, "Building %d AMQP connections\n",
-		ao2_container_count(conf->connections));
-	ao2_callback(conf->connections, OBJ_NODATA, validate_connection_cb, NULL);
 
 	return 0;
 }




More information about the asterisk-commits mailing list