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.

Sign up for GitHub

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

Mod last remove dynamic backend module #3339

Merged
merged 5 commits into from
Oct 19, 2021
Merged

Mod last remove dynamic backend module #3339

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c36279
Removing dynamically compiled modules from mod_last
Oct 14, 2021
c579418
Adding mistakenly deleted ignore_xref to mod_last
Oct 14, 2021
926579c
Resolving remarks from PR
Oct 14, 2021
3a3261f
Changing init_per_host_type in mongoose_backend module
Oct 15, 2021
c46fa10
Removing xref to mod_last from rebar.config
Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 2 additions & 27 deletions src/mod_last.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@

-define(MOD_LAST_BACKEND, mod_last_backend).
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be deleted now.

-ignore_xref([
{?MOD_LAST_BACKEND, init, 2},
{?MOD_LAST_BACKEND, get_last, 3},
{?MOD_LAST_BACKEND, count_active_users, 3},
{?MOD_LAST_BACKEND, set_last_info, 5},
{?MOD_LAST_BACKEND, remove_user, 3},
{?MOD_LAST_BACKEND, remove_domain, 2},
behaviour_info/1, on_presence_update/5, process_local_iq/4,
process_sm_iq/4, remove_user/3, session_cleanup/5, remove_domain/3
]).
Expand All @@ -73,34 +67,15 @@
%% ------------------------------------------------------------------
%% Backend callbacks

-export_type([host_type/0, timestamp/0, status/0]).

-type host_type() :: mongooseim:host_type().
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this makes much sense in the new code - it is exported but mongooseim:host_type is exported as well. IMO we should either use host_type() instead of mongooseim:host_type() in this whole module or delete this line.

-type timestamp() :: non_neg_integer().
-type status() :: binary().

-export_type([host_type/0, timestamp/0, status/0]).

-callback init(host_type(), gen_mod:module_opts()) -> ok.

-callback get_last(host_type(), jid:luser(), jid:lserver()) ->
{ok, timestamp(), status()} | {error, term()} | not_found.

-callback count_active_users(host_type(), jid:lserver(), timestamp()) ->
non_neg_integer().

-callback set_last_info(host_type(), jid:luser(), jid:lserver(), timestamp(), status()) ->
ok | {error, term()}.

-callback remove_user(host_type(), jid:luser(), jid:lserver()) ->
ok | {error, term()}.

-callback remove_domain(host_type(), jid:lserver()) ->
ok | {error, term()}.

-spec start(mongooseim:host_type(), list()) -> 'ok'.
JanuszJakubiec marked this conversation as resolved.
Show resolved Hide resolved
start(HostType, Opts) ->
IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue),

gen_mod:start_backend_module(?MODULE, Opts, [get_last, set_last_info]),
mod_last_backend:init(HostType, Opts),

[gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_LAST, Component, Fn, #{}, IQDisc) ||
Expand Down
72 changes: 72 additions & 0 deletions src/mod_last_backend.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
%% Just a proxy interface module between the main mod_last module and
%% the backend modules (i.e. mod_last_rdbms, mod_last_mnesia...).
-module(mod_last_backend).

-export([init/2,
get_last/3,
count_active_users/3,
set_last_info/5,
remove_user/3,
remove_domain/2]).

-define(MAIN_MODULE, mod_last).

-callback init(mod_last:host_type(), gen_mod:module_opts()) -> ok.
Copy link
Member

Choose a reason for hiding this comment

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

Please use mongooseim:host_type() or declare -type host_type() :: mongooseim:host_type().

mod_last:host_type() suggests something different than mongooseim:host_type() and is a bit confusing.


-callback get_last(mod_last:host_type(), jid:luser(), jid:lserver()) ->
{ok, mod_last:timestamp(), mod_last:status()} | {error, term()} | not_found.

-callback count_active_users(mod_last:host_type(), jid:lserver(), mod_last:timestamp()) ->
non_neg_integer().

-callback set_last_info(
mod_last:host_type(),
jid:luser(),
jid:lserver(),
mod_last:timestamp(),
mod_last:status()) -> ok | {error, term()}.

-callback remove_user(mod_last:host_type(), jid:luser(), jid:lserver()) ->
ok | {error, term()}.

-callback remove_domain(mod_last:host_type(), jid:lserver()) ->
ok | {error, term()}.

-spec init(mod_last:host_type(), gen_mod:module_opts()) -> ok.
init(HostType, Opts) ->
Args = [HostType, Opts],
Copy link
Contributor

Choose a reason for hiding this comment

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

There has to be a call to mongoose_backend:init_per_host_type/4 here. Without it the calls will fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to pass tracked functions to it - these will have automatic metrics created by mongoose_backend.

Copy link
Contributor Author

@JanuszJakubiec JanuszJakubiec Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can I use mongoose_backend:init_per_host_type/3 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, if the default backend is mnesia - from what I see that's the case.

mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-spec get_last(mod_last:host_type(), jid:luser(), jid:lserver()) ->
{ok, mod_last:timestamp(), mod_last:status()} | {error, term()} | not_found.
get_last(HostType, LUser, LServer) ->
Args = [HostType, LUser, LServer],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be call_tracked, because it was passed as a tracked function to gen_mod:start_backend_module.


-spec count_active_users(mod_last:host_type(), jid:lserver(), mod_last:timestamp()) ->
non_neg_integer().
count_active_users(HostType, LServer, Timestamp) ->
Args = [HostType, LServer, Timestamp],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-spec set_last_info(
mod_last:host_type(),
jid:luser(),
jid:lserver(),
mod_last:timestamp(),
mod_last:status()) -> ok | {error, term()}.
set_last_info(HostType, LUser, LServer, Timestamp, Status) ->
Args = [HostType, LUser, LServer, Timestamp, Status],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to get_last.


-spec remove_user(mod_last:host_type(), jid:luser(), jid:lserver()) ->
ok | {error, term()}.
remove_user(HostType, LUser, LServer) ->
Args = [HostType, LUser, LServer],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-spec remove_domain(mod_last:host_type(), jid:lserver()) ->
ok | {error, term()}.
remove_domain(HostType, LServer) ->
Args = [HostType, LServer],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).
Copy link
Member

Choose a reason for hiding this comment

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

Please put a newline at the end of the file.

2 changes: 1 addition & 1 deletion src/mod_last_mnesia.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

-module(mod_last_mnesia).

-behaviour(mod_last).
-behaviour(mod_last_backend).

-include("mod_last.hrl").
-include("mongoose.hrl").
Expand Down
2 changes: 1 addition & 1 deletion src/mod_last_rdbms.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

-module(mod_last_rdbms).

-behaviour(mod_last).
-behaviour(mod_last_backend).

-include("mongoose.hrl").

Expand Down
2 changes: 1 addition & 1 deletion src/mod_last_riak.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
%%% @end
-module(mod_last_riak).

-behaviour(mod_last).
-behaviour(mod_last_backend).

-include("mongoose.hrl").

Expand Down