Remove unnecessary hooks #3990
Remove unnecessary hooks #3990
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3990 +/- ##
==========================================
+ Coverage 82.06% 82.10% +0.03%
==========================================
Files 536 536
Lines 33902 33859 -43
==========================================
- Hits 27822 27799 -23
+ Misses 6080 6060 -20
... and 7 files with indirect coverage changes 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 in Codecov by Sentry. |
jacekwegr
force-pushed
the
remove-unnecessary-hooks
branch
from
ac22875
to
463e142
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jacekwegr
force-pushed
the
remove-unnecessary-hooks
branch
from
463e142
to
3f5b34b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jacekwegr
force-pushed
the
remove-unnecessary-hooks
branch
from
3f5b34b
to
f978156
Compare
This comment was marked as outdated.
This comment was marked as outdated.
src/hooks/mongoose_hooks.erl
Outdated
| @@ -419,19 +412,6 @@ user_ping_response(HostType, Acc, JID, Response, TDelta) -> | |||
| Params = #{jid => JID, response => Response, time_delta => TDelta}, | |||
| run_hook_for_host_type(user_ping_response, HostType, Acc, Params). | |||
|
|
|||
| %%% @doc The `vcard_set' hook is called to inform that the vcard | |||
There was a problem hiding this comment.
it is pretty useful when we need to notify about need to download vcard again (if avatar is stored there).
jacekwegr
force-pushed
the
remove-unnecessary-hooks
branch
from
f978156
to
f4b49aa
Compare
This comment was marked as outdated.
This comment was marked as outdated.
| text => <<"Found a bridge, relay_to_bridge next.">>, | ||
| type => Type, | ||
| myname => StateData#state.myname, server => StateData#state.server}), | ||
| NewStateData = StateData#state{bridge={Mod, Fun}}, |
There was a problem hiding this comment.
Once you remove this, I think #state.bridge is unused (also, relay_to_bridge state).
So, we need to check what are the bridges. It could be even non-tested feature.
You can probably remove bridge attribute and relay_to_bridge statename (i.e. exported function callback). And check if tests are still passing.
|
small_tests_25_arm64 / small_tests / f2ce066 small_tests_24 / small_tests / f2ce066 small_tests_25 / small_tests / f2ce066 ldap_mnesia_24 / ldap_mnesia / f2ce066 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f2ce066 ldap_mnesia_25 / ldap_mnesia / f2ce066 dynamic_domains_mysql_redis_25 / mysql_redis / f2ce066 graphql_session_SUITE:admin_session:admin_session_cli:admin_kick_user_session{error,
{function_clause,
[{graphql_helper,get_error,
[1,
{{<<"200">>,<<"OK">>},
#{<<"data">> =>
#{<<"session">> =>
#{<<"kickUserSession">> =>
#{<<"code">> => null,
<<"jid">> =>
<<"[email protected]/res1">>,
<<"kicked">> => true,
<<"message">> => <<"Session kicked">>}}}}}],
[]},
{graphql_helper,get_err_msg,2,
[{file,
"/home/circleci/project/big_tests/tests/graphql_helper.erl"},
{line,227}]},
{graphql_session_SUITE,admin_kick_user_session_story,3,
[{file,
"/home/circleci/project/big_tests/tests/graphql_session_SUITE.erl"},
{line,441}]},
{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,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_pgsql_mnesia_25 / pgsql_mnesia / f2ce066 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / f2ce066 internal_mnesia_25 / internal_mnesia / f2ce066 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / f2ce066 pgsql_mnesia_24 / pgsql_mnesia / f2ce066 pgsql_mnesia_25 / pgsql_mnesia / f2ce066 mssql_mnesia_25 / odbc_mssql_mnesia / f2ce066 mysql_redis_25 / mysql_redis / f2ce066 dynamic_domains_mysql_redis_25 / mysql_redis / f2ce066 |
This PR removes some of the hooks that had no handlers registered to them in order to lower the code complexity.
List of the hooks that have no handlers, with an explanation why they were or weren't removed:
user_ping_timeout,c2s_update_presence- these hooks were already removedadhoc_sm_commands- if we already have local_commands, it makes sense to also have sm_commandsvcard_set- might be used to notify about the need to download the vcard againcheck_bl_c2s- it is useful and doesn’t add too much logicforbidden_session_hook- it seems useful and doesn’t complicate the code muchsession_opening_allowed_for_user- can be removed, but I think it’s better to leave it as isset_presence_hook- doesn’t complicate logic very much and can be useful in the futureroster_groups- might be useful for some implementations that don’t allow some roster groups to subscribe to pubsubfind_s2s_bridge- always returnedundefinedand was causing unnecessary branchings2s_allow_host- might be useful in the future and doesn’t complicate code very muchs2s_connect_hook- removed, always returnedoks2s_send_packet,s2s_receive_packet- left because it might be useful to know whether a message is routedinvitation_sent- left because there is a TODO inmod_inbbox_muc.erljoin_room,reave_room,room_packet- not used anywhere, but may be needed in some implementationspubsub_create_node,pubsub_delete_node,pubsub_publish_item- not very useful as this information is already broadcast. Furthermore, removing these hooks allows for some code to be simplified.mam_retraction,mam_muc_retraction- can be useful for implementations that use message retractions