Rework system of symbols registration

It is possible now to use priorities when adding symbols to metrics and
override scores for symbols with lower priority with the scores with
high priority.

5 files changed, 140 insertions, 121 deletions

diff --git a/src/libserver/cfg_file.h b/src/libserver/cfg_file.h
index 373daf744..88b5b4538 100644
--- a/src/libserver/cfg_file.h
+++ b/src/libserver/cfg_file.h
@@ -101,6 +101,7 @@ struct rspamd_symbol_def {
 	gchar *description;
 	gdouble *weight_ptr;
 	gdouble score;
+	guint priority;
 	struct rspamd_symbols_group *gr;
 	GList *groups;
 	guint flags;
@@ -494,6 +495,7 @@ gboolean rspamd_init_filters (struct rspamd_config *cfg, bool reconfig);
 
 /**
  * Add new symbol to the metric
+ * @param cfg
  * @param metric metric's name (or NULL for the default metric)
  * @param symbol symbol's name
  * @param score symbol's score
@@ -501,12 +503,14 @@ gboolean rspamd_init_filters (struct rspamd_config *cfg, bool reconfig);
  * @param group optional group name
  * @param one_shot TRUE if symbol can add its score once
  * @param rewrite_existing TRUE if we need to rewrite the existing symbol
+ * @param priority use the following priority for a symbol
  * @return TRUE if symbol has been inserted or FALSE if `rewrite_existing` is not enabled and symbol already exists
  */
 gboolean rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
 		const gchar *metric,
 		const gchar *symbol, gdouble score, const gchar *description,
-		const gchar *group, gboolean one_shot, gboolean rewrite_existing);
+		const gchar *group, guint flags,
+		guint priority);
 
 /**
  * Checks if a specified C or lua module is enabled or disabled in the config.
diff --git a/src/libserver/cfg_rcl.c b/src/libserver/cfg_rcl.c
index 7b6d1180a..c21c8321d 100644
--- a/src/libserver/cfg_rcl.c
+++ b/src/libserver/cfg_rcl.c
@@ -316,67 +316,52 @@ rspamd_rcl_symbol_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 		struct rspamd_rcl_section *section, GError **err)
 {
 	struct rspamd_rcl_symbol_data *sd = ud;
-	struct rspamd_symbol_def *sym_def;
 	struct metric *metric;
 	struct rspamd_config *cfg;
 	const ucl_object_t *elt;
-	GList *metric_list;
+	const gchar *description = NULL;
+	gdouble score = 0.0;
+	guint priority = 1, flags = 0;
 
 	g_assert (key != NULL);
 	metric = sd->metric;
 	g_assert (metric != NULL);
 	cfg = sd->cfg;
 
-	sym_def = g_hash_table_lookup (metric->symbols, key);
-
-	if (sym_def == NULL) {
-		sym_def = rspamd_mempool_alloc0 (pool, sizeof (*sym_def));
-		sym_def->name = rspamd_mempool_strdup (pool, key);
-		sym_def->gr = sd->gr;
-		sym_def->weight_ptr = rspamd_mempool_alloc (pool, sizeof (gdouble));
-
-		g_hash_table_insert (metric->symbols, sym_def->name, sym_def);
-
-		if (sd->gr) {
-			g_hash_table_insert (sd->gr->symbols, sym_def->name, sym_def);
-		}
-		if ((metric_list =
-				g_hash_table_lookup (cfg->metrics_symbols, sym_def->name)) == NULL) {
-			metric_list = g_list_prepend (NULL, metric);
-			rspamd_mempool_add_destructor (cfg->cfg_pool,
-					(rspamd_mempool_destruct_t)g_list_free,
-					metric_list);
-			g_hash_table_insert (cfg->metrics_symbols, sym_def->name, metric_list);
-		}
-		else {
-			if (!g_list_find (metric_list, metric)) {
-				metric_list = g_list_append (metric_list, metric);
-			}
-		}
-	}
-	else {
-		msg_warn_config ("redefining symbol '%s' in metric '%s'", key, metric->name);
-	}
-
 	if ((elt = ucl_object_lookup (obj, "one_shot")) != NULL) {
 		if (ucl_object_toboolean (elt)) {
-			sym_def->flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
+			flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
 		}
 	}
 
 	if ((elt = ucl_object_lookup (obj, "ignore")) != NULL) {
 		if (ucl_object_toboolean (elt)) {
-			sym_def->flags |= RSPAMD_SYMBOL_FLAG_IGNORE;
+			flags |= RSPAMD_SYMBOL_FLAG_IGNORE;
 		}
 	}
 
-	if (!rspamd_rcl_section_parse_defaults (section, pool, obj,
-			sym_def, err)) {
-		return FALSE;
+	elt = ucl_object_lookup_any (obj, "score", "weight", NULL);
+	if (elt) {
+		score = ucl_object_todouble (elt);
+	}
+
+	elt = ucl_object_lookup (obj, "priority");
+	if (elt) {
+		priority = ucl_object_toint (elt);
+	}
+
+	elt = ucl_object_lookup (obj, "description");
+	if (elt) {
+		description = ucl_object_tostring (elt);
 	}
 
-	if (ucl_object_lookup_any (obj, "score", "weight", NULL) != NULL) {
-		*sym_def->weight_ptr = sym_def->score;
+	if (sd->gr) {
+		rspamd_config_add_metric_symbol (cfg, metric->name, key, score,
+				description, sd->gr->name, flags, priority);
+	}
+	else {
+		rspamd_config_add_metric_symbol (cfg, metric->name, key, score,
+				description, NULL, flags, priority);
 	}
 
 	return TRUE;
@@ -1790,24 +1775,6 @@ rspamd_rcl_config_init (struct rspamd_config *cfg)
 			TRUE,
 			sub->doc_ref,
 			"Symbols settings");
-	rspamd_rcl_add_default_handler (ssub,
-			"description",
-			rspamd_rcl_parse_struct_string,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, description),
-			0,
-			"Symbol's description");
-	rspamd_rcl_add_default_handler (ssub,
-			"score",
-			rspamd_rcl_parse_struct_double,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, score),
-			0,
-			"Symbol's score");
-	rspamd_rcl_add_default_handler (ssub,
-			"weight",
-			rspamd_rcl_parse_struct_double,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, score),
-			0,
-			"Symbol's score");
 
 	/* Actions part */
 	ssub = rspamd_rcl_add_section_doc (&sub->subsections,
@@ -1856,24 +1823,6 @@ rspamd_rcl_config_init (struct rspamd_config *cfg)
 			TRUE,
 			ssub->doc_ref,
 			"Symbols settings");
