Merge pull request #3387 from esl/static-module-config

Simplify MUC Light config
esl · Nov 8, 2021 · ef89bef · ef89bef
2 parents 1ec0530 + fb042ee
commit ef89bef
Show file tree

Hide file tree

Showing 10 changed files with 148 additions and 327 deletions.
diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl
@@ -90,10 +90,11 @@
                            stanza_create_room/3,
                            create_room/6,
                            stanza_aff_set/2,
-                           default_config/0,
                            user_leave/3,
                            set_mod_config/3,
-                           stanza_blocking_set/1
+                           stanza_blocking_set/1,
+                           default_config/1,
+                           default_schema/0
 ]).
 
 -include("muc_light.hrl").
@@ -103,9 +104,6 @@
 
 -define(MUCHOST, (muc_light_helper:muc_host())).
 
--define(CHECK_FUN, fun mod_muc_light_room:participant_limit_check/2).
--define(BACKEND, mod_muc_light_db_backend).
-
 -type ct_aff_user() :: {EscalusClient :: escalus:client(), Aff :: atom()}.
 -type ct_aff_users() :: [ct_aff_user()].
 -type ct_block_item() :: muc_light_helper:ct_block_item().
@@ -252,7 +250,7 @@ init_per_testcase(disco_rooms_rsm, Config) ->
 init_per_testcase(CaseName, Config) ->
     set_default_mod_config(),
     case lists:member(CaseName, ?CUSTOM_CONFIG_CASES) of
-        true -> set_custom_config(common_custom_config());
+        true -> set_config_schema(custom_schema());
         _ -> ok
     end,
     case lists:member(CaseName, ?ROOM_LESS_CASES) of
@@ -268,7 +266,7 @@ end_per_testcase(CaseName, Config) when CaseName =:= disco_features_with_mam;
     escalus:end_per_testcase(CaseName, Config);
 end_per_testcase(CaseName, Config) ->
     case lists:member(CaseName, ?CUSTOM_CONFIG_CASES) of
-        true -> set_custom_config(default_schema_definition());
+        true -> set_config_schema(default_schema());
         _ -> ok
     end,
     muc_light_helper:clear_db(),
@@ -437,7 +435,7 @@ rooms_in_rosters(Config) ->
                        RosterResult, [{element, <<"query">>}, {element, <<"item">>}]),
             ProperJID = room_bin_jid(?ROOM),
             ProperJID = exml_query:attr(Item, <<"jid">>),
-            ProperName = proplists:get_value(roomname, default_config()),
+            ProperName = proplists:get_value(<<"roomname">>, default_config(default_schema())),
             ProperName = exml_query:attr(Item, <<"name">>),
             ProperVer = ver(1),
             ProperVer = exml_query:path(Item, [{element, <<"version">>}, cdata])
