-
Notifications
You must be signed in to change notification settings - Fork 426
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
Fixing errors in mod_roster #3873
Conversation
small_tests_24 / small_tests / d8b17a9 small_tests_25 / small_tests / d8b17a9 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / d8b17a9 ldap_mnesia_24 / ldap_mnesia / d8b17a9 ldap_mnesia_25 / ldap_mnesia / d8b17a9 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / d8b17a9 dynamic_domains_mysql_redis_25 / mysql_redis / d8b17a9 internal_mnesia_25 / internal_mnesia / d8b17a9 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / d8b17a9 pgsql_mnesia_24 / pgsql_mnesia / d8b17a9 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / d8b17a9 riak_mnesia_24 / riak_mnesia / d8b17a9 bosh_SUITE:essential_https:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"072488a25951c4bae0f47238f8a4ee0608574556">>,
<8846.5281.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{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}]}]}} pgsql_mnesia_25 / pgsql_mnesia / d8b17a9 mysql_redis_25 / mysql_redis / d8b17a9 mssql_mnesia_25 / odbc_mssql_mnesia / d8b17a9 inbox_extensions_SUITE:regular:one_to_one:pagination:max_queries_can_fetch_ahead{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bob_max_queries_can_fetch_ahead_1533@localhost/res1">>,
escalus_tcp,<0.8556.1>,
[{event_manager,<0.8545.1>},
{server,<<"localhost">>},
{username,<<"bOb_max_queries_can_fetch_ahead_1533">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.8545.1>},
{server,<<"localhost">>},
{username,<<"bOb_max_queries_can_fetch_ahead_1533">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"bob_max_queries_can_fetch_ahead_1533">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"bOb_max_queries_can_fetch_ahead_1533">>},
{server,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"1cff74e26e98b31d">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
{line,543}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1350}]},
{inbox_extensions_SUITE,'-max_queries_can_fetch_ahead/1-fun-0-',4,
[{file,... inbox_extensions_SUITE:regular:one_to_one:pagination:can_paginate_forwards{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"kate_can_paginate_forwards_1532@localhost/res1">>,
escalus_tcp,<0.8559.1>,
[{event_manager,<0.8538.1>},
{server,<<"localhost">>},
{username,<<"kate_can_paginate_forwards_1532">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.8538.1>},
{server,<<"localhost">>},
{username,<<"kate_can_paginate_forwards_1532">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"kate_can_paginate_forwards_1532">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"kate_can_paginate_forwards_1532">>},
{server,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"0989acb6e56c4c3e">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
{line,543}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{inbox_extensions_SUITE,'-can_paginate_forwards/1-fun-0-',4,
[{file,
"/home/circleci/project/big_t... inbox_extensions_SUITE:regular:one_to_one:pagination:pagination_overrides_form{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"kate_pagination_overrides_form_1530@localhost/res1">>,
escalus_tcp,<0.8557.1>,
[{event_manager,<0.8524.1>},
{server,<<"localhost">>},
{username,<<"kate_pagination_overrides_form_1530">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.8524.1>},
{server,<<"localhost">>},
{username,<<"kate_pagination_overrides_form_1530">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"kate_pagination_overrides_form_1530">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"kate_pagination_overrides_form_1530">>},
{server,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"663b6d857ab654bd">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
{line,543}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{inbox_extensions_SUITE,'-pagination_overrides_form/1-fun-0-',4,
[{file,
... inbox_extensions_SUITE:regular:one_to_one:pagination:can_paginate_backwards{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"kate_can_paginate_backwards_1529@localhost/res1">>,
escalus_tcp,<0.8558.1>,
[{event_manager,<0.8517.1>},
{server,<<"localhost">>},
{username,<<"kate_can_paginate_backwards_1529">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.8517.1>},
{server,<<"localhost">>},
{username,<<"kate_can_paginate_backwards_1529">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"kate_can_paginate_backwards_1529">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"kate_can_paginate_backwards_1529">>},
{server,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"2e793bd7c70ea041">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
{line,543}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{inbox_extensions_SUITE,'-can_paginate_backwards/1-fun-0-',4,
[{file,
"/home/circleci/project... inbox_extensions_SUITE:regular:one_to_one:pagination:max_queries_can_be_limited{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"kate_max_queries_can_be_limited_1531@localhost/res1">>,
escalus_tcp,<0.8560.1>,
[{event_manager,<0.8531.1>},
{server,<<"localhost">>},
{username,<<"kate_max_queries_can_be_limited_1531">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.8531.1>},
{server,<<"localhost">>},
{username,<<"kate_max_queries_can_be_limited_1531">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"kate_max_queries_can_be_limited_1531">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"kate_max_queries_can_be_limited_1531">>},
{server,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"d5c9d854906af407">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
{line,543}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{inbox_extensions_SUITE,'-max_queries_can_be_limited/1-fun-0-',4,
[{file... riak_mnesia_24 / riak_mnesia / d8b17a9 mssql_mnesia_25 / odbc_mssql_mnesia / d8b17a9 |
Codecov ReportBase: 83.09% // Head: 83.13% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3873 +/- ##
==========================================
+ Coverage 83.09% 83.13% +0.03%
==========================================
Files 535 535
Lines 34168 34171 +3
==========================================
+ Hits 28393 28409 +16
+ Misses 5775 5762 -13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
The changes look OK to me, but after checkout I found the following:
mongoose $ _build/mim1/rel/mongooseim/bin/mongooseimctl roster listContacts --user baduser@localhost
{
"data" : {
"roster" : {
"listContacts" : [
]
}
}
It should report that the user does not exist. Could you fix it and add a test for it?
Another minor issue - we should not print the <<
and >>
around binaries:
mongoose $ _build/mim1/rel/mongooseim/bin/mongooseimctl roster setMutualSubscription --userA alice@localhost/1 --userB bob@localhost/1 --action CONNECT
{
"data" : {
"roster" : {
"setMutualSubscription" : "Subscription between users <<\"alice@localhost/1\">> and <<\"bob@localhost/1\">> created successfully"
}
}
}
Another issue - we shouldn't crash in this case. Please find the best behaviour for an empty list.
mongoose $ _build/mim1/rel/mongooseim/bin/mongooseimctl roster subscribeAllToAll --contacts []
{
"errors" : [
{
"path" : [
"roster",
"subscribeAllToAll"
],
"message" : "The schema specifies the field is non-null, but a null value was returned by the backend",
"extensions" : {
"code" : "null_value"
}
},
{
"path" : [
"roster",
"subscribeAllToAll"
],
"message" : "Unexpected RosterAdminMutation resolver crash",
"extensions" : {
"code" : "resolver_crash"
}
}
],
"data" : {
"roster" : null
}
}
Minor: could you fix the typo mututal
?
mongoose $ _build/mim1/rel/mongooseim/bin/mongooseimctl roster setMutualSubscription --userA alice@localhost/1 --userB bob@localhost/1 --action DISCONNECT
{
"data" : {
"roster" : {
"setMutualSubscription" : "Mututal subscription removed successfully"
}
}
}
small_tests_24 / small_tests / 6555ab7 small_tests_25 / small_tests / 6555ab7 ldap_mnesia_24 / ldap_mnesia / 6555ab7 bosh_SUITE:essential_https:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"4de48d4b1ff1c73959ff4603cce4a9a3cb072433">>,
<8846.5333.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{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}]}]}} dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6555ab7 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 6555ab7 dynamic_domains_mysql_redis_25 / mysql_redis / 6555ab7 ldap_mnesia_25 / ldap_mnesia / 6555ab7 pgsql_mnesia_24 / pgsql_mnesia / 6555ab7 internal_mnesia_25 / internal_mnesia / 6555ab7 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 6555ab7 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 6555ab7 riak_mnesia_24 / riak_mnesia / 6555ab7 pgsql_mnesia_25 / pgsql_mnesia / 6555ab7 mysql_redis_25 / mysql_redis / 6555ab7 mssql_mnesia_25 / odbc_mssql_mnesia / 6555ab7 ldap_mnesia_24 / ldap_mnesia / 6555ab7 |
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 good
Fixing one error in schema, it was possible to call subscription and not pass subscription action.
I've also changed the mod_roster_api:subscription to ensure the stanza is being sent to bare jid.