-	rspamd_rcl_add_default_handler (sssub,
-			"description",
-			rspamd_rcl_parse_struct_string,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, description),
-			0,
-			"Description of a symbol");
-	rspamd_rcl_add_default_handler (sssub,
-			"score",
-			rspamd_rcl_parse_struct_double,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, score),
-			0,
-			"Symbol's score");
-	rspamd_rcl_add_default_handler (sssub,
-			"weight",
-			rspamd_rcl_parse_struct_double,
-			G_STRUCT_OFFSET (struct rspamd_symbol_def, score),
-			0,
-			"Symbol's score");
 
 	/**
 	 * Worker section
diff --git a/src/libserver/cfg_utils.c b/src/libserver/cfg_utils.c
index a857571c4..a9ccb4231 100644
--- a/src/libserver/cfg_utils.c
+++ b/src/libserver/cfg_utils.c
@@ -1298,39 +1298,17 @@ rspamd_init_filters (struct rspamd_config *cfg, bool reconfig)
 	return rspamd_init_lua_filters (cfg);
 }
 
-gboolean
-rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
-		const gchar *metric_name, const gchar *symbol,
+static void
+rspamd_config_new_metric_symbol (struct rspamd_config *cfg,
+		struct metric *metric, const gchar *symbol,
 		gdouble score, const gchar *description, const gchar *group,
-		gboolean one_shot, gboolean rewrite_existing)
+		guint flags, guint priority)
 {
 	struct rspamd_symbols_group *sym_group;
 	struct rspamd_symbol_def *sym_def;
 	GList *metric_list;
-	struct metric *metric;
 	gdouble *score_ptr;
 
-	g_assert (cfg != NULL);
-	g_assert (symbol != NULL);
-
-	if (metric_name == NULL) {
-		metric_name = DEFAULT_METRIC;
-	}
-
-	metric = g_hash_table_lookup (cfg->metrics, metric_name);
-
-	if (metric == NULL) {
-		msg_err_config ("metric %s has not been found", metric_name);
-		return FALSE;
-	}
-
-	if (g_hash_table_lookup (cfg->metrics_symbols, symbol) != NULL &&
-			!rewrite_existing) {
-		msg_debug_config ("symbol %s has been already registered, do not override",
-				symbol);
-		return FALSE;
-	}
-
 	sym_def =
 		rspamd_mempool_alloc0 (cfg->cfg_pool, sizeof (struct rspamd_symbol_def));
 	score_ptr = rspamd_mempool_alloc (cfg->cfg_pool, sizeof (gdouble));
@@ -1339,10 +1317,8 @@ rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
 	sym_def->score = score;
 	sym_def->weight_ptr = score_ptr;
 	sym_def->name = rspamd_mempool_strdup (cfg->cfg_pool, symbol);
-
-	if (one_shot) {
-		sym_def->flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
-	}
+	sym_def->priority = priority;
+	sym_def->flags = flags;
 
 	if (description) {
 		sym_def->description = rspamd_mempool_strdup (cfg->cfg_pool, description);
@@ -1354,11 +1330,11 @@ rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
 	g_hash_table_insert (metric->symbols, sym_def->name, sym_def);
 
 	if ((metric_list =
-		g_hash_table_lookup (cfg->metrics_symbols, sym_def->name)) == NULL) {
+			g_hash_table_lookup (cfg->metrics_symbols, sym_def->name)) == NULL) {
 		metric_list = g_list_prepend (NULL, metric);
 		rspamd_mempool_add_destructor (cfg->cfg_pool,
-			(rspamd_mempool_destruct_t)g_list_free,
-			metric_list);
+				(rspamd_mempool_destruct_t)g_list_free,
+				metric_list);
 		g_hash_table_insert (cfg->metrics_symbols, sym_def->name, metric_list);
 	}
 	else {
@@ -1381,6 +1357,77 @@ rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
 
 	sym_def->gr = sym_group;
 	g_hash_table_insert (sym_group->symbols, sym_def->name, sym_def);
+}
+
+
+gboolean
+rspamd_config_add_metric_symbol (struct rspamd_config *cfg,
+		const gchar *metric_name, const gchar *symbol,
+		gdouble score, const gchar *description, const gchar *group,
+		guint flags, guint priority)
+{
+	struct rspamd_symbol_def *sym_def;
+	struct metric *metric;
+
+	g_assert (cfg != NULL);
+	g_assert (symbol != NULL);
+
+	if (metric_name == NULL) {
+		metric_name = DEFAULT_METRIC;
+	}
+
+	metric = g_hash_table_lookup (cfg->metrics, metric_name);
+
+	if (metric == NULL) {
+		msg_err_config ("metric %s has not been found", metric_name);
+		return FALSE;
+	}
+
+	sym_def = g_hash_table_lookup (cfg->metrics_symbols, symbol);
+
+	if (sym_def != NULL) {
+		if (sym_def->priority >= priority) {
+			msg_info_config ("symbol %s has been already registered with"
+					"priority %ud, do not override (new priority: %ud)",
+					symbol,
+					sym_def->priority,
+					priority);
+			/* But we can still add description */
+			if (!sym_def->description && description) {
+				sym_def->description = rspamd_mempool_strdup (cfg->cfg_pool,
+						description);
+			}
+
+			return FALSE;
+		}
+		else {
+			msg_info_config ("symbol %s has been already registered with"
+					"priority %ud, override it with new priority: %ud, "
+					"old score: %.2f, new score: %.2f",
+					symbol,
+					sym_def->priority,
+					priority,
+					sym_def->score,
+					score);
+
+			*sym_def->weight_ptr = score;
+			sym_def->score = score;
+
+			sym_def->flags = flags;
+
+			if (description) {
+				sym_def->description = rspamd_mempool_strdup (cfg->cfg_pool,
+						description);
+			}
+
+			sym_def->priority = priority;
+
+			return TRUE;
+		}
+	}
+
+	rspamd_config_new_metric_symbol (cfg, metric, symbol, score, description,
+			group, flags, priority);
 
 	return TRUE;
 }
