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

Refactor/presences #4207

Merged
merged 11 commits into from
Jan 19, 2024
Merged

Refactor/presences #4207

merged 11 commits into from
Jan 19, 2024

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Jan 11, 2024

The main points are

Unify sets in the presence state record
The idea is to keep track of a smaller set of data, I'm assuming that jids that
would fall both on pres_f and pres_t are common, so we can keep track of them
only once instead of duplicating. This way we also give a meaning to the values
of the map underlying the implementation of sets.
Note too that this set is rarely updated: it is build once upon user starting a
presence session, and it is updated only on roster changes.

This is functionality that was deprecated from XMPP already back in 2003 (!),
and we still have code for it that has been surviving different XEP
advertisement updates as well as shuffling of presences code.
Note that this is not related to the https://xmpp.org/extensions/xep-0126.html
XEP, as this one achieves the same by usage of privacy lists specific for the
purpose, thereby establishing only a client-based protocal that is transparent
for the server.

ttps://www.rfc-editor.org/rfc/rfc6121.html#section-4.7.1 defines a strict list of valid types.

This should all make code more readable and hopefully have some minimal performance improvements.

@mongoose-im

This comment was marked as outdated.

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (61830ee) 84.32% compared to head (0a3f2e8) 84.35%.

Files Patch % Lines
src/mod_presence.erl 85.36% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4207      +/-   ##
==========================================
+ Coverage   84.32%   84.35%   +0.02%     
==========================================
  Files         552      552              
  Lines       33459    33436      -23     
==========================================
- Hits        28215    28204      -11     
+ Misses       5244     5232      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides marked this pull request as ready for review January 11, 2024 14:10
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 16, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 79547ea
Reports root/ big
OK: 382 / Failed: 0 / User-skipped: 40 / Auto-skipped: 0


small_tests_25 / small_tests / 79547ea
Reports root / small


small_tests_26 / small_tests / 79547ea
Reports root / small


small_tests_26_arm64 / small_tests / 79547ea
Reports root / small


ldap_mnesia_25 / ldap_mnesia / 79547ea
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 856 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / 79547ea
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 856 / Auto-skipped: 0


dynamic_domains_mysql_redis_26 / mysql_redis / 79547ea
Reports root/ big
OK: 4255 / Failed: 0 / User-skipped: 144 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 79547ea
Reports root/ big
OK: 4288 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 79547ea
Reports root/ big
OK: 4288 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 79547ea
Reports root/ big
OK: 4285 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


pgsql_cets_26 / pgsql_cets / 79547ea
Reports root/ big
OK: 4262 / Failed: 0 / User-skipped: 177 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / 79547ea
Reports root/ big
OK: 2413 / Failed: 2 / User-skipped: 716 / Auto-skipped: 0

pubsub_SUITE:dag+basic:publish_with_max_items_test
{error,{{badmatch,false},
    [{pubsub_tools,check_response,2,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,444}]},
     {pubsub_tools,receive_response,3,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,434}]},
     {pubsub_tools,receive_and_check_response,4,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,424}]},
     {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,1793}]},
     {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1302}]},
     {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1234}]}]}}

Report log

