From 0b11ab2d596dfa0a94b092c871aef8ddeb8e816f Mon Sep 17 00:00:00 2001
From: Zaiming Shi <zmstone@gmail.com>
Date: Wed, 8 Sep 2021 11:01:07 +0200
Subject: [PATCH] refactor(schema): reorder config root fields

so more important configs are ordered before less important
---
 apps/emqx/src/emqx_schema.erl                 | 95 ++++++++++++-------
 apps/emqx_machine/src/emqx_machine_schema.erl | 17 +++-
 2 files changed, 73 insertions(+), 39 deletions(-)

diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl
index f9ee7e7e0..e9a4385c7 100644
--- a/apps/emqx/src/emqx_schema.erl
+++ b/apps/emqx/src/emqx_schema.erl
@@ -69,56 +69,82 @@
                 cipher/0,
                 comma_separated_atoms/0]).
 
--export([namespace/0, roots/0, fields/1]).
+-export([namespace/0, roots/0, roots/1, fields/1]).
 -export([conf_get/2, conf_get/3, keys/2, filter/1]).
 -export([ssl/1]).
 
 namespace() -> undefined.
 
 roots() ->
+    %% TODO change config importance to a field metadata
+    roots(high) ++ roots(medium) ++ roots(low).
+
+roots(high) ->
     [ {"listeners",
       sc(ref("listeners"),
          #{ desc => "MQTT listeners identified by their protocol type and assigned names"
           })
-     },
-     {"zones",
-      sc(map("name", ref("zone")),
-         #{ desc => "A zone is a set of configs grouped by the zone <code>name</code>. <br>"
-                    "For flexible configuration mapping, the <code>name</code> "
-                    "can be set to a listener's <code>zone</code> config . <br>"
-                    "NOTE: A builtin zone named <code>default</code> is auto created "
-                    "and can not be deleted."
-          })},
-     {"mqtt",
-      sc(ref("mqtt"),
+      }
+    , {"zones",
+       sc(map("name", ref("zone")),
+          #{ desc => "A zone is a set of configs grouped by the zone <code>name</code>. <br>"
+                     "For flexible configuration mapping, the <code>name</code> "
+                     "can be set to a listener's <code>zone</code> config . <br>"
+                     "NOTE: A builtin zone named <code>default</code> is auto created "
+                     "and can not be deleted."
+           })}
+    , {"mqtt",
+       sc(ref("mqtt"),
          #{ desc => "Global MQTT configuration.<br>"
                     "The configs here work as default values which can be overriden "
                     "in <code>zone</code> configs"
-          })},
-     "rate_limit",
-     "force_shutdown",
-     "force_gc",
-     "conn_congestion",
-     "quota",
-     "broker",
-     "plugins", %% TODO: move to emqx_machine_schema
-     "stats",
-     "sysmon",
-     "alarm",
-     {"authentication",
+          })}
+    , {"authentication",
       sc(hoconsc:lazy(hoconsc:array(map())),
          #{ desc => "Default authentication configs for all MQTT listeners.<br>"
                     "For per-listener overrides see <code>authentication</code> "
                     "in listener configs"
-          })},
-     {"authentication",
-      sc(hoconsc:lazy(hoconsc:array(map())),
-         #{ desc => "Default authentication configs for all MQTT listeners.<br>"
-                    "For per-listener overrides see <code>authentication</code> "
-                    "in listener configs"
-          })},
-     "authorization",
-     "flapping_detect"
+          })}
+    , {"authorization",
+       sc(ref("authorization"),
+          #{})}
+    ];
+roots(medium) ->
+    [ {"broker",
+       sc(ref("broker"),
+         #{})}
+    , {"rate_limit",
+       sc(ref("rate_limit"),
+          #{})}
+    , {"force_shutdown",
+       sc(ref("force_shutdown"),
+          #{})}
+    ];
+roots(low) ->
+    [ {"force_gc",
+       sc(ref("force_gc"),
+          #{})}
+   , {"conn_congestion",
+       sc(ref("conn_congestion"),
+          #{})}
+   , {"quota",
+       sc(ref("quota"),
+          #{})}
+   , {"plugins", %% TODO: move to emqx_machine_schema
+       sc(ref("plugins"),
+          #{})}
+   , {"stats",
+       sc(ref("stats"),
+          #{})}
+   , {"sysmon",
+       sc(ref("sysmon"),
+          #{})}
+   , {"alarm",
+       sc(ref("alarm"),
+          #{})}
+   , {"flapping_detect",
+       sc(ref("flapping_detect"),
+          #{})}
     ].
 
 fields("stats") ->
@@ -140,8 +166,7 @@ fields("authorization") ->
     , {"cache",
        sc(ref(?MODULE, "cache"),
           #{
-           })
-       }
+           })}
     ];
 
 fields("cache") ->
diff --git a/apps/emqx_machine/src/emqx_machine_schema.erl b/apps/emqx_machine/src/emqx_machine_schema.erl
index 5ef9df7c6..e4f7a8d1a 100644
--- a/apps/emqx_machine/src/emqx_machine_schema.erl
+++ b/apps/emqx_machine/src/emqx_machine_schema.erl
@@ -42,8 +42,7 @@
 %% The list can not be made a dynamic read at run-time as it is used
 %% by nodetool to generate app.<time>.config before EMQ X is started
 -define(MERGED_CONFIGS,
-        [ emqx_schema
-        , emqx_bridge_schema
+        [ emqx_bridge_schema
         , emqx_retainer_schema
         , emqx_statsd_schema
         , emqx_authz_schema
@@ -59,7 +58,8 @@
 namespace() -> undefined.
 
 roots() ->
-    lists:flatmap(fun roots/1, ?MERGED_CONFIGS) ++
+    %% authorization configs are merged in THIS schema's "authorization" fields
+    lists:keydelete("authorization", 1, emqx_schema:roots(high)) ++
     [ {"node",
        sc(hoconsc:ref("node"),
           #{ desc => "Node name, cookie, config & data directories "
@@ -84,7 +84,16 @@ roots() ->
                      "should work, but in case you need to do performance "
                      "fine-turning or experiment a bit, this is where to look."
            })}
-    ].
+    , {"authorization",
+       sc(hoconsc:ref("authorization"),
+          #{ desc => "In EMQ X, MQTT client access control can be just a few "
+                     "lines of text based rules, or delegated to an external "
+                     "HTTP API, or base externa database query results."
+           })}
+    ] ++
+    emqx_schema:roots(medium) ++
+    emqx_schema:roots(low) ++
+    lists:flatmap(fun roots/1, ?MERGED_CONFIGS).
 
 fields("cluster") ->
     [ {"name",