diff --git a/src/lua/lua_config.c b/src/lua/lua_config.c
index 4421bf179..59ca08864 100644
--- a/src/lua/lua_config.c
+++ b/src/lua/lua_config.c
@@ -1282,15 +1282,17 @@ lua_config_set_metric_symbol (lua_State * L)
 	struct metric *metric;
 	gboolean one_shot = FALSE;
 	GError *err = NULL;
+	gdouble priority = 0.0;
+	guint flags = 0;
 
 	if (cfg) {
 
 		if (lua_type (L, 2) == LUA_TTABLE) {
 			if (!rspamd_lua_parse_table_arguments (L, 2, &err,
 					"*name=S;score=N;description=S;"
-					"group=S;one_shot=B;metric=S",
+					"group=S;one_shot=B;metric=S;priority=N",
 					&name, &weight, &description,
-					&group, &one_shot, &metric_name)) {
+					&group, &one_shot, &metric_name, &priority)) {
 				msg_err_config ("bad arguments: %e", err);
 				g_error_free (err);
 
@@ -1320,13 +1322,16 @@ lua_config_set_metric_symbol (lua_State * L)
 		}
 
 		metric = g_hash_table_lookup (cfg->metrics, metric_name);
+		if (one_shot) {
+			flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
+		}
 
 		if (metric == NULL) {
 			msg_err_config ("metric named %s is not defined", metric_name);
 		}
 		else if (name != NULL && weight != 0) {
 			rspamd_config_add_metric_symbol (cfg, metric_name, name,
-					weight, description, group, one_shot, FALSE);
+					weight, description, group, flags, (guint)priority);
 		}
 	}
 