sm_SUITE:ping_timeout
{error,
  {{assertion_failed,assert,is_presence,
     {xmlel,<<"iq">>,
       [{<<"from">>,<<"localhost">>},
        {<<"to">>,
         <<"alice_ping_timeout_2051@localhost/escalus-default-resource">>},
        {<<"type">>,<<"get">>},
        {<<"id">>,<<"18ef4356ca52f7a5">>}],
       [{xmlel,<<"ping">>,[{<<"xmlns">>,<<"urn:xmpp:ping">>}],[]}]},
     "<iq from='localhost' to='alice_ping_timeout_2051@localhost/escalus-default-resource' type='get' id='18ef4356ca52f7a5'><ping xmlns='urn:xmpp:ping'/></iq>"},
   [{escalus_new_assert,assert_true,2,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
       {line,84}]},
    {sm_helper,initial_presence_step,2,
      [{file,"/home/circleci/project/big_tests/tests/sm_helper.erl"},
       {line,135}]},
    {escalus_connection,connection_step,2,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
       {line,163}]},
    {lists,foldl_1,3,[{file,"lists.erl"},{line,1599}]},
    {escalus_connection,start,2,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
       {line,145}]},
    {sm_helper,connect_spec,3,
      [{file,"/home/circleci/project/big_tests/tests/sm_helper.erl"},
       {line,153}]},
    {sm_SUITE,ping_timeout,1,
      [{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
       {line,625}]},
    {test_server,ts_tc,3,[{file,"test_ser...

Report log


pgsql_mnesia_25 / pgsql_mnesia / 79547ea
Reports root/ big
OK: 4677 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / 79547ea
Reports root/ big
OK: 4677 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / 79547ea
Reports root/ big
OK: 4656 / Failed: 0 / User-skipped: 139 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / 79547ea
Reports root/ big
OK: 4674 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0

graphql_muc_light_SUITE:user:user_muc_light:end_per_group
{error,
 {{unregistering_failed,
   {amount,1},
   {unregistered_items,
  [{{<<"_user_send_message_to_room_errors_1018">>,
     [{escalus_event_mgr,<0.23277.0>},
    {tc_name,user_send_message_to_room_errors},
    {escalus_cleaner,<0.23276.0>},
    {watchdog,<0.23275.0>},
    {muc_light_host,<<"muclight.localhost">>},
    {secondary_muc_light_host,<<"muclight.localhost.bis">>},
    {schema_endpoint,user},
    {{ejabberd_cwd,mongooseim@localhost},
     "/home/circleci/project/_build/mim1/rel/mongooseim"},
    {preset,"odbc_mssql_mnesia"},
    {mim_data_dir,
     "/home/circleci/project/big_tests/tests/graphql_muc_light_SUITE_data"},
    {tc_logfile,
     "https://circleci-mim-results.s3.eu-central-1.amazonaws.com/PR/4207/205876/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%4003c247d4a9fa.2024-01-16_16.30.11/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-01-16_16.34.48/graphql_muc_light_suite.user_send_message_to_room_errors.61826.html"},
    {tc_group_properties,[{name,user_muc_light},parallel]},
    {tc_group_path,[[{name,user}]]},
    {data_dir,
     "/home/circleci/project/big_tests/_build/default/lib/mongoose_tests/ebin/graphql_muc_light_SUITE_data/"},
    {priv_dir,
     "https://circleci-mim-results.s3.eu-central-1.amazonaws.com/PR/4207/205876/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%4003c247d4a9fa.2024-01-16_16.30.11/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-01-16_16.34.48/log_private/"},
    {{saved_modules,mongooseim@localhost,<<"localhost">>},
     #{mod_vcard =>
      #{matches => 30,
        host => {prefix,<<"vjud.">>},
        search => true,backend => rdbms,iqdisc => pa...

Report log


internal_mnesia_26 / internal_mnesia / 79547ea
Reports root/ big
OK: 2415 / Failed: 0 / User-skipped: 716 / Auto-skipped: 0

Copy link
Contributor

@arcusfelis arcusfelis left a comment

Choose a reason for hiding this comment

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

looks ok, one type

get_presence_type(Acc) ->
case mongoose_acc:stanza_type(Acc) of
%% Note that according to https://www.rfc-editor.org/rfc/rfc6121.html#section-4.7.1
%% there is no defailt value nor "available" is considered a valid type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%% there is no defailt value nor "available" is considered a valid type.
%% there is no default value nor "available" is considered a valid type.

The idea is to keep track of a smaller set of data, I'm assuming that jids that
would fall both on pres_f and pres_t are common, so we can keep track of them
only once instead of duplicating. This way we also give a meaning to the values
of the map underlying the implementation of sets.

Note too that this set is rarely updated: it is build once upon user starting a
presence session, and it is updated only on roster changes.
Available and invisible sets can be unified into a map of tagged jids.
These sets vary a lot more that subscriptions, as they are changed as peers
connect and disconnect.
This is functionality that was deprecated from XMPP already back in 2003 (!),
and we still have code for it that has been surviving different XEP
advertisement updates as well as shuffling of presences code.

Note that this is not related to the https://xmpp.org/extensions/xep-0126.html
XEP, as this one achieves the same by usage of privacy lists specific for the
purpose, thereby establishing only a client-based protocal that is transparent
for the server.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 19, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 0a3f2e8
Reports root/ big
OK: 382 / Failed: 0 / User-skipped: 40 / Auto-skipped: 0


small_tests_25 / small_tests / 0a3f2e8
Reports root / small


small_tests_26 / small_tests / 0a3f2e8
Reports root / small


small_tests_26_arm64 / small_tests / 0a3f2e8
Reports root / small


dynamic_domains_mysql_redis_26 / mysql_redis / 0a3f2e8
Reports root/ big
OK: 4255 / Failed: 0 / User-skipped: 144 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 0a3f2e8
Reports root/ big
OK: 4288 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / 0a3f2e8
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 856 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 0a3f2e8
Reports root/ big
OK: 4288 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 0a3f2e8
Reports root/ big
OK: 4285 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


pgsql_cets_26 / pgsql_cets / 0a3f2e8
Reports root/ big
OK: 4261 / Failed: 0 / User-skipped: 178 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / 0a3f2e8
Reports root/ big
OK: 2415 / Failed: 0 / User-skipped: 716 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 0a3f2e8
Reports root/ big
OK: 4677 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / 0a3f2e8
Reports root/ big
OK: 4656 / Failed: 0 / User-skipped: 139 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / 0a3f2e8
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 856 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / 0a3f2e8
Reports root/ big
OK: 4677 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / 0a3f2e8
Reports root/ big
OK: 4674 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0

@arcusfelis arcusfelis merged commit 1ada930 into master Jan 19, 2024
4 checks passed
@arcusfelis arcusfelis deleted the refactor/presences branch January 19, 2024 10:22
@jacekwegr jacekwegr added this to the 6.2.1 milestone Apr 3, 2024
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.

4 participants