@@ -535,7 +533,7 @@ set_config_deny(Config) ->
 get_room_config(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             Stanza = stanza_config_get(?ROOM, <<"oldver">>),
-            ConfigKV = [{"version", binary_to_list(ver(1))} | standard_default_config()],
+            ConfigKV = [{<<"version">>, ver(1)} | default_config(default_schema())],
             foreach_occupant([Alice, Bob, Kate], Stanza, config_iq_verify_fun(ConfigKV)),
 
             %% Empty result when user has most recent version
@@ -548,7 +546,7 @@ get_room_config(Config) ->
 custom_default_config_works(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             Stanza = stanza_config_get(?ROOM, <<"oldver">>),
-            ConfigKV = [{"version", binary_to_list(ver(1))} | common_custom_config()],
+            ConfigKV = [{<<"version">>, ver(1)} | default_config(custom_schema())],
             foreach_occupant([Alice, Bob, Kate], Stanza, config_iq_verify_fun(ConfigKV))
         end).
 
@@ -568,10 +566,9 @@ get_room_occupants(Config) ->
 get_room_info(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             Stanza = stanza_info_get(?ROOM, <<"oldver">>),
-            ConfigKV = default_config(),
-            ConfigKVBin = [{list_to_binary(atom_to_list(Key)), Val} || {Key, Val} <- ConfigKV],
+            ConfigKV = default_config(default_schema()),
             foreach_occupant([Alice, Bob, Kate], Stanza,
-                             info_iq_verify_fun(?DEFAULT_AFF_USERS, ver(1), ConfigKVBin)),
+                             info_iq_verify_fun(?DEFAULT_AFF_USERS, ver(1), ConfigKV)),
 
             escalus:send(Bob, stanza_aff_get(?ROOM, ver(1))),
             IQRes = escalus:wait_for_stanza(Bob),
@@ -966,7 +963,7 @@ stanza_aff_get(Room, Ver) ->
 stanza_destroy_room(Room) ->
     escalus_stanza:to(escalus_stanza:iq_set(?NS_MUC_LIGHT_DESTROY, []), room_bin_jid(Room)).
 
--spec stanza_config_set(Room :: binary(), ConfigChanges :: [{binary(), binary()}]) -> xmlel().
+-spec stanza_config_set(Room :: binary(), ConfigChanges :: [muc_light_helper:config_item()]) -> xmlel().
 stanza_config_set(Room, ConfigChanges) ->
     Items = [ kv_el(Key, Value) || {Key, Value} <- ConfigChanges],
     escalus_stanza:to(
@@ -1008,7 +1005,7 @@ verify_no_stanzas(Users) ->
               {false, _} = {escalus_client:has_stanzas(User), User}
       end, Users).
 
--spec verify_config(ConfigRoot :: xmlel(), Config :: [{binary(), binary()}]) -> ok.
+-spec verify_config(ConfigRoot :: xmlel(), Config :: [muc_light_helper:config_item()]) -> ok.
 verify_config(ConfigRoot, Config) ->
     lists:foreach(
       fun({Key, Val}) ->
@@ -1019,7 +1016,7 @@ verify_config(ConfigRoot, Config) ->
 %% Verification funs generators
 %%--------------------------------------------------------------------
 
--spec config_msg_verify_fun(RoomConfig :: [{binary(), binary()}]) -> verify_fun().
+-spec config_msg_verify_fun(RoomConfig :: [muc_light_helper:config_item()]) -> verify_fun().
 config_msg_verify_fun(RoomConfig) ->
     fun(Incoming) ->
             escalus:assert(is_groupchat_message, Incoming),
@@ -1036,14 +1033,12 @@ config_msg_verify_fun(RoomConfig) ->
               end, RoomConfig)
     end.
 
--spec config_iq_verify_fun(RoomConfig :: [{string(), string()}]) -> verify_fun().
+-spec config_iq_verify_fun(RoomConfig :: [muc_light_helper:config_item()]) -> verify_fun().
 config_iq_verify_fun(RoomConfig) ->
     fun(Incoming) ->
             [Query] = exml_query:subelements(Incoming, <<"query">>),
             ?NS_MUC_LIGHT_CONFIGURATION = exml_query:attr(Query, <<"xmlns">>),
-            BinaryRoomConfig = [{list_to_binary(K), list_to_binary(V)}
-                                || {K, V} <- RoomConfig],
-            verify_config(Query, BinaryRoomConfig)
+            verify_config(Query, RoomConfig)
     end.
 
 -spec aff_iq_verify_fun(AffUsers :: ct_aff_users(), Version :: binary()) -> verify_fun().
@@ -1058,8 +1053,8 @@ aff_iq_verify_fun(AffUsers, Version) ->
     end.
 
 -spec info_iq_verify_fun(AffUsers :: ct_aff_users(), Version :: binary(),
-                         ConfigKVBin :: [{binary(), binary()}]) -> verify_fun().
-info_iq_verify_fun(AffUsers, Version, ConfigKVBin) ->
+                         ConfigKV :: [muc_light_helper:config_item()]) -> verify_fun().
+info_iq_verify_fun(AffUsers, Version, ConfigKV) ->
     BinAffUsers = bin_aff_users(AffUsers),
     fun(Incoming) ->
             [Query] = exml_query:subelements(Incoming, <<"query">>),
@@ -1069,7 +1064,7 @@ info_iq_verify_fun(AffUsers, Version, ConfigKVBin) ->
                                           {element, <<"user">>}]),
             verify_aff_users(UsersItems, BinAffUsers),
             ConfigurationEl = exml_query:subelement(Query, <<"configuration">>),
-            verify_config(ConfigurationEl, ConfigKVBin)
+            verify_config(ConfigurationEl, ConfigKV)
     end.
 
 -spec verify_user_has_one_room(User :: escalus:client()) -> any().
@@ -1106,20 +1101,11 @@ assert_process_memory_not_growing(Pid, OldMemory, Counter) ->
                  end,
   assert_process_memory_not_growing(Pid, Memory, NewCounter).
 
