Refactor/roster #4209

NelsonVides · 2024-01-12T23:25:21Z

This modernises a bit code in mod_roster, by using exml_query, preferring tagged jids for type information, and most importantly, refactoring RDBMS operations to be transactional and use primary keys, see for example how priv/pg.sql#L56 has no use of primary keys and lots of redundant indexes.

codecov · 2024-01-19T10:55:33Z

Codecov Report

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

Comparison is base (1ada930) 84.33% compared to head (6ace2c0) 84.34%.
Report is 16 commits behind head on master.

Files	Patch %	Lines
src/roster/mod_roster.erl	86.00%	7 Missing ⚠️
src/roster/mod_shared_roster_ldap.erl	50.00%	2 Missing ⚠️
src/roster/mod_roster_rdbms.erl	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4209      +/-   ##
==========================================
+ Coverage   84.33%   84.34%   +0.01%     
==========================================
  Files         552      552              
  Lines       33436    33413      -23     
==========================================
- Hits        28198    28182      -16     
+ Misses       5238     5231       -7

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

Use the roster list as the accumulator of the hook, for more intentionality.

pawlooss1

Thank you for the refactor ❤️ I left some comments with my thoughts to consider.

src/roster/mod_roster_rdbms.erl

src/roster/mod_roster.erl

mongoose-im · 2024-02-20T20:14:07Z

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

small_tests_25 / small_tests / 6ace2c0
Reports root / small

small_tests_26 / small_tests / 6ace2c0
Reports root / small

small_tests_26_arm64 / small_tests / 6ace2c0
Reports root / small

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

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

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

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

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

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

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 6ace2c0
Reports root/ big
OK: 4284 / Failed: 1 / User-skipped: 114 / Auto-skipped: 0

graphql_muc_light_SUITE:domain_admin:domain_admin_muc_light:admin_create_room_with_unprepped_id

