From 2dd919171853b7408a2d32aa0282070f80781047 Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Tue, 2 May 2023 11:56:01 +0200
Subject: [PATCH] refactor: use different terms for config tombstone

there are 3 different kind of Erlang terms for tombstone related configs
1. the schema type (must be an atom)
2. the config value (must be a binary)
3. the config change comamnd (request) which is only used
   in the code, but never persisted
---
 apps/emqx/include/emqx_schema.hrl     | 5 +++--
 apps/emqx/src/emqx_config_handler.erl | 3 ++-
 apps/emqx/src/emqx_listeners.erl      | 7 +++----
 apps/emqx/src/emqx_schema.erl         | 7 +++----
 apps/emqx_conf/src/emqx_conf.erl      | 3 ++-
 5 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/apps/emqx/include/emqx_schema.hrl b/apps/emqx/include/emqx_schema.hrl
index 7f96e15a9..307bb20c5 100644
--- a/apps/emqx/include/emqx_schema.hrl
+++ b/apps/emqx/include/emqx_schema.hrl
@@ -16,7 +16,8 @@
 -ifndef(EMQX_SCHEMA_HRL).
 -define(EMQX_SCHEMA_HRL, true).
 
--define(TOMBSTONE, marked_for_deletion).
--define(TOMBSTONE_BIN, <<"marked_for_deletion">>).
+-define(TOMBSTONE_TYPE, marked_for_deletion).
+-define(TOMBSTONE_VALUE, <<"marked_for_deletion">>).
+-define(TOMBSTONE_CONFIG_CHANGE_REQ, mark_it_for_deletion).
 
 -endif.
diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl
index adbba032a..0bad19f9e 100644
--- a/apps/emqx/src/emqx_config_handler.erl
+++ b/apps/emqx/src/emqx_config_handler.erl
@@ -18,6 +18,7 @@
 -module(emqx_config_handler).
 
 -include("logger.hrl").
+-include("emqx_schema.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
 
 -behaviour(gen_server).
@@ -448,7 +449,7 @@ up_req({remove, _Opts}) -> '$remove';
 up_req({{update, Req}, _Opts}) -> Req.
 
 return_change_result(ConfKeyPath, {{update, Req}, Opts}) ->
-    case Req =/= emqx_schema:tombstone() of
+    case Req =/= ?TOMBSTONE_CONFIG_CHANGE_REQ of
         true ->
             #{
                 config => emqx_config:get(ConfKeyPath),
diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl
index 769e18e4d..b3043effc 100644
--- a/apps/emqx/src/emqx_listeners.erl
+++ b/apps/emqx/src/emqx_listeners.erl
@@ -67,8 +67,7 @@
 
 -define(CONF_KEY_PATH, [listeners, '?', '?']).
 -define(TYPES_STRING, ["tcp", "ssl", "ws", "wss", "quic"]).
--define(MARK_DEL, ?TOMBSTONE).
--define(MARK_DEL_BIN, ?TOMBSTONE_BIN).
+-define(MARK_DEL, ?TOMBSTONE_CONFIG_CHANGE_REQ).
 
 -spec id_example() -> atom().
 id_example() -> 'tcp:default'.
@@ -429,7 +428,7 @@ do_start_listener(quic, ListenerName, #{bind := Bind} = Opts) ->
 
 %% Update the listeners at runtime
 pre_config_update([listeners, Type, Name], {create, NewConf}, V) when
-    V =:= undefined orelse V =:= ?MARK_DEL_BIN
+    V =:= undefined orelse V =:= ?TOMBSTONE_VALUE
 ->
     CertsDir = certs_dir(Type, Name),
     {ok, convert_certs(CertsDir, NewConf)};
@@ -446,7 +445,7 @@ pre_config_update([listeners, _Type, _Name], {action, _Action, Updated}, RawConf
     NewConf = emqx_utils_maps:deep_merge(RawConf, Updated),
     {ok, NewConf};
 pre_config_update([listeners, _Type, _Name], ?MARK_DEL, _RawConf) ->
-    {ok, ?MARK_DEL};
+    {ok, ?TOMBSTONE_VALUE};
 pre_config_update(_Path, _Request, RawConf) ->
     {ok, RawConf}.
 
diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl
index f33dcb979..cf0f05abd 100644
--- a/apps/emqx/src/emqx_schema.erl
+++ b/apps/emqx/src/emqx_schema.erl
@@ -103,7 +103,6 @@
 
 %% tombstone types
 -export([
-    tombstone/0,
     tombstone_map/2,
     get_tombstone_map_value_type/1
 ]).
@@ -3194,7 +3193,7 @@ special_env(_Name) -> error.
 
 %% The tombstone atom.
 tombstone() ->
-    ?TOMBSTONE.
+    ?TOMBSTONE_TYPE.
 
 %% Make a map type, the value of which is allowed to be 'marked_for_deletion'
 %% 'marked_for_delition' is a special value which means the key is deleted.
@@ -3203,7 +3202,7 @@ tombstone() ->
 tombstone_map(Name, Type) ->
     %% marked_for_deletion must be the last member of the union
     %% because we need to first union member to populate the default values
-    map(Name, ?UNION([Type, tombstone()])).
+    map(Name, ?UNION([Type, ?TOMBSTONE_TYPE])).
 
 %% inverse of mark_del_map
 get_tombstone_map_value_type(Schema) ->
@@ -3220,7 +3219,7 @@ get_tombstone_map_value_type(Schema) ->
 keep_default_tombstone(Map, _Opts) when is_map(Map) ->
     maps:filter(
         fun(Key, Value) ->
-            Key =:= <<"default">> orelse Value =/= ?TOMBSTONE_BIN
+            Key =:= <<"default">> orelse Value =/= ?TOMBSTONE_VALUE
         end,
         Map
     );
diff --git a/apps/emqx_conf/src/emqx_conf.erl b/apps/emqx_conf/src/emqx_conf.erl
index 1b37f652b..28fcaf624 100644
--- a/apps/emqx_conf/src/emqx_conf.erl
+++ b/apps/emqx_conf/src/emqx_conf.erl
@@ -18,6 +18,7 @@
 -compile({no_auto_import, [get/1, get/2]}).
 -include_lib("emqx/include/logger.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
+-include_lib("emqx/include/emqx_schema.hrl").
 
 -export([add_handler/2, remove_handler/1]).
 -export([get/1, get/2, get_raw/1, get_raw/2, get_all/1]).
@@ -110,7 +111,7 @@ update(Node, KeyPath, UpdateReq, Opts) ->
 
 %% @doc Mark the specified key path as tombstone
 tombstone(KeyPath, Opts) ->
-    update(KeyPath, emqx_schema:tombstone(), Opts).
+    update(KeyPath, ?TOMBSTONE_CONFIG_CHANGE_REQ, Opts).
 
 %% @doc remove all value of key path in cluster-override.conf or local-override.conf.
 -spec remove(emqx_utils_maps:config_key_path(), emqx_config:update_opts()) ->