--spec common_custom_config() -> list().
-common_custom_config() -> [{"background", "builtin:hell"}, {"music", "builtin:screams"}].
-
--spec default_schema_definition() -> list().
-default_schema_definition() ->
-    rpc(mod_muc_light, default_schema_definition, []).
-
--spec standard_default_config() -> list().
-standard_default_config() ->
-    % Default schema definition is actually a proplist with the option name as a key
-    % and the default as a value, but we verify it to avoid strange errors.
-    DefaultConfig = default_schema_definition(),
-    lists:foreach(fun({K, V}) when is_list(K), is_list(V) -> ok end, DefaultConfig),
-    DefaultConfig.
+-spec custom_schema() -> [muc_light_helper:schema_item()].
+custom_schema() ->
+    % This list needs to be sorted
+    [{<<"background">>, <<"builtin:hell">>, background, binary},
+     {<<"music">>, <<"builtin:screams">>, music, binary}].
 
 -spec set_default_mod_config() -> ok.
 set_default_mod_config() ->
@@ -1135,6 +1121,6 @@ set_default_mod_config() ->
        {rooms_per_page, infinity}
       ]).
 
--spec set_custom_config(UserDefSchema :: list()) -> any().
-set_custom_config(UserDefSchema) when is_list(UserDefSchema) ->
-    set_mod_config(config_schema, UserDefSchema, ?MUCHOST).
+-spec set_config_schema(Schema :: list()) -> any().
+set_config_schema(Schema) when is_list(Schema) ->
+    set_mod_config(config_schema, Schema, ?MUCHOST).
diff --git a/big_tests/tests/muc_light_helper.erl b/big_tests/tests/muc_light_helper.erl
@@ -15,7 +15,12 @@
 -type ct_aff_users() :: [ct_aff_user()].
 -type ct_block_item() :: {What :: atom(), Action :: atom(), Who :: binary()}.
 
--export_type([ct_block_item/0]).
+%% The tests use only binary config fields
+-type schema_item() :: {binary(), binary(), atom(), binary}.
+-type config_item() :: {binary(), binary()}.
+-type internal_config_item() :: {atom(), binary()}.
+
+-export_type([ct_block_item/0, schema_item/0, config_item/0, internal_config_item/0]).
 
 -spec room_bin_jid(Room :: binary()) -> binary().
 room_bin_jid(Room) ->
@@ -28,7 +33,7 @@ muc_host_pattern() ->
     ct:get_config({hosts, mim, muc_light_service_pattern}).
 
 create_room(RoomU, MUCHost, Owner, Members, Config, Version) ->
-    DefaultConfig = default_config(),
+    DefaultConfig = default_internal_config(MUCHost),
     RoomUS = {RoomU, MUCHost},
     AffUsers = [{to_lus(Owner, Config), owner}
                 | [ {to_lus(Member, Config), member} || Member <- Members ]],
@@ -37,10 +42,23 @@ create_room(RoomU, MUCHost, Owner, Members, Config, Version) ->
     {ok, _RoomUS} = rpc(mim(), mod_muc_light_db_backend, create_room,
                         [RoomUS, DefaultConfig, AffUsersSort, Version]).
 
--spec default_config() -> list().
-default_config() -> assert_list(rpc(mim(), mod_muc_light, default_config, [muc_host()])).
+-spec default_internal_config(jid:lserver()) -> [internal_config_item()].
+default_internal_config(MUCHost) ->
+    Schema = rpc(mim(), mod_muc_light, config_schema, [MUCHost]),
+    {ok, Config} = rpc(mim(), mod_muc_light_room_config, from_binary_kv, [[], Schema]),
+    Config.
+
+-spec default_schema() -> [schema_item()].
+default_schema() ->
+    rpc(mim(), mod_muc_light, default_schema, []).
+
+-spec default_config([schema_item()]) -> [config_item()].
+default_config(Schema) ->
+    [config_item(SchemaItem) || SchemaItem <- Schema].
 
-assert_list(X) when is_list(X) -> X.
+-spec config_item(schema_item()) -> config_item().
+config_item({BinKey, BinValue, _Key, binary}) when is_binary(BinKey), is_binary(BinValue) ->
+    {BinKey, BinValue}.
 
 -spec ns_muc_light_affiliations() -> binary().
 ns_muc_light_affiliations() ->
@@ -207,7 +225,7 @@ verify_aff_users(Items, BinAffUsers) ->
                      Aff :: binary(), AffAcc :: list()) -> list().
 verify_keytake({value, {_, Aff}, NewAffAcc}, _JID, Aff, _AffAcc) -> NewAffAcc.
 
--spec stanza_create_room(RoomNode :: binary() | undefined, InitConfig :: [{binary(), binary()}],
+-spec stanza_create_room(RoomNode :: binary() | undefined, InitConfig :: [config_item()],
                          InitOccupants :: ct_aff_users()) -> exml:element().
 stanza_create_room(RoomNode, InitConfig, InitOccupants) ->
     Host = muc_host(),

diff --git a/big_tests/tests/muc_light_legacy_SUITE.erl b/big_tests/tests/muc_light_legacy_SUITE.erl
@@ -57,7 +57,10 @@
                            bin_aff_users/1,
                            to_lus/2,
                            lbin/1,
-                           set_mod_config/3
+                           create_room/6,
+                           set_mod_config/3,
+                           default_config/1,
+                           default_schema/0
                           ]).
 
 -define(ROOM, <<"testroom">>).
