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

Inbox/refactoring – add hooks and clean redundant code #3604

Merged
merged 2 commits into from
Mar 22, 2022

Conversation

NelsonVides
Copy link
Collaborator

Custom code might want to add metadata to each inbox entry, as they
fetch it, like for example markers, or information about how to display
the entry. In order to avoid multiple round-trips between client and
server, this metadata could be added in the inbox query already. For
that, we insert a hook that takes the accumulator, the inbox row as
returned from the DB, and the IQ that initiated the query to begin with.

That way, custom xml elements can be added to the inbox iqs in the same
way they can be added to the mam queries, and hooks can parse extra
flags and generate more xml elements to append to the inbox response.

Custom code might want to add metadata to each inbox entry, as they
fetch it, like for example markers, or information about how to display
the entry. In order to avoid multiple round-trips between client and
server, this metadata could be added in the inbox query already. For
that, we insert a hook that takes the accumulator, the inbox row as
returned from the DB, and the IQ that initiated the query to begin with.

That way, custom xml elements can be added to the inbox iqs in the same
way they can be added to the mam queries, and hooks can parse extra
flags and generate more xml elements to append to the inbox response.
@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #3604 (a0fd192) into master (a9c68bd) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3604      +/-   ##
==========================================
+ Coverage   80.89%   80.91%   +0.01%     
==========================================
  Files         426      426              
  Lines       32250    32254       +4     
==========================================
+ Hits        26090    26099       +9     
+ Misses       6160     6155       -5     
Impacted Files Coverage Δ
src/inbox/mod_inbox_backend.erl 100.00% <ø> (ø)
src/inbox/mod_inbox_rdbms.erl 93.12% <ø> (-0.63%) ⬇️
src/inbox/mod_inbox_rdbms_async.erl 66.10% <ø> (ø)
src/inbox/mod_inbox.erl 87.42% <100.00%> (-1.72%) ⬇️
src/inbox/mod_inbox_entries.erl 95.89% <100.00%> (+0.05%) ⬆️
src/mongoose_hooks.erl 95.38% <100.00%> (+0.07%) ⬆️
src/mod_muc_log.erl 63.21% <0.00%> (ø)
... and 2 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 a9c68bd...a0fd192. Read the comment docs.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Mar 22, 2022

small_tests_24 / small_tests / a0fd192
Reports root / small


small_tests_23 / small_tests / a0fd192
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / a0fd192
Reports root/ big
OK: 2824 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / a0fd192
Reports root/ big
OK: 2821 / Failed: 3 / User-skipped: 133 / Auto-skipped: 0

service_domain_db_SUITE:db:db_keeps_syncing_after_cluster_join
{error,{test_case_failed,{[<<"example1.com">>],
              [<<"example1.com">>,<<"example2.com">>]}}}

Report log

service_domain_db_SUITE:db:rest_with_auth:rest_delete_domain_cleans_data_from_mam
{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,
          <<"bob_rest_delete_domain_cleans_data_from_mam_2376@example.org/res1">>,
          escalus_tcp,<0.30782.1>,
          [{event_manager,<0.30776.1>},
           {server,<<"example.org">>},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2376">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.30776.1>},
            {server,<<"example.org">>},
            {username,
              <<"bob_rest_delete_domain_cleans_data_from_mam_2376">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2376">>},
           {server,<<"example.org">>},
           {host,<<"localhost">>},
           {port,5232},
           {auth,{escalus_auth,auth_plain}},
           {wspath,undefined},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2376">>},
           {server,<<"example.org">>},
           {host,<<"localhost">>},
           {password,<<"makota3">>},
           {port,5232},
           {stream_id,<<"6ffa05897dcfb91f">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {service_domain_db_SUITE,
       '-rest_delete_domain_cleans_data_from_mam/1-fun-0-',5,
       [{file,
    ...

Report log

service_domain_db_SUITE:db:rest_without_auth:rest_delete_domain_cleans_data_from_mam
{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,
          <<"bob_rest_delete_domain_cleans_data_from_mam_2377@example.org/res1">>,
          escalus_tcp,<0.31396.1>,
          [{event_manager,<0.31390.1>},
           {server,<<"example.org">>},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2377">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.31390.1>},
            {server,<<"example.org">>},
            {username,
              <<"bob_rest_delete_domain_cleans_data_from_mam_2377">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2377">>},
           {server,<<"example.org">>},
           {host,<<"localhost">>},
           {port,5232},
           {auth,{escalus_auth,auth_plain}},
           {wspath,undefined},
           {username,
             <<"bob_rest_delete_domain_cleans_data_from_mam_2377">>},
           {server,<<"example.org">>},
           {host,<<"localhost">>},
           {password,<<"makota3">>},
           {port,5232},
           {stream_id,<<"5f3c4c7a15444762">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {service_domain_db_SUITE,
       '-rest_delete_domain_cleans_data_from_mam/1-fun-0-',5,
       [{file,
    ...

Report log


dynamic_domains_mysql_redis_24 / mysql_redis / a0fd192
Reports root/ big
OK: 2807 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / a0fd192
Reports root/ big
OK: 1505 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / a0fd192
Reports root/ big
OK: 1505 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / a0fd192
Reports root/ big
OK: 1546 / Failed: 0 / User-skipped: 358 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / a0fd192
Reports root/ big
OK: 2824 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / a0fd192
Reports root/ big
OK: 1853 / Failed: 0 / User-skipped: 366 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / a0fd192
Reports root/ big
OK: 3198 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / a0fd192
Reports root/ big
OK: 3198 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / a0fd192
Reports root/ big
OK: 3193 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / a0fd192
Reports root/ big
OK: 3210 / Failed: 1 / User-skipped: 142 / Auto-skipped: 0

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,4383}]},
     {muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4124}]},
     {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,4120}]},
     {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


riak_mnesia_24 / riak_mnesia / a0fd192
Reports root/ big
OK: 1696 / Failed: 0 / User-skipped: 365 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / a0fd192
Reports root/ big
OK: 2824 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

Looks good, one minor comment.

List :: list(inbox_res()),
QueryId :: id(),
List :: [inbox_res()],
QueryId :: jlib:iq(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
QueryId :: jlib:iq(),
QueryEl :: jlib:iq(),

Skipping names is also fine for me.

@chrzaszcz chrzaszcz merged commit 17fbc73 into master Mar 22, 2022
@chrzaszcz chrzaszcz deleted the inbox/refactoring branch March 22, 2022 11:56
@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