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

GraphQL - Implement session api #3521

Merged
merged 9 commits into from
Feb 3, 2022
Merged

Conversation

Premwoik
Copy link
Contributor

@Premwoik Premwoik commented Jan 28, 2022

This PR addresses MIM-1581 and adds support for the session category.

All session command functions have been extracted to the mongoose_session_api module.

@codecov
Copy link

codecov bot commented Jan 28, 2022

Codecov Report

Merging #3521 (4412621) into feature/graphql (54590bf) will increase coverage by 0.10%.
The diff coverage is 98.01%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           feature/graphql    #3521      +/-   ##
===================================================
+ Coverage            81.23%   81.33%   +0.10%     
===================================================
  Files                  446      449       +3     
  Lines                32763    32854      +91     
===================================================
+ Hits                 26614    26721     +107     
+ Misses                6149     6133      -16     
Impacted Files Coverage Δ
.../admin/mongoose_graphql_account_admin_mutation.erl 96.15% <ø> (ø)
...hql/admin/mongoose_graphql_account_admin_query.erl 100.00% <ø> (ø)
src/graphql/mongoose_graphql.erl 91.11% <ø> (ø)
...ql/user/mongoose_graphql_account_user_mutation.erl 100.00% <ø> (ø)
src/graphql/mongoose_graphql_enum.erl 42.85% <50.00%> (+9.52%) ⬆️
src/mongoose_session_api.erl 94.44% <98.46%> (+27.77%) ⬆️
src/admin_extra/service_admin_extra_sessions.erl 100.00% <100.00%> (+6.00%) ⬆️
.../graphql/admin/mongoose_graphql_admin_mutation.erl 100.00% <100.00%> (ø)
src/graphql/admin/mongoose_graphql_admin_query.erl 100.00% <100.00%> (ø)
.../admin/mongoose_graphql_session_admin_mutation.erl 100.00% <100.00%> (ø)
... and 16 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 54590bf...4412621. Read the comment docs.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@Premwoik Premwoik force-pushed the graphql/session-api branch from a16e25e to 4a2d87b Compare January 31, 2022 20:17
@mongoose-im

This comment has been minimized.

@Premwoik Premwoik force-pushed the graphql/session-api branch from 4a2d87b to dc65904 Compare February 1, 2022 09:18
@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@Premwoik Premwoik force-pushed the graphql/session-api branch from 49e72cd to 97e7244 Compare February 2, 2022 07:04
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 2, 2022

internal_mnesia_24 / internal_mnesia / 97e7244
Reports root


small_tests_23 / small_tests / 97e7244
Reports root / small


small_tests_24 / small_tests / 97e7244
Reports root / small


dynamic_domains_mysql_redis_24 / mysql_redis / 97e7244
Reports root/ big
OK: 2745 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 97e7244
Reports root/ big
OK: 2762 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 97e7244
Reports root/ big
OK: 2762 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 97e7244
Reports root/ big
OK: 1539 / Failed: 0 / User-skipped: 422 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 97e7244
Reports root/ big
OK: 1539 / Failed: 0 / User-skipped: 422 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 97e7244
Reports root/ big
OK: 3144 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 97e7244
Reports root/ big
OK: 1891 / Failed: 0 / User-skipped: 377 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 97e7244
Reports root/ big
OK: 3149 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 97e7244
Reports root/ big
OK: 3149 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 97e7244
Reports root/ big
OK: 1737 / Failed: 0 / User-skipped: 377 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 97e7244
Reports root


internal_mnesia_24 / internal_mnesia / 97e7244
Reports root

@Premwoik Premwoik marked this pull request as ready for review February 2, 2022 07:30
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.

The code looks good to me. I know it was not in the doc, but I think there should be a command allowing the user to set their own presence - just like the admin command. Otherwise the API is kind of incomplete.

Comment on lines 56 to 57
init_per_group(_, Config) ->
Config.
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? There are no other groups.

Comment on lines 64 to 65
end_per_group(_, _Config) ->
ok.
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

