Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to format a section/list as a map #3420

Merged
merged 16 commits into from
Dec 7, 2021
Merged

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Nov 29, 2021

Introduce the new format_items field to allow conversion of selected config sections to maps. In the future, when we convert enough sections, we could make format_items = map default for sections. For now it would mean too many changes.

Another important change is the ability to call get_opt and lookup_opt in mongoose_config with the complete path for a nested option, e.g. [{auth, HostType}, riak, bucket_type]. The path might look cleaner with HostType passed as a separate argument, but this would increase the complexity of mongoose_config, which is quite clean and simple right now.

The conversion to maps is done for the auth section and for selected s2s options. The main motivation for choosing these particular options was that their processing used the foreach format, which was a temporary hack.

Other changes and notes:

  • Option names are simplified.
  • Dummy auth method has better config validation and tests.
  • LDAP auth-related options are simplified to use less custom processing, but the changes are limited to auth for now. This should be finished when converting modules using LDAP.
  • It is possible to inject a sub-section into the parent section with format = none just like before.
  • Putting the format_items step before process allows simpler processing functions, taking a map as an argument.
  • Putting the format_items step after validate allows validation of lists just like before, improving steps separation.

Some further polishing could be done later, e.g. regarding the dafaults and password format.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@codecov
Copy link

codecov bot commented Nov 29, 2021

Codecov Report

Merging #3420 (1af9d2d) into master (9b906c5) will increase coverage by 0.02%.
The diff coverage is 94.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3420      +/-   ##
==========================================
+ Coverage   80.76%   80.79%   +0.02%     
==========================================
  Files         414      414              
  Lines       32335    32312      -23     
==========================================
- Hits        26116    26106      -10     
+ Misses       6219     6206      -13     
Impacted Files Coverage Δ
src/event_pusher/mod_event_pusher.erl 65.00% <ø> (ø)
src/global_distrib/mod_global_distrib.erl 85.71% <ø> (ø)
src/mam/mod_mam_meta.erl 92.39% <ø> (ø)
src/mod_auth_token.erl 82.11% <ø> (ø)
src/mod_bosh.erl 94.36% <ø> (ø)
src/mod_extdisco.erl 100.00% <ø> (ø)
src/mod_last.erl 86.56% <ø> (ø)
src/mod_muc.erl 74.32% <ø> (ø)
src/mod_muc_log.erl 78.11% <ø> (ø)
src/mod_private.erl 79.66% <ø> (-3.39%) ⬇️
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b906c5...1af9d2d. Read the comment docs.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

This option will allow to format a KV list as a map.
Put it:
- after 'validate' to validate the lists normally (without two
  branches: for lists and maps)
- before 'process' to process maps instead of KV lists (easier).
If the supplied key is a list, it denotes a path in the config tree
The whole 'auth' section becomes a map as it was complicated,
  having both nested and top-level options.
Regarding s2s, only the options with tagged keys are changed to maps.
The whole s2s section could also be a map, but it is not necessary.
- The 'foreach' format (now unused) is removed (it was temporary)
- The types are updated with maps and without the complex s2s keys
Use mongoose_config API directly instead of ejabberd_auth - it seems
more straightforward and the former convention was used only partially.
The changes in other modules were minimised for now.
Full conversion needs changing mod_vcard and shared roster code.
Option overriding is now simplified.
@chrzaszcz chrzaszcz force-pushed the config-with-maps branch 2 times, most recently from bc5c5c6 to 3f2fe04 Compare December 7, 2021 14:10
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 7, 2021

small_tests_24 / small_tests / bc5c5c6
Reports root / small


small_tests_23 / small_tests / bc5c5c6
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / bc5c5c6
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / bc5c5c6
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / bc5c5c6
Reports root/ big
OK: 2709 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / bc5c5c6
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / bc5c5c6
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / bc5c5c6
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / bc5c5c6
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / bc5c5c6
Reports root/ big
OK: 1880 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / bc5c5c6
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / bc5c5c6
Reports root/ big
OK: 3108 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / bc5c5c6
Reports root/ big
OK: 3120 / Failed: 1 / User-skipped: 195 / Auto-skipped: 0

sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_stop_c2s
{error,{thrown,{timeout,msg}}}

Report log


mssql_mnesia_24 / odbc_mssql_mnesia / bc5c5c6
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / bc5c5c6
Reports root/ big
OK: 1726 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 7, 2021

small_tests_24 / small_tests / 3f2fe04
Reports root / small


small_tests_23 / small_tests / 3f2fe04
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 3f2fe04
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 3f2fe04
Reports root/ big
OK: 2709 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 3f2fe04
Reports root/ big
OK: 2754 / Failed: 1 / User-skipped: 186 / Auto-skipped: 0

