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

Event pusher opts in a map #3603

Merged
merged 22 commits into from
Mar 25, 2022
Merged

Event pusher opts in a map #3603

merged 22 commits into from
Mar 25, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Mar 22, 2022

Use maps with defaults for mod_event_pusher options and introduce host types to the code.

Other changes:

  • The backend subsection was removed, because it did not introduce any value.
  • The backend is now reported separately by mod_event_pusher_push in config_metrics (for clarity)

Bug fixes

  • The handlers subsection was added for http to allow specifying multiple handlers - this was described in the docs, but not configurable. Big tests had lists that were impossible to get from TOML.
  • event_pusher has to handle filter_local_packet before inbox. Having the same priority for both made the result unpredictable and it was possible to make the tests fail by changing the order in which modules were started (wrong unread_count).

Left for the future (out of scope for now)

  • Enable support for dynamic domains in the module and update big tests to support them as well.
  • Refactor the wpool tests for outgoing pools.
  • More system metrics tests

Details in commit messages.

@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #3603 (dd4a097) into master (dca553d) will decrease coverage by 62.59%.
The diff coverage is 28.57%.

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

@@             Coverage Diff             @@
##           master    #3603       +/-   ##
===========================================
- Coverage   80.92%   18.33%   -62.60%     
===========================================
  Files         425      425               
  Lines       32241    32194       -47     
===========================================
- Hits        26091     5902    -20189     
- Misses       6150    26292    +20142     
Impacted Files Coverage Δ
src/config/mongoose_config_spec.erl 98.82% <ø> (-0.40%) ⬇️
src/ejabberd_c2s.erl 19.83% <0.00%> (-68.82%) ⬇️
src/event_pusher/mod_event_pusher_push_backend.erl 0.00% <0.00%> (-100.00%) ⬇️
src/event_pusher/mod_event_pusher_push_mnesia.erl 0.00% <0.00%> (-90.63%) ⬇️
...t_pusher/mod_event_pusher_push_plugin_defaults.erl 0.00% <0.00%> (-93.11%) ⬇️
src/event_pusher/mod_event_pusher_push_rdbms.erl 0.00% <0.00%> (-97.44%) ⬇️
src/mongoose_hooks.erl 16.24% <0.00%> (-79.15%) ⬇️
src/event_pusher/mod_event_pusher_push_plugin.erl 5.26% <9.09%> (-94.74%) ⬇️
src/event_pusher/mod_event_pusher_push.erl 4.12% <9.67%> (-90.62%) ⬇️
src/event_pusher/mod_event_pusher_http.erl 7.69% <12.50%> (-89.81%) ⬇️
... and 377 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 dca553d...04cf562. Read the comment docs.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

- Use host types
- It is best to register hooks last, so it is better not to have
  a dependency, which is started first
@mongoose-im

This comment was marked as outdated.

The event_pusher one has to run before the inbox one to ensure that
unread_count has not been yet updated by mod_inbox.

mod_inbox_rdbms:get_inbox_unread/4 function always adds 1 to the result

The order can't be easily swapped because of the possibility of having
asynchronous workers for inbox.
- 'push_notifications' still needs the HostType separately
  because the Acc can be just 'ok'
Drop the host type where it can be obtained from the acc
Host type is now obtained from the acc.

Also: don't use the config of the sender's domain (which can be hosted
by a different server) to figure out if the recipient's message
triggers a push notification
@chrzaszcz chrzaszcz force-pushed the mod_event_pusher-map branch 2 times, most recently from 24c1294 to 59a2cae Compare March 25, 2022 11:14
- Drop the 'backend' subsection, which provided no extra value
- Do not store backends in ets, the config is enough
- Order backends alphabetically
- Simplify config_metrics logic
- Use maps for opts
- Add a new option with a list of handlers - previously it was
  impossible to set up multiple handlers, which was against the docs.
- Process the path prefix on parsing (only once)
- Use host types
- Use maps for opts
- Use host types
- Use the wpool defaults from mongoose_config_spec
Use host types, update specs for module opts
- Use host types and module opts in a map
- Use maps for opts
- Add a type for exchange opts
- Make the code more straightforward
- Use maps for opts
- Use host types
- Use maps and defaults
- Use a separate helper for the HTTP handlers, because it is an item in
  a list, and thus it doesn't have a path
- Test wpool in a generic way, in the future we can rework other wpool
  tests to do the same
- Check multiple handlers for HTTP
- Do not test changing config on the fly, this is not supported
- Start the full module
- Host type is the same as domain, but in the future we can change this
- Configure event_pusher with maps
- Path should be binary without the leading '/'
- Remove obsolete options that had no effect
@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz marked this pull request as ready for review March 25, 2022 12:27
- Use opts in maps
- Start module always in init_per_testcase
- Set opts in maps
- Return the correct Acc in process_packet to avoid errors in logs
- Set opts in maps
- Set opts in maps
- Refactor code in a functional way
@mongoose-im

This comment was marked as outdated.

The backend can be reported from mod_event_pusher_push,
it looks cleaner this way.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Mar 25, 2022

small_tests_24 / small_tests / dd4a097
Reports root / small


small_tests_23 / small_tests / dd4a097
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


dynamic_domains_mysql_redis_24 / mysql_redis / dd4a097
Reports root/ big
OK: 2802 / Failed: 0 / User-skipped: 150 / Auto-skipped: 7


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


ldap_mnesia_24 / ldap_mnesia / dd4a097
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 399 / Auto-skipped: 7


ldap_mnesia_23 / ldap_mnesia / dd4a097
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 399 / Auto-skipped: 7


