-
Notifications
You must be signed in to change notification settings - Fork 428
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
S2S/out rework #4479
S2S/out rework #4479
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/listeners #4479 +/- ##
=====================================================
+ Coverage 85.48% 85.64% +0.15%
=====================================================
Files 558 558
Lines 33971 33754 -217
=====================================================
- Hits 29041 28907 -134
+ Misses 4930 4847 -83 ☔ View full report in Codecov by Sentry. |
628f021
to
0b500da
Compare
This comment was marked as outdated.
This comment was marked as outdated.
0b500da
to
fbb07e1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
fbb07e1
to
568ce24
Compare
This comment was marked as outdated.
This comment was marked as outdated.
568ce24
to
bfe2a3f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
34a03ab
to
8972597
Compare
This comment was marked as outdated.
This comment was marked as outdated.
3d394ec
to
26fe76e
Compare
26fe76e
to
23644bd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks very good but some minor comments must be addressed.
elasticsearch_and_cassandra_27 / elasticsearch_and_cassandra_mnesia / e70c80a small_tests_26 / small_tests / e70c80a small_tests_27 / small_tests / e70c80a small_tests_27_arm64 / small_tests / e70c80a dynamic_domains_mysql_redis_27 / mysql_redis / e70c80a ldap_mnesia_27 / ldap_mnesia / e70c80a ldap_mnesia_26 / ldap_mnesia / e70c80a internal_mnesia_27 / internal_mnesia / e70c80a dynamic_domains_pgsql_mnesia_27 / pgsql_mnesia / e70c80a dynamic_domains_mssql_mnesia_27 / odbc_mssql_mnesia / e70c80a dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / e70c80a pgsql_cets_27 / pgsql_cets / e70c80a pgsql_mnesia_26 / pgsql_mnesia / e70c80a pgsql_mnesia_27 / pgsql_mnesia / e70c80a cockroachdb_cets_27 / cockroachdb_cets / e70c80a mysql_redis_27 / mysql_redis / e70c80a accounts_SUITE:register:already_registered{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"alice_already_registered_25@localhost/res1">>,
escalus_tcp,<0.1376.0>,
[{event_manager,<0.1342.0>},
{server,<<"localhost">>},
{username,<<"alicE_already_registered_25">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.1342.0>},
{server,<<"localhost">>},
{username,<<"alicE_already_registered_25">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"alice_already_registered_25">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,fun escalus_auth:auth_plain/2},
{wspath,undefined},
{username,<<"alicE_already_registered_25">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"9282954657ca95a3">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{accounts_SUITE,already_registered_story,1,
[{file,
"/home/circleci/project/big_tests/tests/accounts_SUITE.erl"},
{line,198}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,75}]},
{test_server,ts_tc,3,[{file... mssql_mnesia_27 / odbc_mssql_mnesia / e70c80a mysql_redis_27 / mysql_redis / e70c80a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
The first important changes to notice are those in
mongoose_xmpp_socket
; it adds:The next big thing is the
mongoose_addr_list
module. It usesinet_res:lookup/5
alone to implement DNS discovery as required in the documents below, and also encapsulates preparing all data necessary to know where to connect to. It is currently used by s2s alone, but so much of the logic could be reused by a client by minimal extensions to this module. I'm not implementing the client side of it in this rework though.Next, the actual big thing: replace s2s_out modules with a new
gen_statem
basedmongoose_s2s_out
module.At last, many related minor changes are scattered around code, like:
mongoose_s2s_dialback
(exml attrs matching),mongoose_s2s_lib
(avoid binary<->string back-nd-forth conversions if not needed),ejabberd_s2s
(more explicit binary shapes than just binary).