diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 2ef2699184..3dbe43decf 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -48,6 +48,7 @@ destroy_room_get_disco_items_empty/1, destroy_room_get_disco_items_one_left/1, set_config/1, + set_partial_config/1, set_config_with_custom_schema/1, deny_config_change_that_conflicts_with_schema/1, assorted_config_doesnt_lead_to_duplication/1, @@ -169,6 +170,7 @@ groups() -> destroy_room_get_disco_items_empty, destroy_room_get_disco_items_one_left, set_config, + set_partial_config, set_config_with_custom_schema, deny_config_change_that_conflicts_with_schema, assorted_config_doesnt_lead_to_duplication, @@ -713,18 +715,31 @@ destroy_room_get_disco_items_one_left(Config) -> set_config(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> ConfigChange = [{<<"roomname">>, <<"The Coven">>}], - escalus:send(Alice, stanza_config_set(?ROOM, ConfigChange)), - foreach_recipient([Alice, Bob, Kate], config_msg_verify_fun(ConfigChange)), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)) + change_room_config(Alice, [Alice, Bob, Kate], ConfigChange) + end). + +set_partial_config(Config) -> + escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> + FinalRoomName = <<"The Coven">>, + %% Change all the config + ConfigChange = [{<<"roomname">>, FinalRoomName}, {<<"subject">>, <<"Evil Ravens">>}], + change_room_config(Alice, [Alice, Bob, Kate], ConfigChange), + %% Now change only the subject + ConfigChange2 = [{<<"subject">>, <<"Good Ravens">>}], + change_room_config(Alice, [Alice, Bob, Kate], ConfigChange2), + %% Verify that roomname is still the original change + escalus:send(Alice, stanza_config_get(?ROOM, ver(1))), + IQRes = escalus:wait_for_stanza(Alice), + escalus:assert(is_iq_result, IQRes), + RoomName = exml_query:path(IQRes, [{element, <<"query">>}, {element, <<"roomname">>}, cdata]), + ?assertEqual(FinalRoomName, RoomName) end). set_config_with_custom_schema(Config) -> escalus:story(Config, [{alice, 1}, {bob, 1}, {kate, 1}], fun(Alice, Bob, Kate) -> ConfigChange = [{<<"background">>, <<"builtin:unicorns">>}, {<<"music">>, <<"builtin:rainbow">>}], - escalus:send(Alice, stanza_config_set(?ROOM, ConfigChange)), - foreach_recipient([Alice, Bob, Kate], config_msg_verify_fun(ConfigChange)), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)) + change_room_config(Alice, [Alice, Bob, Kate], ConfigChange) end). deny_config_change_that_conflicts_with_schema(Config) -> @@ -740,10 +755,7 @@ assorted_config_doesnt_lead_to_duplication(Config) -> ConfigChange = [{<<"subject">>, <<"Elixirs">>}, {<<"roomname">>, <<"The Coven">>}, {<<"subject">>, <<"Elixirs">>}], - ConfigSetStanza = stanza_config_set(?ROOM, ConfigChange), - escalus:send(Alice, ConfigSetStanza), - foreach_recipient([Alice, Bob, Kate], config_msg_verify_fun(ConfigChange)), - escalus:assert(is_iq_result, escalus:wait_for_stanza(Alice)), + change_room_config(Alice, [Alice, Bob, Kate], ConfigChange), Stanza = stanza_config_get(?ROOM, <<"oldver">>), VerifyFun = fun(Incoming) -> @@ -937,6 +949,12 @@ blocking_disabled(Config) -> %% Subroutines %%-------------------------------------------------------------------- +-spec change_room_config(escalus:client(), [escalus:client()], list({binary(), binary()})) -> term(). +change_room_config(User, Users, ConfigChange) -> + escalus:send(User, stanza_config_set(?ROOM, ConfigChange)), + foreach_recipient(Users, config_msg_verify_fun(ConfigChange)), + escalus:assert(is_iq_result, escalus:wait_for_stanza(User)). + -spec get_disco_rooms(User :: escalus:client()) -> list(xmlel()). get_disco_rooms(User) -> DiscoStanza = escalus_stanza:to(escalus_stanza:iq_get(?NS_DISCO_ITEMS, []), ?MUCHOST), diff --git a/src/muc_light/mod_muc_light_db_rdbms.erl b/src/muc_light/mod_muc_light_db_rdbms.erl index 4709a62ccb..48bad0ea1f 100644 --- a/src/muc_light/mod_muc_light_db_rdbms.erl +++ b/src/muc_light/mod_muc_light_db_rdbms.erl @@ -657,7 +657,7 @@ create_room_transaction(HostType, {RoomU, RoomS}, Config, AffUsers, Version) -> insert_room(HostType, RoomU, RoomS, Version), RoomID = mongoose_rdbms:selected_to_integer(select_room_id(HostType, RoomU, RoomS)), Schema = mod_muc_light:config_schema(RoomS), - ConfigFields = mod_muc_light_room_config:to_binary_kv(Config, Schema), + {ok, ConfigFields} = mod_muc_light_room_config:to_binary_kv(Config, Schema), [insert_aff_tuple(HostType, RoomID, AffUser) || AffUser <- AffUsers], [insert_config_kv(HostType, RoomID, KV) || KV <- ConfigFields], ok. @@ -708,11 +708,12 @@ set_config_transaction({RoomU, RoomS} = RoomUS, ConfigChanges, Version) -> {selected, [{DbRoomID, PrevVersion}]} -> RoomID = mongoose_rdbms:result_to_integer(DbRoomID), {updated, _} = update_room_version(HostType, RoomU, RoomS, Version), + {ok, Config} = mod_muc_light_room_config:to_binary_kv_diff( + ConfigChanges, mod_muc_light:config_schema(RoomS)), lists:foreach( fun({Key, Val}) -> {updated, _} = update_config(HostType, RoomID, Key, Val) - end, mod_muc_light_room_config:to_binary_kv( - ConfigChanges, mod_muc_light:config_schema(RoomS))), + end, Config), {ok, PrevVersion}; {selected, []} -> {error, not_exists} diff --git a/src/muc_light/mod_muc_light_room.erl b/src/muc_light/mod_muc_light_room.erl index d1fb6b1a5a..a6d1fc4d97 100644 --- a/src/muc_light/mod_muc_light_room.erl +++ b/src/muc_light/mod_muc_light_room.erl @@ -108,7 +108,7 @@ process_request({get, #config{} = ConfigReq}, {_, RoomS} = RoomUS, HostType = mongoose_acc:host_type(Acc), {ok, Config, RoomVersion} = mod_muc_light_db_backend:get_config(HostType, RoomUS), - RawConfig = mod_muc_light_room_config:to_binary_kv(Config, mod_muc_light:config_schema(RoomS)), + {ok, RawConfig} = mod_muc_light_room_config:to_binary_kv(Config, mod_muc_light:config_schema(RoomS)), {get, ConfigReq#config{ version = RoomVersion, raw_config = RawConfig }}; process_request({get, #affiliations{} = AffReq}, @@ -121,7 +121,7 @@ process_request({get, #info{} = InfoReq}, _From, {_, RoomS} = RoomUS, _Auth, _AffUsers, Acc) -> HostType = mongoose_acc:host_type(Acc), {ok, Config, AffUsers, RoomVersion} = mod_muc_light_db_backend:get_info(HostType, RoomUS), - RawConfig = mod_muc_light_room_config:to_binary_kv(Config, mod_muc_light:config_schema(RoomS)), + {ok, RawConfig} = mod_muc_light_room_config:to_binary_kv(Config, mod_muc_light:config_schema(RoomS)), {get, InfoReq#info{ version = RoomVersion, aff_users = AffUsers, raw_config = RawConfig }}; process_request({set, #config{} = ConfigReq}, diff --git a/src/muc_light/mod_muc_light_room_config.erl b/src/muc_light/mod_muc_light_room_config.erl index 5db3941f99..5d0eed70dc 100644 --- a/src/muc_light/mod_muc_light_room_config.erl +++ b/src/muc_light/mod_muc_light_room_config.erl @@ -23,7 +23,8 @@ -module(mod_muc_light_room_config). %% API --export([from_binary_kv_diff/2, from_binary_kv/2, to_binary_kv/2]). +-export([from_binary_kv_diff/2, from_binary_kv/2, + to_binary_kv_diff/2, to_binary_kv/2]). -include("mod_muc_light.hrl"). @@ -40,8 +41,7 @@ -type binary_kv() :: [{Key :: binary(), Value :: binary()}]. %% User definition processing --type schema_item() :: {FieldName :: binary(), DefaultValue :: value(), - key(), value_type()}. +-type schema_item() :: {FieldName :: binary(), DefaultValue :: value(), key(), value_type()}. -type schema() :: [schema_item()]. % has to be sorted %%==================================================================== @@ -52,36 +52,40 @@ -spec from_binary_kv_diff(RawConfig :: binary_kv(), ConfigSchema :: schema()) -> {ok, kv()} | validation_error(). from_binary_kv_diff(RawConfig, ConfigSchema) -> - from_binary_kv_diff(lists:ukeysort(1, RawConfig), ConfigSchema, []). - -from_binary_kv_diff([], [], Config) -> - {ok, Config}; -from_binary_kv_diff(RawConfig, ConfigSchema, Config) -> - case take_next_kv(RawConfig, ConfigSchema) of - {error, Reason} -> - {error, Reason}; - {value, RRawConfig, RConfigSchema, KV} -> - from_binary_kv(RRawConfig, RConfigSchema, [KV | Config]); - {default, _, _, _} -> - % do not populate the diff with default values - from_binary_kv(RawConfig, ConfigSchema, Config) - end. + take_next(lists:ukeysort(1, RawConfig), ConfigSchema, true, fun take_next_kv/2, []). -spec from_binary_kv(RawConfig :: binary_kv(), ConfigSchema :: schema()) -> - {ok, kv()} | validation_error(). + {ok, kv()} | validation_error(). from_binary_kv(RawConfig, ConfigSchema) -> - from_binary_kv(lists:ukeysort(1, RawConfig), ConfigSchema, []). + take_next(lists:ukeysort(1, RawConfig), ConfigSchema, false, fun take_next_kv/2, []). -from_binary_kv([], [], Config) -> +-spec to_binary_kv_diff(RawConfig :: kv(), ConfigSchema :: schema()) -> + {ok, binary_kv()} | validation_error(). +to_binary_kv_diff(RawConfig, ConfigSchema) -> + take_next(lists:ukeysort(1, RawConfig), ConfigSchema, true, fun take_next_binary_kv/2, []). + +-spec to_binary_kv(Config :: kv(), ConfigSchema :: schema()) -> + {ok, binary_kv()} | validation_error(). +to_binary_kv(RawConfig, ConfigSchema) -> + take_next(lists:ukeysort(1, RawConfig), ConfigSchema, false, fun take_next_binary_kv/2, []). + +take_next([], [], _, _, Config) -> {ok, Config}; -from_binary_kv(RawConfig, ConfigSchema, Config) -> - case take_next_kv(RawConfig, ConfigSchema) of - {error, Reason} -> - {error, Reason}; - {_, RRawConfig, RConfigSchema, KV} -> - from_binary_kv(RRawConfig, RConfigSchema, [KV | Config]) +take_next(RawConfig, ConfigSchema, DropDefaults, TakeNext, Config) -> + case {DropDefaults, TakeNext(RawConfig, ConfigSchema)} of + {true, {default, RRawConfig, RConfigSchema, _}} -> + % do not populate the diff with default values + take_next(RRawConfig, RConfigSchema, DropDefaults, TakeNext, Config); + {_, {_, RRawConfig, RConfigSchema, KV}} -> + take_next(RRawConfig, RConfigSchema, DropDefaults, TakeNext, [KV | Config]); + {_, {error, Reason}} -> + {error, Reason} end. +%%==================================================================== +%% Internal functions +%%==================================================================== + take_next_kv([{KeyBin, ValBin} | RRawConfig], [{KeyBin, _Default, Key, Type} | RSchema]) -> try {value, RRawConfig, RSchema, {Key, b2value(ValBin, Type)}} catch _:_ -> {error, {KeyBin, type_error}} @@ -91,14 +95,14 @@ take_next_kv(RawConfig, [{_KeyBin, Default, Key, _Type} | RSchema]) -> take_next_kv([{KeyBin, _} | _], _) -> {error, {KeyBin, not_found}}. --spec to_binary_kv(Config :: kv(), ConfigSchema :: schema()) -> binary_kv(). -to_binary_kv(Config, ConfigSchema) -> - ConfigWithSchema = lists:zip(lists:sort(Config), lists:keysort(3, ConfigSchema)), - [{KeyBin, value2b(Val, Type)} || {{Key, Val}, {KeyBin, _Default, Key, Type}} <- ConfigWithSchema]. - -%%==================================================================== -%% Internal functions -%%==================================================================== +take_next_binary_kv([{Key, ValBin} | RRawConfig], [{KeyBin, _Default, Key, Type} | RSchema]) -> + try {value, RRawConfig, RSchema, {KeyBin, value2b(ValBin, Type)}} + catch _:_ -> {error, {KeyBin, type_error}} + end; +take_next_binary_kv(RawConfig, [{KeyBin, Default, _Key, _Type} | RSchema]) -> + {default, RawConfig, RSchema, {KeyBin, Default}}; +take_next_binary_kv([{KeyBin, _} | _], _) -> + {error, {KeyBin, not_found}}. -spec b2value(ValBin :: binary(), Type :: value_type()) -> Converted :: value(). b2value(ValBin, binary) when is_binary(ValBin) -> ValBin;