-
Notifications
You must be signed in to change notification settings - Fork 429
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
Refactored hook handlers in mod_ping #3821
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportBase: 82.97% // Head: 83.03% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3821 +/- ##
==========================================
+ Coverage 82.97% 83.03% +0.05%
==========================================
Files 528 528
Lines 33885 33891 +6
==========================================
+ Hits 28117 28142 +25
+ Misses 5768 5749 -19
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. |
0ea3155
to
8977a21
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8977a21
to
23ed1e9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8712c1e
to
5d7fb6d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
5d7fb6d
to
8007ff7
Compare
small_tests_24 / small_tests / 8007ff7 small_tests_25 / small_tests / 8007ff7 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 8007ff7 ldap_mnesia_24 / ldap_mnesia / 8007ff7 ldap_mnesia_25 / ldap_mnesia / 8007ff7 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 8007ff7 dynamic_domains_mysql_redis_25 / mysql_redis / 8007ff7 internal_mnesia_25 / internal_mnesia / 8007ff7 pubsub_SUITE:tree+basic:publish_with_max_items_test{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bob_publish_with_max_items_test_1527@localhost/res1">>,
escalus_tcp,<0.21947.1>,
[{event_manager,<0.21852.1>},
{server,<<"localhost">>},
{username,<<"bOb_publish_with_max_items_test_1527">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.21852.1>},
{server,<<"localhost">>},
{username,<<"bOb_publish_with_max_items_test_1527">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"bob_publish_with_max_items_test_1527">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"bOb_publish_with_max_items_test_1527">>},
{server,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"a0af40f829c40b32">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{pubsub_tools,receive_notification,3,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,504}]},
{pubsub_tools,receive_item_notification,4,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,303}]},
{pubsub_... pgsql_mnesia_24 / pgsql_mnesia / 8007ff7 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 8007ff7 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 8007ff7 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}]}]}} riak_mnesia_24 / riak_mnesia / 8007ff7 mysql_redis_25 / mysql_redis / 8007ff7 mssql_mnesia_25 / odbc_mssql_mnesia / 8007ff7 pgsql_mnesia_25 / pgsql_mnesia / 8007ff7 graphql_server_SUITE:admin_http:clustering_http_tests:remove_dead_from_cluster_http{error,
{{badrpc,
{'EXIT',
{#{cluster_member => mongooseim@localhost,
reason => set_extra_db_nodes_failed,
result => {ok,[]}},
[{mongoose_cluster,set_extra_db_nodes,1,
[{file,"/home/circleci/project/src/mongoose_cluster.erl"},
{line,132}]},
{mongoose_cluster,unsafe_join,2,
[{file,"/home/circleci/project/src/mongoose_cluster.erl"},
{line,116}]},
{mongoose_cluster,with_app_stopped,2,
[{file,"/home/circleci/project/src/mongoose_cluster.erl"},
{line,221}]},
{global,trans,4,[{file,"global.erl"},{line,463}]},
{mongoose_cluster,join,1,[]}]}}},
[{distributed_helper,rpc,
[#{node => mongooseim3@localhost,timeout => 60000},
mongoose_cluster,join,
[mongooseim@localhost]],
[{file,
"/home/circleci/project/big_tests/tests/distributed_helper.erl"},
{line,121}]},
{graphql_server_SUITE,remove_dead_from_cluster_http,1,
[{file,
"/home/circleci/project/big_tests/tests/graphql_server_SUITE.erl"},
{line,247}]},
{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}]}]}} dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 8007ff7 pgsql_mnesia_25 / pgsql_mnesia / 8007ff7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty straightforward, only one observation 👌🏽
src/mod_ping.erl
Outdated
|
||
-spec user_online(Acc, Params, Extra) -> {ok, Acc} when | ||
Acc :: ok, | ||
Params :: #{sid := 'undefined' | ejabberd_sm:sid()}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 'undefined'
necessary here? It will fail with a function_clause if so 🤔 Also the apostrophes aren't needed, it could be just undefined
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took it from mongoose_hooks:sm_remove_connection_hook/5
and mongoose_hooks:sm_register_connection_hook/4
's spec. You're right though, for this handler it makes no sense. I handler fails we're catching error and proceed with other handlers, so I guess there is no need to implement such function clause. I'll simply remove it from spec.
0f576f2
to
cdc567b
Compare
small_tests_24 / small_tests / 0f576f2 small_tests_25 / small_tests / 0f576f2 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0f576f2 ldap_mnesia_24 / ldap_mnesia / 0f576f2 dynamic_domains_mysql_redis_25 / mysql_redis / 0f576f2 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 0f576f2 ldap_mnesia_25 / ldap_mnesia / 0f576f2 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 0f576f2 internal_mnesia_25 / internal_mnesia / 0f576f2 pgsql_mnesia_24 / pgsql_mnesia / 0f576f2 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 0f576f2 pgsql_mnesia_25 / pgsql_mnesia / 0f576f2 riak_mnesia_24 / riak_mnesia / 0f576f2 mysql_redis_25 / mysql_redis / 0f576f2 mssql_mnesia_25 / odbc_mssql_mnesia / 0f576f2 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}]}]}} |
small_tests_24 / small_tests / cdc567b small_tests_25 / small_tests / cdc567b ldap_mnesia_24 / ldap_mnesia / cdc567b dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / cdc567b dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / cdc567b ldap_mnesia_25 / ldap_mnesia / cdc567b dynamic_domains_mysql_redis_25 / mysql_redis / cdc567b internal_mnesia_25 / internal_mnesia / cdc567b dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / cdc567b disco_and_caps_SUITE:disco_with_caps:user_cannot_query_friend_resources_with_unknown_node{error,{{assertion_failed,assert_many,false,[is_roster_set],[],[]},
[{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,108}]},
{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_1,3,[{file,"lists.erl"},{line,1355}]},
{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,1350}]},
{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}]}]}} pgsql_mnesia_24 / pgsql_mnesia / cdc567b elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / cdc567b riak_mnesia_24 / riak_mnesia / cdc567b mysql_redis_25 / mysql_redis / cdc567b pgsql_mnesia_25 / pgsql_mnesia / cdc567b mssql_mnesia_25 / odbc_mssql_mnesia / cdc567b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌🏽
This PR changes all hook handlers in
mod_ping
module togen_hook
format.