@@ -68,9 +71,6 @@
 
 -define(MUCHOST, (muc_helper:muc_host())).
 
--define(CHECK_FUN, fun mod_muc_light_room:participant_limit_check/2).
--define(BACKEND, mod_muc_light_db_backend).
-
 -type ct_aff_user() :: {EscalusClient :: escalus:client(), Aff :: atom()}.
 -type ct_aff_users() :: [ct_aff_user()].
 -type ct_block_item() :: {What :: atom(), Action :: atom(), Who :: binary()}.
@@ -198,11 +198,7 @@ end_per_testcase(CaseName, Config) ->
 %% ---------------------- Helpers ----------------------
 
 create_room(RoomU, MUCHost, Owner, Members, Config) ->
-    DefaultConfig = default_config(),
-    RoomUS = {RoomU, MUCHost},
-    AffUsers = [{to_lus(Owner, Config), owner}
-                | [ {to_lus(Member, Config), member} || Member <- Members ]],
-    {ok, _RoomUS} = rpc(?BACKEND, create_room, [RoomUS, DefaultConfig, AffUsers, <<"-">>]).
+    create_room(RoomU, MUCHost, Owner, Members, Config, <<"-">>).
 
 clear_db() ->
     muc_light_helper:clear_db().
@@ -336,9 +332,8 @@ set_config_deny(Config) ->
 get_room_config(Config) ->
     escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) ->
             Stanza = stanza_config_get(?ROOM),
-            ConfigKV = default_config(),
-            ConfigKVBin = [{list_to_binary(atom_to_list(Key)), Val} || {Key, Val} <- ConfigKV],
-            foreach_occupant([Alice, Bob, Kate], Stanza, config_iq_verify_fun(ConfigKVBin))
+            ConfigKV = default_config(default_schema()),
+            foreach_occupant([Alice, Bob, Kate], Stanza, config_iq_verify_fun(ConfigKV))
         end).
 
 get_room_occupants(Config) ->
@@ -629,7 +624,7 @@ stanza_destroy_room(Room) ->
     escalus_stanza:to(escalus_stanza:iq_set(?NS_MUC_OWNER, [#xmlel{ name = <<"destroy">> }]),
                       room_bin_jid(Room)).
 
--spec stanza_config_set(Room :: binary(), ConfigChanges :: [{binary(), binary()}]) -> xmlel().
+-spec stanza_config_set(Room :: binary(), ConfigChanges :: [muc_light_helper:config_item()]) -> xmlel().
 stanza_config_set(Room, ConfigChanges) ->
     IQ = escalus_stanza:iq_set(?NS_MUC_OWNER, [form_x_el(ConfigChanges)]),
     escalus_stanza:to(IQ, room_bin_jid(Room)).
@@ -726,7 +721,7 @@ verify_no_stanzas(Users) ->
               {false, _} = {escalus_client:has_stanzas(User), User}
       end, Users).
 
--spec verify_config(ConfigFields :: [xmlel()], Config :: [{binary(), binary()}]) -> ok.
+-spec verify_config(ConfigFields :: [xmlel()], Config :: [muc_light_helper:config_item()]) -> ok.
 verify_config(ConfigFields, Config) ->
     [] = lists:foldl(
            fun(Field, ConfigAcc) ->
@@ -787,7 +782,7 @@ config_msg_verify_fun() ->
             <<"104">> = exml_query:path(X, [{element, <<"status">>}, {attr, <<"code">>}])
     end.
 
--spec config_iq_verify_fun(RoomConfig :: [{binary(), binary()}]) -> verify_fun().
+-spec config_iq_verify_fun(RoomConfig :: [muc_light_helper:config_item()]) -> verify_fun().
 config_iq_verify_fun(RoomConfig) ->
     fun(Incoming) ->
             Fields = exml_query:paths(Incoming, [{element, <<"query">>}, {element, <<"x">>},
@@ -859,10 +854,6 @@ presence_verify(User, UserAff, #xmlel{ name = <<"presence">> } = Incoming) ->
 %% Other helpers
 %%--------------------------------------------------------------------
 
--spec default_config() -> list().
-default_config() ->
-    rpc(mod_muc_light, default_config, [?MUCHOST]).
-
 -spec set_default_mod_config() -> ok.
 set_default_mod_config() ->
     lists:foreach(