sm_SUITE:parallel:messages_are_properly_flushed_during_resumption_p1_fsm_old
{error,
  {{badmatch,
     {error,
       {connection_step_failed,
         {#Fun<sm_SUITE.11.68776247>,
          {client,
            <<"alicE_messages_are_properly_flushed_during_resumption_p1_fsm_old_1964@domain.example.com">>,
            escalus_tcp,<0.31571.1>,undefined,
            [{username,
               <<"alicE_messages_are_properly_flushed_during_resumption_p1_fsm_old_1964">>},
             {server,<<"domain.example.com">>},
             {host,<<"localhost">>},
             {password,<<"matygrysa">>},
             {stream_management,true},
             {stream_id,<<"f6b65b8ca739088c">>}]},
          [{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_p1_fsm_old/1-fun-1-',
      3,
      [{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
       {line,1270}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/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_se...

Report log


ldap_mnesia_23 / ldap_mnesia / 3f2fe04
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 3f2fe04
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 3f2fe04
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 3f2fe04
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 3f2fe04
Reports root/ big
OK: 3108 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 3f2fe04
Reports root/ big
OK: 1880 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 3f2fe04
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 3f2fe04
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 3f2fe04
Reports root/ big
OK: 1726 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

Motivation: after removal of 'foreach' this step no longer modifies
the value and always wraps it in a list (optionally with a key).
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 7, 2021

small_tests_24 / small_tests / 6d03575
Reports root / small


small_tests_23 / small_tests / 6d03575
Reports root / small


dynamic_domains_mysql_redis_24 / mysql_redis / 6d03575
Reports root/ big
OK: 2709 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6d03575
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 6d03575
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 6d03575
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 6d03575
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 6d03575
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 6d03575
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 6d03575
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 6d03575
Reports root/ big
OK: 1880 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 6d03575
Reports root/ big
OK: 3148 / Failed: 2 / User-skipped: 195 / Auto-skipped: 0

sm_SUITE:parallel:messages_are_properly_flushed_during_resumption
{error,
  {{badmatch,
     {error,
       {connection_step_failed,
         {#Fun<sm_SUITE.11.2813143>,
          {client,
            <<"alicE_messages_are_properly_flushed_during_resumption_2046@localhost">>,
            escalus_tcp,<0.13699.2>,undefined,
            [{username,
               <<"alicE_messages_are_properly_flushed_during_resumption_2046">>},
             {server,<<"localhost">>},
             {password,<<"matygrysa">>},
             {stream_management,true},
             {host,<<"localhost">>},
             {stream_id,<<"c723ab1bf95ca86b">>}]},
          [{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/project/big_tests/tests/sm_SUITE.erl"},
       {line,1226}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1754}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1263}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1195}]}]}}

Report log

sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_stop_c2s
{error,{thrown,{timeout,msg}}}

Report log


mssql_mnesia_24 / odbc_mssql_mnesia / 6d03575
Reports root/ big
OK: 3118 / Failed: 1 / User-skipped: 195 / Auto-skipped: 0

rest_client_SUITE:roster:add_and_remove_some_contacts_properly
{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,
          <<"bOb_add_and_remove_some_contacts_properly_2172@localhost/res1">>,
          escalus_tcp,<0.7935.2>,
          [{event_manager,<0.7878.2>},
           {server,<<"localhost">>},
           {username,
             <<"bOb_add_and_remove_some_contacts_properly_2172">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.7878.2>},
            {server,<<"localhost">>},
            {username,
              <<"bOb_add_and_remove_some_contacts_properly_2172">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,
             <<"bOb_add_and_remove_some_contacts_properly_2172">>},
           {server,<<"localhost">>},
           {host,<<"localhost">>},
           {port,5222},
           {auth,{escalus_auth,auth_plain}},
           {wspath,undefined},
           {username,
             <<"bOb_add_and_remove_some_contacts_properly_2172">>},
           {server,<<"localhost">>},
           {password,<<"makrolika">>},
           {stream_id,<<"6f73a5a7caab91d6">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {rest_client_SUITE,add_contact_check_roster_push,2,
       [{file,
          "/home/circleci/project/big_tests/tests/rest_client_SUITE.erl"},
        {line,1310}]},
     {lists,foreach,2,...

Report log


mysql_redis_24 / mysql_redis / 6d03575
Reports root/ big
OK: 3108 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 6d03575
Reports root/ big
OK: 1726 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 7, 2021

small_tests_24 / small_tests / 1af9d2d
Reports root / small


small_tests_23 / small_tests / 1af9d2d
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1af9d2d
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 1af9d2d
Reports root/ big
OK: 2709 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 1af9d2d
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 1af9d2d
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 1af9d2d
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 1af9d2d
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 1af9d2d
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 1af9d2d
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 1af9d2d
Reports root/ big
OK: 1908 / Failed: 1 / User-skipped: 313 / Auto-skipped: 0

sm_SUITE:parallel:messages_are_properly_flushed_during_resumption_p1_fsm_old
{error,
  {{badmatch,
     {error,
       {connection_step_failed,
         {#Fun<sm_SUITE.11.68776247>,
          {client,
            <<"alicE_messages_are_properly_flushed_during_resumption_p1_fsm_old_1612@localhost">>,
            escalus_tcp,<0.25546.1>,undefined,
            [{username,
               <<"alicE_messages_are_properly_flushed_during_resumption_p1_fsm_old_1612">>},
             {server,<<"localhost">>},
             {password,<<"matygrysa">>},
             {stream_management,true},
             {host,<<"localhost">>},
             {stream_id,<<"f6de8750d4604e67">>}]},
          [{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_p1_fsm_old/1-fun-1-',
      3,
      [{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
       {line,1270}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/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,1...

Report log


mysql_redis_24 / mysql_redis / 1af9d2d
Reports root/ big
OK: 3108 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 1af9d2d
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 1af9d2d
Reports root/ big
OK: 3112 / Failed: 1 / User-skipped: 195 / Auto-skipped: 0

service_domain_db_SUITE:db:db_reinserted_from_one_node_while_service_disabled_on_another
{error,
  {{badmatch,{ok,<<"dbgroup2">>}},
   [{service_domain_db_SUITE,
      db_reinserted_from_one_node_while_service_disabled_on_another,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,586}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1754}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1263}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1195}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / 1af9d2d
Reports root/ big
OK: 1726 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 1af9d2d
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me 👌🏽

@NelsonVides NelsonVides merged commit c138fdc into master Dec 7, 2021
@NelsonVides NelsonVides deleted the config-with-maps branch December 7, 2021 18:08
@Premwoik Premwoik modified the milestone: 5.1.0 May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants