[Home]

Summary:ASTERISK-28951: Inconsistent behaviour queues.conf when there is (not) a [general] section
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2020-06-15 04:56:26Date Closed:2020-07-09 05:22:30
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:13.34.0 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Attachments:
Description:app_queue suggests these "defaults" at the top of the file:
{code}
/*! \brief queues.conf [general] option */
static int autofill_default = 1;

/*! \brief queues.conf [general] option */
static int montype_default = 0;

/*! \brief queues.conf [general] option */
static int shared_lastcall = 1;
{code}
The queue reload code contains this:
{code}
/*! Set the global queue parameters as defined in the "general" section of queues.conf */
static void queue_set_global_params(struct ast_config *cfg)
{
...
       autofill_default = 0;
       if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
               autofill_default = ast_true(general_val);
...
       shared_lastcall = 0;
       if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
               shared_lastcall = ast_true(general_val);
{code}
And that code is ran here:
{code}
static int reload_queues(int reload, struct ast_flags *mask, const char *queuename)
...
       if (!(cfg = ast_config_load("queues.conf", config_flags))) {
...
       while ((cat = ast_category_browse(cfg, cat)) ) {
               if (!strcasecmp(cat, "general") && queue_reload) {
                       queue_set_global_params(cfg);
                       continue;
{code}
That means:
- if there is a queues.conf
- it has a {{\[general]}} section (or _had_ a such a section)
- the autofill_default default is 0, and the shared_lastcall default is 0

If there is NO {{\[general]}} section:
- the autofill_default default is 1, and the shared_lastcall default is 1

That's confusing, and also, it disagrees with the sample cfg:
{noformat}
;    in most cases, you will want to enable this behavior. If you
;    do not specify or comment out this option, it will default to yes.
;
;autofill = no
...
; The default value is no.
;
;shared_lastcall=no
{noformat}

That last one was changed here:
{noformat}
commit 4b50e3f1ee84ae29da6d9eb3cfd9896a49d2394b
Author: Mark Michelson <mmichelson@digium.com>
Date:   Mon Aug 27 17:52:16 2012 +0000

   Fix incorrectly documented option in queues.conf
   
   sharedlastcall defaults to "no" not "yes"
{noformat}
But the autofill_default was not. And they still differ depending on the existence of the general section.

Suggested fix:
{noformat}
diff --git a/apps/app_queue.c b/apps/app_queue.c
index 3a572efc2d..1e786fc39f 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -1363,34 +1363,34 @@ static char *app_ql = "QueueLog" ;
static const char * const pm_family = "Queue/PersistentMembers";

/*! \brief queues.conf [general] option */
-static int queue_persistent_members = 0;
+static int queue_persistent_members;

-/*! \brief queues.conf per-queue weight option */
-static int use_weight = 0;
+/*! \brief Records that one or more queues use weight */
+static int use_weight;

/*! \brief queues.conf [general] option */
-static int autofill_default = 1;
+static int autofill_default;

/*! \brief queues.conf [general] option */
-static int montype_default = 0;
+static int montype_default;

/*! \brief queues.conf [general] option */
-static int shared_lastcall = 1;
+static int shared_lastcall;

/*! \brief queuesrules.conf [general] option */
-static int realtime_rules = 0;
+static int realtime_rules;

/*! \brief Subscription to device state change messages */
static struct stasis_subscription *device_state_sub;

/*! \brief queues.conf [general] option */
-static int update_cdr = 0;
+static int update_cdr;

/*! \brief queues.conf [general] option */
-static int negative_penalty_invalid = 0;
+static int negative_penalty_invalid;

/*! \brief queues.conf [general] option */
-static int log_membername_as_agent = 0;
+static int log_membername_as_agent;

/*! \brief name of the ringinuse field in the realtime database */
static char *realtime_ringinuse_field;
@@ -8722,14 +8722,19 @@ static struct ast_custom_function queuememberpenalty_function = {
.write = queue_function_memberpenalty_write,
};

+/*! Reset the global queue rules parameters even if there is no "general" section of queuerules.conf */
+static void queue_rules_reset_global_params()
+{
+ realtime_rules = 0;
+}
+
/*! Set the global queue rules parameters as defined in the "general" section of queuerules.conf */
static void queue_rules_set_global_params(struct ast_config *cfg)
{
-        const char *general_val = NULL;
-        realtime_rules = 0;
-        if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
-                realtime_rules = ast_true(general_val);
-        }
+ const char *general_val = NULL;
+ if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
+ realtime_rules = ast_true(general_val);
+ }
}

/*! \brief Reload the rules defined in queuerules.conf
@@ -8764,6 +8769,7 @@ static int reload_queue_rules(int reload)
ast_free(pr_iter);
ast_free(rl_iter);
}
+ queue_rules_reset_global_params();
while ((rulecat = ast_category_browse(cfg, rulecat))) {
if (!strcasecmp(rulecat, "general")) {
queue_rules_set_global_params(cfg);
@@ -8795,36 +8801,41 @@ static int reload_queue_rules(int reload)
return AST_MODULE_LOAD_SUCCESS;
}

+/*! Always set the global queue defaults, even if there is no "general" section in queues.conf */
+static void queue_reset_global_params()
+{
+ queue_persistent_members = 0;
+ autofill_default = 0;
+ montype_default = 0;
+ update_cdr = 0;
+ shared_lastcall = 0;
+ negative_penalty_invalid = 0;
+ log_membername_as_agent = 0;
+}
+
/*! Set the global queue parameters as defined in the "general" section of queues.conf */
static void queue_set_global_params(struct ast_config *cfg)
{
const char *general_val = NULL;
- queue_persistent_members = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "persistentmembers"))) {
queue_persistent_members = ast_true(general_val);
}
- autofill_default = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
autofill_default = ast_true(general_val);
}
- montype_default = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "monitor-type"))) {
if (!strcasecmp(general_val, "mixmonitor"))
montype_default = 1;
}
- update_cdr = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "updatecdr"))) {
update_cdr = ast_true(general_val);
}
- shared_lastcall = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
shared_lastcall = ast_true(general_val);
}
- negative_penalty_invalid = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "negative_penalty_invalid"))) {
negative_penalty_invalid = ast_true(general_val);
}
- log_membername_as_agent = 0;
if ((general_val = ast_variable_retrieve(cfg, "general", "log_membername_as_agent"))) {
log_membername_as_agent = ast_true(general_val);
}
@@ -9128,6 +9139,7 @@ static int reload_queues(int reload, struct ast_flags *mask, const char *queuena

/* Chug through config file. */
cat = NULL;
+ queue_reset_global_params();
while ((cat = ast_category_browse(cfg, cat)) ) {
if (!strcasecmp(cat, "general") && queue_reload) {
queue_set_global_params(cfg);
diff --git a/configs/samples/queues.conf.sample b/configs/samples/queues.conf.sample
index 5c8a80ffee..f1694929fb 100644
--- a/configs/samples/queues.conf.sample
+++ b/configs/samples/queues.conf.sample
@@ -23,7 +23,7 @@ persistentmembers = yes
;    no more available members or no more waiting callers. This is
;    probably more along the lines of how a queue should work and
;    in most cases, you will want to enable this behavior. If you
-;    do not specify or comment out this option, it will default to yes.
+;    do not specify or comment out this option, it will default to no.
;
;autofill = no
;
{noformat}
Unfortunately, this change will change things for the current users who don't have a {{\[general]}} section.

So, suggested target for this change?
- master only?
- 13 and up, adding an appropriate notice in CHANGES?
Comments:By: Asterisk Team (asteriskteam) 2020-06-15 04:56:27.013-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur.
By: Friendly Automation (friendly-automation) 2020-07-09 05:22:34.634-0500

Change 14596 merged by Joshua Colp:
app_queue: (Breaking change) shared_lastcall and autofill default to no

[https://gerrit.asterisk.org/c/asterisk/+/14596|https://gerrit.asterisk.org/c/asterisk/+/14596]