@@ -1410,7 +1415,7 @@ lua_config_newindex (lua_State *L)
 			gint type = SYMBOL_TYPE_NORMAL, priority = 0, idx;
 			gdouble weight = 1.0, score;
 			const char *type_str, *group = NULL, *description = NULL;
-			gboolean one_shot = FALSE;
+			guint flags = 0;
 
 			/*
 			 * Table can have the following attributes:
@@ -1533,16 +1538,18 @@ lua_config_newindex (lua_State *L)
 					lua_gettable (L, -2);
 
 					if (lua_type (L, -1) == LUA_TBOOLEAN) {
-						one_shot = lua_toboolean (L, -1);
+						if (lua_toboolean (L, -1)) {
+							flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
+						}
 					}
 					lua_pop (L, 1);
 
 					/*
-					 * Do not override the existing symbols, since we are
-					 * having default values here
+					 * Do not override the existing symbols (using zero priority),
+					 * since we are defining default values here
 					 */
 					rspamd_config_add_metric_symbol (cfg, NULL, name, score,
-							description, group, one_shot, FALSE);
+							description, group, flags, 0);
 				}
 				else {
 					lua_pop (L, 1);
@@ -1553,6 +1560,9 @@ lua_config_newindex (lua_State *L)
 			lua_pop (L, 1);
 		}
 	}
+	else {
+		return luaL_error (L, "invalid arguments");
+	}
 
 	return 0;
 }
diff --git a/src/plugins/regexp.c b/src/plugins/regexp.c
index 6458e1577..d80b11f1f 100644
--- a/src/plugins/regexp.c
+++ b/src/plugins/regexp.c
@@ -184,7 +184,8 @@ regexp_module_config (struct rspamd_config *cfg)
 			const gchar *description = NULL, *group = NULL,
 					*metric = DEFAULT_METRIC;
 			gdouble score = 0.0;
-			gboolean one_shot = FALSE, is_lua = FALSE, valid_expression = TRUE;
+			guint flags = 0, priority = 0;
+			gboolean is_lua = FALSE, valid_expression = TRUE;
 
 			/* We have some lua table, extract its arguments */
 			elt = ucl_object_lookup (value, "callback");
@@ -269,11 +270,19 @@ regexp_module_config (struct rspamd_config *cfg)
 				elt = ucl_object_lookup (value, "one_shot");
 
 				if (elt) {
-					one_shot = ucl_object_toboolean (elt);
+					if (ucl_object_toboolean (elt)) {
+						flags |= RSPAMD_SYMBOL_FLAG_ONESHOT;
+					}
+				}
+
+				elt = ucl_object_lookup (value, "priority");
+
+				if (elt) {
+					priority = ucl_object_toint (elt);
 				}
 
 				rspamd_config_add_metric_symbol (cfg, metric, cur_item->symbol,
-						score, description, group, one_shot, FALSE);
+						score, description, group, flags, priority);
 			}
 		}
 		else {