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.

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.

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

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;
 }

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;
 }

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 {