{error,
  {{badmatch,null},
   [{graphql_muc_light_SUITE,admin_create_room_with_unprepped_id,1,
      [{file,
         "/home/circleci/project/big_tests/tests/graphql_muc_light_SUITE.erl"},
       {line,1155}]},
    {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

graphql_muc_light_SUITE:domain_admin:domain_admin_muc_light:end_per_group

{error,
 {{unregistering_failed,
   {amount,3},
   {unregistered_items,
  [{{<<"_domain_admin_create_room_no_permission_1104">>,
     [{escalus_event_mgr,<0.24704.0>},
    {tc_name,domain_admin_create_room_no_permission},
    {escalus_cleaner,<0.24703.0>},
    {watchdog,<0.24702.0>},
    {muc_light_host,<<"muclight.domain.example.com">>},
    {secondary_muc_light_host,<<"muclight.domain.example.org">>},
    {protocol,http},
    {domain_admin,{<<"admin@domain.example.com">>,<<"a3ff986d55adb21d">>}},
    {schema_endpoint,domain_admin},
    {{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/4209/208236/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%40606610c9432f.2024-02-20_20.11.10/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-02-20_20.15.24/graphql_muc_light_suite.domain_admin_create_room_no_permission.html"},
    {tc_group_properties,[{name,domain_admin_muc_light}]},
    {tc_group_path,[[{name,domain_admin}]]},
    {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/4209/208236/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%40606610c9432f.2024-02-20_20.11.10/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-02-20_20.15.24/log_private/"},
    {{saved_modules,mongooseim@localhost,<<"test type"...

Report log

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

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

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

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

mssql_mnesia_26 / odbc_mssql_mnesia / 6ace2c0
Reports root/ big
OK: 4673 / Failed: 1 / User-skipped: 121 / Auto-skipped: 0

graphql_muc_light_SUITE:domain_admin:domain_admin_muc_light:admin_create_room_with_unprepped_id

{error,
  {{badmatch,null},
   [{graphql_muc_light_SUITE,admin_create_room_with_unprepped_id,1,
      [{file,
         "/home/circleci/project/big_tests/tests/graphql_muc_light_SUITE.erl"},
       {line,1155}]},
    {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

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

mssql_mnesia_26 / odbc_mssql_mnesia / 6ace2c0
Reports root/ big
OK: 4673 / Failed: 1 / User-skipped: 121 / Auto-skipped: 0

graphql_muc_light_SUITE:admin_cli:admin_muc_light:end_per_group

{error,
 {{unregistering_failed,
   {amount,2},
   {unregistered_items,
  [{{<<"_admin_invite_user_1074">>,
     [{escalus_event_mgr,<0.24515.0>},
    {tc_name,admin_invite_user},
    {escalus_cleaner,<0.24514.0>},
    {watchdog,<0.24513.0>},
    {muc_light_host,<<"muclight.localhost">>},
    {secondary_muc_light_host,<<"muclight.localhost.bis">>},
    {protocol,cli},
    {schema_endpoint,admin},
    {{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/4209/208251/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%40b9f6146bb6ec.2024-02-20_20.36.48/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-02-20_20.41.03/graphql_muc_light_suite.admin_invite_user.55682.html"},
    {tc_group_properties,[{name,admin_muc_light}]},
    {tc_group_path,[[{name,admin_cli}]]},
    {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/4209/208251/odbc_mssql_mnesia.26.1.2-amd64/big/ct_run.test%40b9f6146bb6ec.2024-02-20_20.36.48/big_tests.tests.graphql_muc_light_SUITE.logs/run.2024-02-20_20.41.03/log_private/"},
    {{saved_modules,mongooseim@localhost,<<"localhost">>},
     #{mod_vcard =>
      #{matches => 30,
        host => {prefix,<<"vjud.">>},
        search => true,iqdisc => parallel,backend => rdbms},
       mod_roster ...

Report log

graphql_muc_light_SUITE:domain_admin:domain_admin_muc_light:admin_create_room_with_unprepped_id

{error,
  {{badmatch,null},
   [{graphql_muc_light_SUITE,admin_create_room_with_unprepped_id,1,
      [{file,
         "/home/circleci/project/big_tests/tests/graphql_muc_light_SUITE.erl"},
       {line,1155}]},
    {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

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

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the refactor/presences branch from 79547ea to 0a3f2e8 Compare January 19, 2024 09:52

Base automatically changed from refactor/presences to master January 19, 2024 10:22

NelsonVides added 2 commits January 19, 2024 11:23

Upgrade mongoose_jid

01ec771

Upgrade exml

ff9a979

NelsonVides force-pushed the refactor/roster branch from 0ce71ac to b30cf42 Compare January 19, 2024 10:44

This comment was marked as outdated.

Sign in to view

NelsonVides added 3 commits January 21, 2024 16:22

Remove duplicated hook handler in mod_caps

cefec1c

Move roster modules to subdir

74ff877

Remove xml module usage from roster

dc19722

NelsonVides force-pushed the refactor/roster branch from b30cf42 to 021e84f Compare January 21, 2024 17:28

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the refactor/roster branch from 021e84f to 41fe974 Compare January 21, 2024 18:04

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the refactor/roster branch from 41fe974 to c4c3acb Compare January 21, 2024 19:01

This comment was marked as outdated.

Sign in to view

NelsonVides added 5 commits January 21, 2024 20:12

Simplify constructing roster item element

53fcf3c

Prefer tagged jids

47eccac

Provide more type information to roster record

d55571a

Skip extracting username from roster as it is already available

c70463b

Trust the DB contained correct jids already, we inserted them

486281a

NelsonVides force-pushed the refactor/roster branch from c4c3acb to 045af80 Compare January 21, 2024 19:12

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the refactor/roster branch from 045af80 to d333835 Compare January 21, 2024 20:30

This comment was marked as outdated.

Sign in to view

NelsonVides added 4 commits January 22, 2024 09:46

Optimise roster rdbms operations

4c17167

Rework roster_get hook

f625b9e

Use the roster list as the accumulator of the hook, for more intentionality.

Clean whitespace in mod_roster_rdbms

deb22cf

Unify decoding roster and groups

4e6e801

Ensure read transactionality for roster users and groups

85ed4f0

NelsonVides force-pushed the refactor/roster branch from d333835 to 85ed4f0 Compare January 22, 2024 08:46

This comment was marked as outdated.

Sign in to view

NelsonVides marked this pull request as ready for review January 23, 2024 08:56

pawlooss1 reviewed Feb 19, 2024

View reviewed changes

src/roster/mod_roster_rdbms.erl Outdated Show resolved Hide resolved

src/roster/mod_roster.erl Outdated Show resolved Hide resolved

src/roster/mod_roster.erl Show resolved Hide resolved

src/roster/mod_roster.erl Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

Apply review

6ace2c0

NelsonVides force-pushed the refactor/roster branch from 025ed1d to 6ace2c0 Compare February 20, 2024 20:06

NelsonVides requested a review from pawlooss1 February 20, 2024 21:34

pawlooss1 approved these changes Feb 21, 2024

View reviewed changes

pawlooss1 merged commit 880776f into master Feb 21, 2024
4 checks passed

pawlooss1 deleted the refactor/roster branch February 21, 2024 09:25

jacekwegr added this to the 6.2.1 milestone Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/roster #4209

Refactor/roster #4209

NelsonVides commented Jan 12, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Jan 19, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

pawlooss1 left a comment

This comment was marked as outdated.

mongoose-im commented Feb 20, 2024 •

edited

Loading

Refactor/roster #4209

Refactor/roster #4209

Conversation

NelsonVides commented Jan 12, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Jan 19, 2024 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

pawlooss1 left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

mongoose-im commented Feb 20, 2024 • edited Loading

NelsonVides commented Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 19, 2024 •

edited

Loading

mongoose-im commented Feb 20, 2024 •

edited

Loading