c2s/presence #3746
c2s/presence #3746
Conversation
Codecov ReportBase: 13.58% // Head: 57.73% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## feature/mongoose_c2s #3746 +/- ##
=========================================================
+ Coverage 13.58% 57.73% +44.15%
=========================================================
Files 535 536 +1
Lines 34535 34809 +274
=========================================================
+ Hits 4690 20096 +15406
+ Misses 29845 14713 -15132
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. |
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
21f1c60
to
e80ecce
Compare
NelsonVides
force-pushed
the
c2s/presences
branch
2 times, most recently
from
a703a6c
to
bdc1f33
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s/presences
branch
from
bdc1f33
to
7b68709
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s/presences
branch
from
7b68709
to
2a5694a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s/presences
branch
2 times, most recently
from
caaf56d
to
c6105e8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s/presences
branch
from
c6105e8
to
7b8e99c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
0c05c38
to
ca19934
Compare
NelsonVides
force-pushed
the
c2s/presences
branch
from
7b8e99c
to
acaf7f4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
a21bb34
to
225849a
Compare
NelsonVides
force-pushed
the
c2s/presences
branch
from
acaf7f4
to
57913d2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s/presences
branch
from
57913d2
to
1edaa0f
Compare
|
small_tests_24 / small_tests / 1edaa0f ldap_mnesia_24 / ldap_mnesia / 1edaa0f ldap_mnesia_25 / ldap_mnesia / 1edaa0f small_tests_25 / small_tests / 1edaa0f dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1edaa0f pgsql_mnesia_24 / pgsql_mnesia / 1edaa0f internal_mnesia_25 / internal_mnesia / 1edaa0f dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 1edaa0f elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 1edaa0f dynamic_domains_mysql_redis_25 / mysql_redis / 1edaa0f dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 1edaa0f pgsql_mnesia_25 / pgsql_mnesia / 1edaa0f riak_mnesia_24 / riak_mnesia / 1edaa0f mssql_mnesia_25 / odbc_mssql_mnesia / 1edaa0f |
| %% We have _subscription to_ these users' presence status; | ||
| %% i.e. they send us presence updates. | ||
| %% This comes from the roster. | ||
| pres_t = gb_sets:new() :: jid_set(), |
There was a problem hiding this comment.
we don't need pres_ prefix here.
So, maybe using subscribed_by and subscribed_to are better name.
available_to, invisible_to.
is_invisbile field.
last_stanza, last_timestamp.
Reasoning for longer names: we use longer names in the variables (i.e. ImAvailableTo), trying to describe what is going on. Instead of just putting this meaning into the field name.
There was a problem hiding this comment.
I also had a discussion with @NelsonVides and he suggested minimizing the refactoring in this PR, so maybe it's acceptable as it is now?
src/mod_presence.erl
Outdated
| #xmlel{attrs = Attrs} = Packet, | ||
| Attrs1 = lists:keystore(<<"type">>, 1, Attrs, {<<"type">>, <<"unavailable">>}), | ||
| NewElement = Packet#xmlel{attrs = Attrs1}, | ||
| Acc1 = mongoose_acc:update_stanza( |
There was a problem hiding this comment.
No need for Acc1 variable here and up. You can use typespecs though.
| -spec presence_update( | ||
| mongoose_acc:t(), jid:jid(), jid:jid(), exml:element(), mongoose_c2s:c2s_data(), undefined | binary()) -> | ||
| mongoose_acc:t(). | ||
| presence_update(Acc, FromJid, ToJid, Packet, StateData, undefined) -> |
There was a problem hiding this comment.
maybe we should use case here :D
There was a problem hiding this comment.
less duplication of code for me. There is one argument to match and other arguments are repeated.
Though it is just my preference.
src/mod_presence.erl
Outdated
| _ -> 0 | ||
| end. | ||
|
|
||
| -spec get_old_priority(presences()) -> priority(). |
There was a problem hiding this comment.
I don't like the name presences here, because it is anything but not [#xmlel{name = <<"presence">>}] (i.e. presences to me).
Maybe pstate could be better.
There was a problem hiding this comment.
It is correct that the name presences might suggest a list, but the type is clearly defined at the top, so I have no doubt what it means when reading the code. Anyway, presence_state might be better, but IMO it's ok as it is now.
NelsonVides
force-pushed
the
c2s/presences
branch
from
1edaa0f
to
4152aa9
Compare
|
ldap_mnesia_24 / ldap_mnesia / 4152aa9 small_tests_24 / small_tests / 4152aa9 ldap_mnesia_25 / ldap_mnesia / 4152aa9 small_tests_25 / small_tests / 4152aa9 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4152aa9 pgsql_mnesia_24 / pgsql_mnesia / 4152aa9 internal_mnesia_25 / internal_mnesia / 4152aa9 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4152aa9 dynamic_domains_mysql_redis_25 / mysql_redis / 4152aa9 pgsql_mnesia_25 / pgsql_mnesia / 4152aa9 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 4152aa9 riak_mnesia_24 / riak_mnesia / 4152aa9 mssql_mnesia_25 / odbc_mssql_mnesia / 4152aa9 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 4152aa9 smart_markers_SUITE:regular:one2one:marker_for_thread_can_be_fetched{error,
{{fetch_marker,ok,
[{times,50,
{error,
{badmatch,[]},
[{smart_markers_SUITE,'-verify_marker_fetch/4-fun-6-',3,
[{file,
"/home/circleci/project/big_tests/tests/smart_markers_SUITE.erl"},
{line,405}]},
{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,374}]},
{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,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}]},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,371}]},
{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,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} |
This module implements xmpp presences. With presences, now default escalus can successfully connect, and that unblocks a myriad of test suites that can now pass.