Take the accumulator in the room hooks #3417
Conversation
This allows to have the accumulator in hooks like is_muc_room_owner and can_access_room, so that if affiliations where already fetched, they will be present in the accumulator, and we then move forwards in preserving the consistency of a message processing pipeline: we want to avoid the case when, during this pipeline, a subsystem fetches the affiliations, take some action, and before the next subsystem acts, a concurrent process modifies the affiliations: then the following subsystem may find out that the user has been removed from the room and reject the request, therefore getting into an inconsistent state. This also helps preventing more duplicate, and usually idempotent, DB queries.
Codecov Report
@@ Coverage Diff @@
## master #3417 +/- ##
==========================================
+ Coverage 78.15% 80.87% +2.72%
==========================================
Files 414 414
Lines 32298 32298
==========================================
+ Hits 25241 26122 +881
+ Misses 7057 6176 -881
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
NelsonVides
force-pushed
the
campaigns
branch
from
e93a895 to
30e24ad
Compare
This comment has been minimized.
This comment has been minimized.
NelsonVides
force-pushed
the
campaigns
branch
from
5c0d299 to
968c20f
Compare
|
small_tests_24 / small_tests / 968c20f small_tests_23 / small_tests / 968c20f dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 968c20f dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 968c20f dynamic_domains_mysql_redis_24 / mysql_redis / 968c20f dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 968c20f ldap_mnesia_24 / ldap_mnesia / 968c20f amp_big_SUITE:basic:notify_deliver_to_online_user_recipient_privacy_test{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[{xmlcdata,<<"Replaced by new connection">>}]}]},
"<stream:error><conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>Replaced by new connection</text></stream:error>"},
[{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,'-drop_presences/2-lc$^0/1-0-',1,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,drop_presences,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,'-start_ready_clients/2-fun-0-',3,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,135}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_story,start_ready_clients,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
...ldap_mnesia_23 / ldap_mnesia / 968c20f internal_mnesia_24 / internal_mnesia / 968c20f pgsql_mnesia_23 / pgsql_mnesia / 968c20f pgsql_mnesia_24 / pgsql_mnesia / 968c20f elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 968c20f mysql_redis_24 / mysql_redis / 968c20f mssql_mnesia_24 / odbc_mssql_mnesia / 968c20f riak_mnesia_24 / riak_mnesia / 968c20f |
NelsonVides
force-pushed
the
campaigns
branch
from
968c20f to
7f59f2a
Compare
|
small_tests_24 / small_tests / 7f59f2a small_tests_23 / small_tests / 7f59f2a dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7f59f2a dynamic_domains_mysql_redis_24 / mysql_redis / 7f59f2a dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7f59f2a dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7f59f2a domain_removal_SUITE:last_removal:last_removal{error,{{assertion_failed,assert,is_last_result,
{xmlel,<<"presence">>,
[{<<"from">>,
<<"bOb_last_removal_44.897159@domain.example.com/friendly">>},
{<<"to">>,
<<"alice_last_removal_44.897159@domain.example.com/res1">>},
{<<"type">>,<<"unavailable">>}],
[]},
"<presence from='bOb_last_removal_44.897159@domain.example.com/friendly' to='alice_last_removal_44.897159@domain.example.com/res1' type='unavailable'/>"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{domain_removal_SUITE,'-last_removal/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/domain_removal_SUITE.erl"},
{line,376}]},
{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,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}]}]}}ldap_mnesia_24 / ldap_mnesia / 7f59f2a ldap_mnesia_23 / ldap_mnesia / 7f59f2a internal_mnesia_24 / internal_mnesia / 7f59f2a pgsql_mnesia_24 / pgsql_mnesia / 7f59f2a elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7f59f2a mysql_redis_24 / mysql_redis / 7f59f2a amp_big_SUITE:offline:offline_failure:notify_deliver_to_online_user_recipient_privacy_test{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[{xmlcdata,<<"Replaced by new connection">>}]}]},
"<stream:error><conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>Replaced by new connection</text></stream:error>"},
[{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,'-drop_presences/2-lc$^0/1-0-',1,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,drop_presences,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,'-start_ready_clients/2-fun-0-',3,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,135}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_story,start_ready_clients,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
...pgsql_mnesia_23 / pgsql_mnesia / 7f59f2a mssql_mnesia_24 / odbc_mssql_mnesia / 7f59f2a riak_mnesia_24 / riak_mnesia / 7f59f2a dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7f59f2a mysql_redis_24 / mysql_redis / 7f59f2a mam_SUITE:rdbms_simple_prefs_cases:messages_filtered_when_prefs_default_policy_is_roster{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok]\n"}} |
NelsonVides
force-pushed
the
campaigns
branch
from
44d51e7 to
1baa7e2
Compare
NelsonVides
force-pushed
the
campaigns
branch
from
1baa7e2 to
1e736cc
Compare
|
small_tests_24 / small_tests / 1e736cc small_tests_23 / small_tests / 1e736cc dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 1e736cc dynamic_domains_mysql_redis_24 / mysql_redis / 1e736cc mam_SUITE:rdbms_prefs_cases:prefs_set_cdata_request{error,{test_case_failed,"ASSERT EQUAL\n\tExpected {prefs_result_iq,<<\"roster\">>,\n [<<\"montague@montague.net\">>,\n <<\"romeo@montague.net\">>],\n []}\n\tValue {prefs_result_iq,<<\"always\">>,\n [<<\"montague@montague.net\">>,\n <<\"romeo@montague.net\">>],\n []}\n"}}dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1e736cc dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 1e736cc ldap_mnesia_23 / ldap_mnesia / 1e736cc ldap_mnesia_24 / ldap_mnesia / 1e736cc internal_mnesia_24 / internal_mnesia / 1e736cc pgsql_mnesia_23 / pgsql_mnesia / 1e736cc pgsql_mnesia_24 / pgsql_mnesia / 1e736cc elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 1e736cc mysql_redis_24 / mysql_redis / 1e736cc mssql_mnesia_24 / odbc_mssql_mnesia / 1e736cc riak_mnesia_24 / riak_mnesia / 1e736cc |
On broadcast to connected users, there's a chance to build a copy of a potentially long list of affiliations, to potentially many connected users, which would make broadcast for this c2s process too slow. Risk of an inconsistency for the recipients is too small to be worth the trouble, they might as well fetch their own affiliations.
|
small_tests_24 / small_tests / 6e620d0 small_tests_23 / small_tests / 6e620d0 dynamic_domains_mysql_redis_24 / mysql_redis / 6e620d0 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6e620d0 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 6e620d0 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 6e620d0 ldap_mnesia_23 / ldap_mnesia / 6e620d0 ldap_mnesia_24 / ldap_mnesia / 6e620d0 internal_mnesia_24 / internal_mnesia / 6e620d0 pgsql_mnesia_24 / pgsql_mnesia / 6e620d0 pgsql_mnesia_23 / pgsql_mnesia / 6e620d0 mysql_redis_24 / mysql_redis / 6e620d0 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 6e620d0 sm_SUITE:unacknowledged_message_hook:unacknowledged_message_hook_offline{error,{{badmatch,false},
[{escalus_session,stream_resumption,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,259}]},
{escalus_connection,connection_step,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,160}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_connection,start,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,144}]},
{sm_SUITE,unacknowledged_message_hook_offline,4,
[{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
{line,826}]},
{sm_SUITE,unacknowledged_message_hook_common,2,
[{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
{line,878}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]}]}}mssql_mnesia_24 / odbc_mssql_mnesia / 6e620d0 riak_mnesia_24 / riak_mnesia / 6e620d0 |
This allows to have the accumulator in hooks like is_muc_room_owner and
can_access_room, so that if affiliations where already fetched, they
will be present in the accumulator, and we then move forwards in
preserving the consistency of a message processing pipeline: we want to
avoid the case when, during this pipeline, a subsystem fetches the
affiliations, take some action, and before the next subsystem acts, a
concurrent process modifies the affiliations: then the following
subsystem may find out that the user has been removed from the room and
reject the request, therefore getting into an inconsistent state.
This also helps preventing more duplicate, and usually idempotent, DB
queries.
This also gives more fine-grained control over other new handlers to this hook that I can implement, where information stored in the accumulator can provide more powerful decisions 😉