Refactored hook handlers in ejabberd_sm module #3763
Conversation
|
small_tests_24 / small_tests / 88d2d7e small_tests_25 / small_tests / 88d2d7e dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 88d2d7e ldap_mnesia_24 / ldap_mnesia / 88d2d7e ldap_mnesia_25 / ldap_mnesia / 88d2d7e dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 88d2d7e dynamic_domains_mysql_redis_25 / mysql_redis / 88d2d7e internal_mnesia_25 / internal_mnesia / 88d2d7e pgsql_mnesia_24 / pgsql_mnesia / 88d2d7e elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 88d2d7e pgsql_mnesia_25 / pgsql_mnesia / 88d2d7e mysql_redis_25 / mysql_redis / 88d2d7e mssql_mnesia_25 / odbc_mssql_mnesia / 88d2d7e riak_mnesia_24 / riak_mnesia / 88d2d7e small_tests_24 / small_tests / 88d2d7e small_tests_25 / small_tests / 88d2d7e |
Codecov ReportBase: 82.84% // Head: 82.81% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3763 +/- ##
==========================================
- Coverage 82.84% 82.81% -0.04%
==========================================
Files 531 531
Lines 34069 34083 +14
==========================================
+ Hits 28226 28227 +1
- Misses 5843 5856 +13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
pawlooss1
force-pushed
the
hooks/ejabberd_sm
branch
from
88d2d7e
to
0196a5d
Compare
|
small_tests_24 / small_tests / 0196a5d small_tests_25 / small_tests / 0196a5d dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0196a5d ldap_mnesia_24 / ldap_mnesia / 0196a5d dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 0196a5d dynamic_domains_mysql_redis_25 / mysql_redis / 0196a5d ldap_mnesia_25 / ldap_mnesia / 0196a5d dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 0196a5d internal_mnesia_25 / internal_mnesia / 0196a5d pgsql_mnesia_24 / pgsql_mnesia / 0196a5d elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 0196a5d pgsql_mnesia_25 / pgsql_mnesia / 0196a5d mssql_mnesia_25 / odbc_mssql_mnesia / 0196a5d mysql_redis_25 / mysql_redis / 0196a5d riak_mnesia_24 / riak_mnesia / 0196a5d |
src/mongoose_hooks.erl
Outdated
| Params = #{user => LUser, server => LServer}, | ||
| Args = [LUser, LServer], | ||
| ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), | ||
| HostType = mongoose_acc:host_type(Acc), | ||
| run_hook_for_host_type(remove_user, HostType, Acc, [LUser, LServer]). | ||
| run_hook_for_host_type(remove_user, HostType, Acc, ParamsWithLegacyArgs). |
There was a problem hiding this comment.
All right, this is a hook that I always wanted to touch and never could, so this is a good chance 😃
The issue is that here we pass user and server, not knowing if they have been normalised already. See how so many subscribers will do again and again jid:nameprep and jid:nodeprep, or jid:make with both, just so that we can be sure they were normalised.
I think that the legacy arguments should stay as they are, but, the new argument should simply be a jid, a-la Jid = jid:make_bare(User, Server), and that way subscribers can be sure the string has been normalised.
In the case of the handler for ejabberd_sm it might feel redundant, we'll need to pattern match on the jid #jid{luser = LUser, lserver = LServer} to extract them, but at this point we can be sure we could omit nameprepping them later on.
pawlooss1
force-pushed
the
hooks/ejabberd_sm
branch
from
0196a5d
to
c5e70f8
Compare
|
small_tests_24 / small_tests / c5e70f8 small_tests_25 / small_tests / c5e70f8 ldap_mnesia_24 / ldap_mnesia / c5e70f8 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / c5e70f8 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / c5e70f8 ldap_mnesia_25 / ldap_mnesia / c5e70f8 dynamic_domains_mysql_redis_25 / mysql_redis / c5e70f8 pgsql_mnesia_24 / pgsql_mnesia / c5e70f8 internal_mnesia_25 / internal_mnesia / c5e70f8 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / c5e70f8 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / c5e70f8 riak_mnesia_24 / riak_mnesia / c5e70f8 pgsql_mnesia_25 / pgsql_mnesia / c5e70f8 mysql_redis_25 / mysql_redis / c5e70f8 mssql_mnesia_25 / odbc_mssql_mnesia / c5e70f8 |
pawlooss1
force-pushed
the
hooks/ejabberd_sm
branch
from
c5e70f8
to
0ba7a50
Compare
|
small_tests_24 / small_tests / 0ba7a50 small_tests_25 / small_tests / 0ba7a50 ldap_mnesia_24 / ldap_mnesia / 0ba7a50 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0ba7a50 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 0ba7a50 ldap_mnesia_25 / ldap_mnesia / 0ba7a50 dynamic_domains_mysql_redis_25 / mysql_redis / 0ba7a50 pgsql_mnesia_24 / pgsql_mnesia / 0ba7a50 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 0ba7a50 internal_mnesia_25 / internal_mnesia / 0ba7a50 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 0ba7a50 pgsql_mnesia_25 / pgsql_mnesia / 0ba7a50 mssql_mnesia_25 / odbc_mssql_mnesia / 0ba7a50 mysql_redis_25 / mysql_redis / 0ba7a50 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,617}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}riak_mnesia_24 / riak_mnesia / 0ba7a50 mysql_redis_25 / mysql_redis / 0ba7a50 muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive{error,{{assertion_failed,assert,is_groupchat_message,
[<<"Restorable message">>],
undefined,"undefined"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{muc_SUITE,wait_for_mam_result,3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4394}]},
{muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4130}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4126}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}mysql_redis_25 / mysql_redis / 0ba7a50 |
This PR changes all hook handlers in
ejabberd_smmodule togen_hookformat.