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

Eliminate gen_mod:get_opt_subhost #3626

Merged
merged 4 commits into from
Apr 8, 2022
Merged

Eliminate gen_mod:get_opt_subhost #3626

merged 4 commits into from
Apr 8, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Apr 7, 2022

This obsolete function was used in two places and both can be eliminated - see the commit messages for details.

The most important change is that the (un)register_subhost hooks were called only for mod_muc and not for other modules, which caused them to be called twice for mod_muc and only once (by mongoose_lazy_routing) for other modules. The issue is that lazy execution is too late, which would be caught if we tested global distribution e.g. with mod_muc_light. The solution is to simply call the hook in mongoose_subdomain_core.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 7, 2022

small_tests_24 / small_tests / b5baf3a
Reports root / small


small_tests_23 / small_tests / b5baf3a
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / b5baf3a
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / b5baf3a
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / b5baf3a
Reports root/ big
OK: 2841 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / b5baf3a
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / b5baf3a
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / b5baf3a
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / b5baf3a
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / b5baf3a
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / b5baf3a
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / b5baf3a
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / b5baf3a
Reports root/ big
OK: 3227 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / b5baf3a
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / b5baf3a
Reports root/ big

inbox_SUITE:async_pools:bin:timeout_cleaner_flush_all
{error,
 {{inbox_size,ok,
   [{times,1,
   {error,
    {inbox_size,0,
     [{times,1,
     #{respond_iq =>
      {xmlel,<<"iq">>,
       [{<<"from">>,<<"kate_timeout_cleaner_flush_all_784@localhost">>},
        {<<"to">>,
         <<"kate_timeout_cleaner_flush_all_784@localhost/res1">>},
        {<<"id">>,<<"5ca98f38903f4d5bf9dd35e6a33aeecd">>},
        {<<"type">>,<<"result">>}],
       [{xmlel,<<"fin">>,
         [{<<"xmlns">>,<<"erlang-solutions.com:xmpp:inbox:0">>}],
         [{xmlel,<<"active-conversations">>,[],[{xmlcdata,<<"1">>}]},
        {xmlel,<<"count">>,[],[{xmlcdata,<<"1">>}]},
        {xmlel,<<"unread-messages">>,[],[{xmlcdata,<<"1">>}]}]}]},
       respond_messages =>
      [{xmlel,<<"message">>,
        [{<<"from">>,<<"kate_timeout_cleaner_flush_all_784@localhost">>},
         {<<"to">>,
        <<"kate_timeout_cleaner_flush_all_784@localhost/res1">>},
         {<<"id">>,<<"1649-337177-302614">>}],
        [{xmlel,<<"result">>,
        [{<<"xmlns">>,<<"erlang-solutions.com:xmpp:inbox:0">>},
         {<<"unread">>,<<"1">>},
         {<<"queryid">>,<<"5ca98f38903f4d5bf9dd35e6a33aeecd">>}],
        [{xmlel,<<"forwarded">>,
          [{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
          [{xmlel,<<"delay">>,
          [{<<"xmlns">>,<<"urn:xmpp:delay">>},
           {<<"stamp">>,<<"2022-04-07T13:12:57.140472Z">>}],
          []},
           {xmlel,<<"message">>,
          [{<<"to">>,
            <<"kate_timeout_cleaner_flush_all_784@localhost">>},
           {<<"id">...

Report log

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 7, 2022

small_tests_24 / small_tests / ddc8ab2
Reports root / small


small_tests_23 / small_tests / ddc8ab2
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / ddc8ab2
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / ddc8ab2
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / ddc8ab2
Reports root/ big
OK: 2841 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / ddc8ab2
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / ddc8ab2
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / ddc8ab2
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / ddc8ab2
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / ddc8ab2
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / ddc8ab2
Reports root/ big
OK: 3227 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / ddc8ab2
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / ddc8ab2
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / ddc8ab2
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / ddc8ab2
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #3626 (f7e4f20) into options-only-in-maps (d539679) will increase coverage by 4.72%.
The diff coverage is 100.00%.

❗ Current head f7e4f20 differs from pull request most recent head fb4ed4e. Consider uploading reports for the commit fb4ed4e to get more accurate results

@@                   Coverage Diff                    @@
##           options-only-in-maps    #3626      +/-   ##
========================================================
+ Coverage                 76.26%   80.99%   +4.72%     
========================================================
  Files                       427      427              
  Lines                     32098    32085      -13     
========================================================
+ Hits                      24479    25986    +1507     
+ Misses                     7619     6099    -1520     
Impacted Files Coverage Δ
src/ejabberd_admin.erl 59.43% <ø> (+1.10%) ⬆️
src/gen_mod.erl 76.28% <ø> (-0.48%) ⬇️
src/gen_mod_deps.erl 97.10% <ø> (ø)
src/mongoose_hooks.erl 95.43% <ø> (ø)
src/mongoose_modules.erl 100.00% <ø> (ø)
src/config/mongoose_config_parser.erl 91.83% <100.00%> (ø)
src/domain/mongoose_subdomain_core.erl 89.71% <100.00%> (+0.19%) ⬆️
src/mod_muc.erl 74.23% <100.00%> (-0.19%) ⬇️
src/mongoose_router.erl 85.71% <100.00%> (-1.79%) ⬇️
src/ejabberd.erl 55.00% <0.00%> (-5.00%) ⬇️
... and 49 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 d539679...fb4ed4e. Read the comment docs.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 7, 2022

small_tests_24 / small_tests / cc3b813
Reports root / small


small_tests_23 / small_tests / cc3b813
Reports root / small


dynamic_domains_mysql_redis_24 / mysql_redis / cc3b813
Reports root/ big
OK: 2841 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / cc3b813
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / cc3b813
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / cc3b813
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / cc3b813
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / cc3b813
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / cc3b813
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / cc3b813
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / cc3b813
Reports root/ big
OK: 3227 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / cc3b813
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / cc3b813
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / cc3b813
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review April 7, 2022 16:18
It was used in two places:
- For ejabberd_admin, where the function was not usable from the CLI
  and would not work with dynamic domains, so it was removed.
  There is stil an utility function in mod_muc, should anyone need it.
- For mod_muc, it was used to trigger the (un)register_subhost hooks,
  which are triggered by mongoose_router anyway when routes are added
  and all other modules including mod_muc_light are making use of this
  fact. mod_muc was the only exception here.
Hooks should be called when the subdomain is (un)registered
istead of when it is lazily added to the routing table.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 7, 2022

small_tests_24 / small_tests / f7e4f20
Reports root / small


small_tests_23 / small_tests / f7e4f20
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / f7e4f20
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f7e4f20
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / f7e4f20
Reports root/ big
OK: 2841 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / f7e4f20
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / f7e4f20
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / f7e4f20
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / f7e4f20
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / f7e4f20
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / f7e4f20
Reports root/ big
OK: 3227 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / f7e4f20
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / f7e4f20
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / f7e4f20
Reports root/ big
OK: 3230 / Failed: 2 / User-skipped: 142 / Auto-skipped: 0

service_domain_db_SUITE:db:db_record_is_ignored_if_domain_static
{error,
  {{badrpc,timeout},
   [{distributed_helper,rpc,
      [#{node => mongooseim@localhost},service_domain_db,sync_local,[]],
      [{file,
         "/home/circleci/project/big_tests/tests/distributed_helper.erl"},
       {line,117}]},
    {service_domain_db_SUITE,sync_local,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,1147}]},
    {service_domain_db_SUITE,db_record_is_ignored_if_domain_static,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,497}]},
    {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}]}]}}

Report log

service_domain_db_SUITE:db:db_get_all_static
{error,
  {{badrpc,{'EXIT',{timeout,{gen_server,call,[service_domain_db,ping]}}}},
   [{distributed_helper,rpc,
      [#{node => mongooseim@localhost},service_domain_db,sync_local,[]],
      [{file,
         "/home/circleci/project/big_tests/tests/distributed_helper.erl"},
       {line,117}]},
    {service_domain_db_SUITE,sync_local,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,1147}]},
    {service_domain_db_SUITE,sync,0,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,1126}]},
    {service_domain_db_SUITE,db_get_all_static,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,350}]},
    {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}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / f7e4f20
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 7, 2022

small_tests_24 / small_tests / fb4ed4e
Reports root / small


small_tests_23 / small_tests / fb4ed4e
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / fb4ed4e
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / fb4ed4e
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / fb4ed4e
Reports root/ big
OK: 2841 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / fb4ed4e
Reports root/ big
OK: 2858 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / fb4ed4e
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / fb4ed4e
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / fb4ed4e
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / fb4ed4e
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / fb4ed4e
Reports root/ big
OK: 3232 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / fb4ed4e
Reports root/ big
OK: 3227 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / fb4ed4e
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / fb4ed4e
Reports root/ big
OK: 3242 / Failed: 2 / User-skipped: 142 / Auto-skipped: 0

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription
{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_1942@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_1942@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"7QGt9Zy5VP+3bVEft42Tqg==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {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,1224}]}]}}

Report log

pep_SUITE:pep_tests:publish_and_notify_test
{error,{{assertion_failed,assert_many,false,[is_presence],[],[]},
    [{escalus_new_assert,assert_true,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
                {line,84}]},
     {escalus_story,'-make_all_clients_friends/1-fun-0-',2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,109}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
     {escalus_utils,distinct_pairs,2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,60}]},
     {escalus_story,make_all_clients_friends,1,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,106}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / fb4ed4e
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

Copy link
Contributor

@Premwoik Premwoik left a comment

Choose a reason for hiding this comment

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

Looks good :)

Base automatically changed from options-only-in-maps to master April 8, 2022 06:44
@Premwoik Premwoik merged commit 4f6a845 into master Apr 8, 2022
@Premwoik Premwoik deleted the subhost-fix branch April 8, 2022 06:45
@Premwoik Premwoik added this to the 5.1.0 milestone 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

3 participants