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;
}
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
svn-commits mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/svn-commits