internal_mnesia_24 / internal_mnesia / dd4a097
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 358 / Auto-skipped: 7


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / dd4a097
Reports root/ big
OK: 1848 / Failed: 0 / User-skipped: 366 / Auto-skipped: 7


pgsql_mnesia_24 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 142 / Auto-skipped: 7


pgsql_mnesia_23 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 142 / Auto-skipped: 7


mysql_redis_24 / mysql_redis / dd4a097
Reports root/ big
OK: 3188 / Failed: 0 / User-skipped: 147 / Auto-skipped: 7


mssql_mnesia_24 / odbc_mssql_mnesia / dd4a097
Reports root/ big
OK: 3201 / Failed: 1 / User-skipped: 142 / Auto-skipped: 7

disco_and_caps_SUITE:disco_with_caps:user_cannot_query_friend_resources_with_unknown_node
{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 / dd4a097
Reports root/ big
OK: 1691 / Failed: 0 / User-skipped: 365 / Auto-skipped: 7


dynamic_domains_mysql_redis_24 / mysql_redis / dd4a097
Reports root/ big
OK: 2802 / Failed: 0 / User-skipped: 150 / Auto-skipped: 7


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / dd4a097
Reports root/ big
OK: 2819 / Failed: 0 / User-skipped: 133 / Auto-skipped: 7


ldap_mnesia_24 / ldap_mnesia / dd4a097
Reports root/ big
OK: 1501 / Failed: 1 / User-skipped: 402 / Auto-skipped: 7

rest_client_SUITE:messages:msg_is_sent_and_delivered_over_sse
{error,{{badmap,{error,timeout}},
    [{erlang,map_get,
         [data,{error,timeout}],
         [{error_info,#{module => erl_erts_errors}}]},
     {rest_client_SUITE,msg_is_sent_and_delivered_over_sse,1,
              [{file,"/home/circleci/project/big_tests/tests/rest_client_SUITE.erl"},
               {line,214}]},
     {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


ldap_mnesia_23 / ldap_mnesia / dd4a097
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 399 / Auto-skipped: 7


internal_mnesia_24 / internal_mnesia / dd4a097
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 358 / Auto-skipped: 7


pgsql_mnesia_24 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 142 / Auto-skipped: 7


pgsql_mnesia_23 / pgsql_mnesia / dd4a097
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 142 / Auto-skipped: 7


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / dd4a097
Reports root/ big
OK: 1847 / Failed: 1 / User-skipped: 366 / Auto-skipped: 7

sm_SUITE:parallel_unacknowledged_message_hook:unacknowledged_message_hook_bounce
{error,
  {{assertion_failed,assert,is_chat_message,
     [<<"msg-1">>],
     {xmlel,<<"message">>,
       [{<<"from">>,
         <<"bOb_unacknowledged_message_hook_bounce_1602@localhost/escalus-default-resource">>},
        {<<"to">>,
         <<"alicE_unacknowledged_message_hook_bounce_1606@localhost">>},
        {<<"xml:lang">>,<<"en">>},
        {<<"type">>,<<"chat">>}],
       [{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-3">>}]},
        {xmlel,<<"delay">>,
          [{<<"xmlns">>,<<"urn:xmpp:delay">>},
           {<<"stamp">>,<<"2022-03-25T16:01:41.228606Z">>},
           {<<"from">>,<<"localhost">>}],
          [{xmlcdata,<<"SM Storage">>}]}]},
     "<message from='bOb_unacknowledged_message_hook_bounce_1602@localhost/escalus-default-resource' to='alicE_unacknowledged_message_hook_bounce_1606@localhost' xml:lang='en' type='chat'><body>msg-3</body><delay xmlns='urn:xmpp:delay' stamp='2022-03-25T16:01:41.228606Z' from='localhost'>SM Storage</delay></message>"},
   [{escalus_new_assert,assert_true,2,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
       {line,84}]},
    {sm_SUITE,unacknowledged_message_hook_common,2,
      [{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
       {line,805}]},
    {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,"tes...

Report log


mysql_redis_24 / mysql_redis / dd4a097
Reports root/ big
OK: 3188 / Failed: 0 / User-skipped: 147 / Auto-skipped: 7


mssql_mnesia_24 / odbc_mssql_mnesia / dd4a097
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 142 / Auto-skipped: 7


riak_mnesia_24 / riak_mnesia / dd4a097
Reports root/ big
OK: 1691 / Failed: 0 / User-skipped: 365 / Auto-skipped: 7

@mongoose-im
Copy link
Collaborator

mongoose-im commented Mar 25, 2022

small_tests_24 / small_tests / 04cf562
Reports root / small


small_tests_23 / small_tests / 04cf562
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 04cf562
Reports root/ big
OK: 2826 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 04cf562
Reports root/ big
OK: 2826 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 04cf562
Reports root/ big
OK: 2809 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 04cf562
Reports root/ big
OK: 2826 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 04cf562
Reports root/ big
OK: 1507 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 04cf562
Reports root/ big
OK: 1507 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 04cf562
Reports root/ big
OK: 1548 / Failed: 0 / User-skipped: 358 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 04cf562
Reports root/ big
OK: 3200 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 04cf562
Reports root/ big
OK: 3200 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 04cf562
Reports root/ big
OK: 1855 / Failed: 0 / User-skipped: 366 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 04cf562
Reports root/ big
OK: 3195 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 04cf562
Reports root/ big
OK: 3200 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 04cf562
Reports root/ big
OK: 1698 / Failed: 0 / User-skipped: 365 / Auto-skipped: 0

@arcusfelis arcusfelis merged commit 227a7ee into master Mar 25, 2022
@arcusfelis arcusfelis deleted the mod_event_pusher-map branch March 25, 2022 16:19
@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

4 participants