?assertMatch([#{<<"user">> := ExpectedUser}], get_ok_value(Path, Result)).

admin_list_sessions(Config) ->
escalus:story(Config, [{alice, 1}, {alice_bis, 1}, {bob, 1}], fun(_Alice, AliceB, _Bob) ->
Copy link
Member

Choose a reason for hiding this comment

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

Could you do these separate story functions for all test cases?

AwayPresence = escalus_stanza:presence_show(AwayStatus),
DndStatus = <<"dnd">>,
DndPresence = escalus_stanza:presence_show(DndStatus),
% Count users with away status globally
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
% Count users with away status globally
% List users with away status globally

StatusUsers = get_ok_value(Path, Res),
?assertEqual(2, length(StatusUsers)),
?assert(users_match([AliceJID, AliceBJID], StatusUsers)),
% Count users with away status for a domain
Copy link
Member

Choose a reason for hiding this comment

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

As above

}

"Short info about user's status"
type StatusUser{
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
type StatusUser{
type UserStatus{


-spec null_to_empty(null | integer() | binary()) -> binary().
null_to_empty(null) -> <<>>;
null_to_empty(Int) when is_integer(Int)-> integer_to_binary(Int);
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
null_to_empty(Int) when is_integer(Int)-> integer_to_binary(Int);
null_to_empty(Int) when is_integer(Int) -> integer_to_binary(Int);

Comment on lines 38 to 50
-type status_user_info() :: { User :: jid:user(),
Server :: jid:server(),
Res :: jid:resource(),
Prio :: ejabberd_sm:priority(),
Status :: status()}.

-type session_info() :: {USR :: binary(),
Conn :: binary(),
IPS :: binary(),
Port :: inet:port_number(),
Prio :: ejabberd_sm:priority(),
NodeS :: binary(),
Uptime :: integer()}.
Copy link
Member

Choose a reason for hiding this comment

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

Please fix indentation

maybe_pres_priority(Priority,
maybe_pres_show(Show, []))),
Message = {xmlstreamelement,
#xmlel{ name = <<"presence">>,
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
#xmlel{ name = <<"presence">>,
#xmlel{name = <<"presence">>,

Comment on lines 213 to 215
|| #session{sid = {_, Pid}, usr = {_, S, _}, priority = P}
<- Sessions0 ],

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
|| #session{sid = {_, Pid}, usr = {_, S, _}, priority = P}
<- Sessions0 ],
|| #session{sid = {_, Pid}, usr = {_, S, _}, priority = P} <- Sessions0 ],

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.

I wanted to comment, but I clicked 'approve' by mistake, apparently the code is just good 🙂

@Premwoik Premwoik force-pushed the graphql/session-api branch from 97e7244 to d34f161 Compare February 2, 2022 14:59
@mongoose-im

This comment was marked as outdated.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 3, 2022

small_tests_24 / small_tests / 4412621
Reports root / small


small_tests_23 / small_tests / 4412621
Reports root / small


dynamic_domains_mysql_redis_24 / mysql_redis / 4412621
Reports root/ big
OK: 2745 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 4412621
Reports root/ big
OK: 2762 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4412621
Reports root/ big
OK: 2762 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 4412621
Reports root/ big
OK: 2762 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 4412621
Reports root/ big
OK: 1539 / Failed: 0 / User-skipped: 422 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 4412621
Reports root/ big
OK: 1539 / Failed: 0 / User-skipped: 422 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 4412621
Reports root/ big
OK: 1585 / Failed: 0 / User-skipped: 376 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 4412621
Reports root/ big
OK: 3149 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 4412621
Reports root/ big
OK: 3144 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 4412621
Reports root/ big
OK: 3149 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 4412621
Reports root/ big
OK: 1891 / Failed: 0 / User-skipped: 377 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 4412621
Reports root/ big
OK: 3149 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 4412621
Reports root/ big
OK: 1737 / Failed: 0 / User-skipped: 377 / 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!

@chrzaszcz chrzaszcz merged commit a957367 into feature/graphql Feb 3, 2022
@chrzaszcz chrzaszcz deleted the graphql/session-api branch February 3, 2022 16:24
@chrzaszcz chrzaszcz added this to the 6.0.0 milestone Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants