Simplify MUC Light config #3387
Conversation
It is already validated in the config parser
This comment has been minimized.
This comment has been minimized.
chrzaszcz
force-pushed
the
static-module-config
branch
from
ed1a2dd
to
051266b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
chrzaszcz
force-pushed
the
static-module-config
branch
from
59fb063
to
7304f07
Compare
Reason: clarity, simplicity and little to no real performance benefit, as the computational cost was dominated by lists:keystore anyway. Also: separate functions for conversion of the initial config and the diff, which eliminates the need for storing the default config and using lists:keystore. The config schema needs to be sorted now.
|
small_tests_24 / small_tests / 7304f07 internal_mnesia_24 / internal_mnesia / 7304f07 small_tests_23 / small_tests / 7304f07 ldap_mnesia_24 / ldap_mnesia / 7304f07 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7304f07 ldap_mnesia_23 / ldap_mnesia / 7304f07 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7304f07 dynamic_domains_mysql_redis_24 / mysql_redis / 7304f07 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7304f07 pgsql_mnesia_24 / pgsql_mnesia / 7304f07 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7304f07 mssql_mnesia_24 / odbc_mssql_mnesia / 7304f07 mysql_redis_24 / mysql_redis / 7304f07 mod_event_pusher_rabbit_SUITE:group_chat_message_publish:group_chat_message_received_event_properly_formatted{error,
{{assertMatch,
[{module,mod_event_pusher_rabbit_SUITE},
{line,435},
{expression,
"get_decoded_message_from_rabbit ( AliceGroupChatMsgRecvRK )"},
{pattern,
"# { << \"from_user_id\" >> := BobRoomJID , << \"to_user_id\" >> := AliceFullJID , << \"message\" >> := Message }"},
{value,
#{<<"from_user_id">> =>
<<"[email protected]/bOb_unnamed_1.19116">>,
<<"message">> => <<"Hi there!">>,
<<"to_user_id">> =>
<<"alice_unnamed_1.181986@localhost/res1">>}}]},
[{mod_event_pusher_rabbit_SUITE,
'-group_chat_message_received_event_properly_formatted/1-fun-1-',3,
[{file,
"/home/circleci/app/big_tests/tests/mod_event_pusher_rabbit_SUITE.erl"},
{line,435}]},
{escalus_story,story,4,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}mod_event_pusher_rabbit_SUITE:group_chat_message_publish:group_chat_message_received_event_properly_formatted{error,
{{assertMatch,
[{module,mod_event_pusher_rabbit_SUITE},
{line,435},
{expression,
"get_decoded_message_from_rabbit ( AliceGroupChatMsgRecvRK )"},
{pattern,
"# { << \"from_user_id\" >> := BobRoomJID , << \"to_user_id\" >> := AliceFullJID , << \"message\" >> := Message }"},
{value,
#{<<"from_user_id">> => <<"[email protected]">>,
<<"message">> => <<>>,
<<"to_user_id">> =>
<<"alice_unnamed_1.934439@localhost/res1">>}}]},
[{mod_event_pusher_rabbit_SUITE,
'-group_chat_message_received_event_properly_formatted/1-fun-1-',3,
[{file,
"/home/circleci/app/big_tests/tests/mod_event_pusher_rabbit_SUITE.erl"},
{line,435}]},
{escalus_story,story,4,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}mod_event_pusher_rabbit_SUITE:group_chat_message_publish:group_chat_message_received_event_properly_formatted{error,
{{assertMatch,
[{module,mod_event_pusher_rabbit_SUITE},
{line,435},
{expression,
"get_decoded_message_from_rabbit ( AliceGroupChatMsgRecvRK )"},
{pattern,
"# { << \"from_user_id\" >> := BobRoomJID , << \"to_user_id\" >> := AliceFullJID , << \"message\" >> := Message }"},
{value,
#{<<"from_user_id">> =>
<<"[email protected]/bOb_unnamed_1.181986">>,
<<"message">> => <<"Hi there!">>,
<<"to_user_id">> =>
<<"alice_unnamed_2.749520@localhost/res1">>}}]},
[{mod_event_pusher_rabbit_SUITE,
'-group_chat_message_received_event_properly_formatted/1-fun-1-',3,
[{file,
"/home/circleci/app/big_tests/tests/mod_event_pusher_rabbit_SUITE.erl"},
{line,435}]},
{escalus_story,story,4,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}pgsql_mnesia_23 / pgsql_mnesia / 7304f07 riak_mnesia_24 / riak_mnesia / 7304f07 |
Codecov Report
@@ Coverage Diff @@
## master #3387 +/- ##
==========================================
+ Coverage 80.64% 80.70% +0.05%
==========================================
Files 397 397
Lines 32268 32239 -29
==========================================
- Hits 26024 26019 -5
+ Misses 6244 6220 -24
Continue to review full report at Codecov.
|
The schema is now fully tested in config_parser_SUITE
Whenever possible, call deafult_config(Schema) to verify against the specific expected schema instead of the one from MIM. Use default_config() only in room creation helpers.
chrzaszcz
force-pushed
the
static-module-config
branch
from
7304f07
to
fb042ee
Compare
|
small_tests_24 / small_tests / fb042ee internal_mnesia_24 / internal_mnesia / fb042ee small_tests_23 / small_tests / fb042ee ldap_mnesia_24 / ldap_mnesia / fb042ee dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / fb042ee dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / fb042ee sm_SUITE:parallel:messages_are_properly_flushed_during_resumption{error,
{{badmatch,
{error,
{connection_step_failed,
{#Fun<sm_SUITE.11.118723036>,
{client,
<<"alicE_messages_are_properly_flushed_during_resumption_76.128008@domain.example.com">>,
escalus_tcp,<0.31400.1>,undefined,
[{username,
<<"alicE_messages_are_properly_flushed_during_resumption_76.128008">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_management,true},
{stream_id,<<"dcf04e41bfa44cbd">>}]},
[{compression,[<<"zlib">>]},
{starttls,true},
{stream_management,true},
{advanced_message_processing,true},
{client_state_indication,false},
{sasl_mechanisms,[<<"SCRAM-SHA-256">>,<<"PLAIN">>]},
{caps,undefined}]},
{timeout,get_resumed}}}},
[{sm_SUITE,'-messages_are_properly_flushed_during_resumption/1-fun-1-',
3,
[{file,"/home/circleci/app/big_tests/tests/sm_SUITE.erl"},
{line,1230}]},
{escalus_story,story,4,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}dynamic_domains_mysql_redis_24 / mysql_redis / fb042ee dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / fb042ee ldap_mnesia_23 / ldap_mnesia / fb042ee sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_stop_c2s{error,{thrown,{timeout,msg}}}sm_SUITE:unacknowledged_message_hook:unacknowledged_message_hook_offline{error,{{badmatch,false},
[{escalus_session,stream_resumption,2,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,259}]},
{escalus_connection,connection_step,2,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,160}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_connection,start,2,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,144}]},
{sm_SUITE,unacknowledged_message_hook_offline,4,
[{file,"/home/circleci/app/big_tests/tests/sm_SUITE.erl"},
{line,830}]},
{sm_SUITE,unacknowledged_message_hook_common,2,
[{file,"/home/circleci/app/big_tests/tests/sm_SUITE.erl"},
{line,882}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1784}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1293}]}]}}elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / fb042ee mysql_redis_24 / mysql_redis / fb042ee mam_SUITE:rdbms_async_pool_prefs_cases:messages_filtered_when_prefs_default_policy_is_never{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok]\n"}}mam_SUITE:rdbms_async_cache_prefs_cases:messages_filtered_when_prefs_default_policy_is_roster{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok,ok]\n"}}pgsql_mnesia_24 / pgsql_mnesia / fb042ee service_domain_db_SUITE:db:db_keeps_syncing_after_cluster_join{error,{test_case_failed,{[<<"example1.com">>],
[<<"example1.com">>,<<"example2.com">>]}}}service_domain_db_SUITE:db:rest_with_auth:rest_delete_domain_cleans_data_from_mam{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"[email protected]/res1">>,
escalus_tcp,<0.13498.2>,
[{event_manager,<0.13492.2>},
{server,<<"example.org">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_4.258839">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.13492.2>},
{server,<<"example.org">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_4.258839">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_4.258839">>},
{server,<<"example.org">>},
{host,<<"localhost">>},
{port,5232},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_4.258839">>},
{server,<<"example.org">>},
{host,<<"localhost">>},
{password,<<"makota3">>},
{port,5232},
{stream_id,<<"c032acd4015fa2aa">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{service_domain_db_SUITE,
'-rest_delete_domain_cleans_data_from_mam/1-fun-0-',5,
...service_domain_db_SUITE:db:rest_without_auth:rest_delete_domain_cleans_data_from_mam{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bob_rest_delete_domain_cleans_data_from_mam_12.449295@example.org/res1">>,
escalus_tcp,<0.14140.2>,
[{event_manager,<0.14134.2>},
{server,<<"example.org">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_12.449295">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.14134.2>},
{server,<<"example.org">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_12.449295">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_12.449295">>},
{server,<<"example.org">>},
{host,<<"localhost">>},
{port,5232},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"bob_rest_delete_domain_cleans_data_from_mam_12.449295">>},
{server,<<"example.org">>},
{host,<<"localhost">>},
{password,<<"makota3">>},
{port,5232},
{stream_id,<<"362ddc650c0c88cd">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{service_domain_db_SUITE,
'-rest_delete_domain_cleans_data_from_mam/1-fun-0-',5...mssql_mnesia_24 / odbc_mssql_mnesia / fb042ee sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message{error,
{{assertion_failed,assert_many,true,
[#Fun<sm_SUITE.26.118723036>,#Fun<sm_SUITE.26.118723036>,
#Fun<sm_SUITE.26.118723036>],
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_resume_session_state_send_message_64.272692@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alice_resume_session_state_send_message_64.272692@localhost/escalus-default-resource">>},
{<<"xml:lang">>,<<"en">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_64.206466@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_64.272692@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-1">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2021-11-05T15:06:04.348858Z">>},
{<<"from">>,<<"localhost">>}],
[{xmlcdata,<<"SM Storage">>}]}]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_64.206466@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_64.272692@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-2">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xm...sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message{error,
{{assertion_failed,assert_many,true,
[#Fun<sm_SUITE.26.118723036>,#Fun<sm_SUITE.26.118723036>,
#Fun<sm_SUITE.26.118723036>],
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_resume_session_state_send_message_70.633249@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alice_resume_session_state_send_message_70.633249@localhost/escalus-default-resource">>},
{<<"xml:lang">>,<<"en">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_70.574936@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_70.633249@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-1">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2021-11-05T15:06:10.727391Z">>},
{<<"from">>,<<"localhost">>}],
[{xmlcdata,<<"SM Storage">>}]}]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_70.574936@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_70.633249@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-2">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xm...pgsql_mnesia_23 / pgsql_mnesia / fb042ee jingle_SUITE:all:resp_4xx_from_sip_proxy_results_in_session_terminate{error,
{{assertion_failed,assert,is_iq_result,
{xmlel,<<"iq">>,
[{<<"from">>,<<"error.480@localhost">>},
{<<"to">>,
<<"alice_resp_4xx_from_sip_proxy_results_in_session_terminate_5.238821@localhost/res1">>},
{<<"id">>,<<"7ab42426-7a4c-4e6a-aef7-db5fc6ff8ecf">>},
{<<"type">>,<<"set">>}],
[{xmlel,<<"jingle">>,
[{<<"xmlns">>,<<"urn:xmpp:jingle:1">>},
{<<"action">>,<<"session-terminate">>},
{<<"sid">>,<<"d4569827-a901-439c-9bf9-fd05a2d036ff">>}],
[{xmlel,<<"reason">>,[],
[{xmlel,<<"general-error">>,[],[]},
{xmlel,<<"sip-error">>,
[{<<"code">>,<<"480">>}],
[{xmlcdata,<<"Temporarily Unavailable">>}]}]}]}]},
"<iq from='error.480@localhost' to='alice_resp_4xx_from_sip_proxy_results_in_session_terminate_5.238821@localhost/res1' id='7ab42426-7a4c-4e6a-aef7-db5fc6ff8ecf' type='set'><jingle xmlns='urn:xmpp:jingle:1' action='session-terminate' sid='d4569827-a901-439c-9bf9-fd05a2d036ff'><reason><general-error/><sip-error code='480'>Temporarily Unavailable</sip-error></reason></jingle></iq>"},
[{escalus_new_assert,assert_true,2,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{jingle_SUITE,send_initiate_and_wait_for_first_iq_set,2,
[{file,"/home/circleci/app/big_tests/tests/jingle_SUITE.erl"},
{line,395}]},
{jingle_SUITE,
'-resp_...riak_mnesia_24 / riak_mnesia / fb042ee pgsql_mnesia_24 / pgsql_mnesia / fb042ee mongooseimctl_SUITE:stats:stats_global{error,{{badmatch,{"5\n",0}},
[{mongooseimctl_SUITE,'-stats_global/1-fun-0-',3,
[{file,"/home/circleci/app/big_tests/tests/mongooseimctl_SUITE.erl"},
{line,1057}]},
{escalus_story,story,4,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}mssql_mnesia_24 / odbc_mssql_mnesia / fb042ee pgsql_mnesia_24 / pgsql_mnesia / fb042ee domain_removal_SUITE:last_removal:last_removal{error,{{assertion_failed,assert,is_last_result,
{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_last_removal_22.154012@localhost/friendly">>},
{<<"to">>,
<<"alice_last_removal_22.154012@localhost/res1">>},
{<<"type">>,<<"unavailable">>}],
[]},
"<presence from='alicE_last_removal_22.154012@localhost/friendly' to='alice_last_removal_22.154012@localhost/res1' type='unavailable'/>"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{domain_removal_SUITE,'-last_removal/1-fun-0-',2,
[{file,"/home/circleci/app/big_tests/tests/domain_removal_SUITE.erl"},
{line,379}]},
{escalus_story,story,4,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}pgsql_mnesia_24 / pgsql_mnesia / fb042ee |
| @@ -185,8 +185,8 @@ process_config_set(#config{ raw_config = [{<<"subject">>, _}] } = ConfigReq, Roo | |||
| process_config_set(_ConfigReq, _RoomUS, member, _AffUsers, false) -> | |||
| {error, not_allowed}; | |||
| process_config_set(ConfigReq, {_, RoomS} = RoomUS, _UserAff, AffUsers, _AllCanConfigure) -> | |||
| case mod_muc_light_room_config:apply_binary_kv( | |||
| ConfigReq#config.raw_config, [], mod_muc_light:config_schema(RoomS)) of | |||
| case mod_muc_light_room_config:from_binary_kv_diff( | |||
There was a problem hiding this comment.
Why do we need this new function here? Because we don't want to store the default values? I'm not sure if I understand this change, as there are places which used apply_binary_kv and run from_binary_kv now, not apply_binary_kv_diff.
There was a problem hiding this comment.
There are exactly two types of conversions from binary KV:
- Conversion of the initial config. Here all missing values should be substituted with defaults.
- Conversion of the config change (diff). Here all missing values should be skipped.
In the past there was a function called apply_binary_kv which was used for both cases and merged two configs. The point is that we actually never need to merge any two configs, so for each case I checked which of the two conversions needs to happen and explicitly called that one, making the code easier to understand and to optimize. Moreover, there is now no need for storing the default config anymore.
There was a problem hiding this comment.
After the discussion with @gustawlippa I have to add that I omitted the third type of conversion, which is used by the RDBMS backend when reading the data from the DB. I used from_binary_kv there because it is more natural, but there should be no missing values in the DB, so no defaults will be injected anyway.
The goal of this PR is to simplify the MUC Light config and get rid of the dynamically set options.
Not done in this PR:
mongoose_config_specto eliminate the need to create the default schema in a function every time it is accessed. This change will be done later in a separate task.mod_muc_light_db_rdbms. This is not trivial and seems to fall outside of the scope of this PR.