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

Mod last remove dynamic backend module #3339

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 0 additions & 9 deletions src/gen_mod.erl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
start_module/3,
start_backend_module/2,
start_backend_module/3,
get_backend_module/3,
stop_module/2,
stop_module_keep_config/2,
reload_module/3,
Expand Down Expand Up @@ -225,14 +224,6 @@ start_backend_module(Module, Opts, TrackedFuncs) ->
Backend = gen_mod:get_opt(backend, Opts, mnesia),
backend_module:create(Module, Backend, TrackedFuncs).

-spec get_backend_module(host_type(), module(), atom()) -> module().
get_backend_module(HostType, Module, DefaultBackend) ->
Backend = get_module_opt(HostType, Module, backend, DefaultBackend),
backend_module(Module, Backend).

backend_module(Module, Backend) ->
list_to_atom(atom_to_list(Module) ++ "_" ++ atom_to_list(Backend)).

-spec is_app_running(_) -> boolean().
is_app_running(AppName) ->
%% Use a high timeout to prevent a false positive in a high load system
Expand Down
2 changes: 1 addition & 1 deletion src/mod_auth_token.erl
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
start(HostType, Opts) ->
IQDisc = gen_mod:get_opt(iqdisc, Opts, no_queue),
mod_auth_token_backend:start(HostType),
mod_auth_token_backend:start(HostType, Opts),
ejabberd_hooks:add(hooks(HostType)),
gen_iq_handler:add_iq_handler_for_domain(
HostType, ?NS_ESL_TOKEN_AUTH, ejabberd_sm,
Expand Down
8 changes: 4 additions & 4 deletions src/mod_auth_token_backend.erl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
%%% @end
%%%----------------------------------------------------------------------
-module(mod_auth_token_backend).
-export([start/1,
-export([start/2,
revoke/2,
get_valid_sequence_number/2,
clean_tokens/2]).
Expand All @@ -26,9 +26,9 @@
%% ----------------------------------------------------------------------
%% API Functions

-spec start(HostType :: mongooseim:host_type()) -> ok.
start(HostType) ->
mongoose_backend:init_per_host_type(HostType, ?MAIN_MODULE, [], rdbms),
-spec start(HostType :: mongooseim:host_type(), Opts :: gen_mod:module_opts()) -> ok.
start(HostType, Opts) ->
mongoose_backend:init_per_host_type(HostType, ?MAIN_MODULE, [], [{backend, rdbms} | Opts]),
Args = [HostType],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

Expand Down
29 changes: 2 additions & 27 deletions src/mod_last.erl
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,7 @@

-export([config_metrics/1]).

-define(MOD_LAST_BACKEND, mod_last_backend).
-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 +66,16 @@
%% ------------------------------------------------------------------
%% 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
74 changes: 74 additions & 0 deletions src/mod_last_backend.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
%% 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) ->
TrackedFuns = [get_last, set_last_info],
mongoose_backend:init_per_host_type(HostType, ?MAIN_MODULE, TrackedFuns, Opts),
Args = [HostType, Opts],
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_tracked(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-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_tracked(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

-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
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
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
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
2 changes: 1 addition & 1 deletion src/mod_private_backend.erl
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
Opts :: list().
init(HostType, Opts) ->
TrackedFuns = [multi_get_data, multi_set_data],
mongoose_backend:init_per_host_type(HostType, ?MAIN_MODULE, TrackedFuns),
mongoose_backend:init_per_host_type(HostType, ?MAIN_MODULE, TrackedFuns, Opts),
Args = [HostType, Opts],
mongoose_backend:call(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).

Expand Down
21 changes: 9 additions & 12 deletions src/mongoose_backend.erl
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
-module(mongoose_backend).

%% API
-export([init_per_host_type/3,
init_per_host_type/4,
-export([init_per_host_type/4,
call/4,
call_tracked/4]).

Expand All @@ -17,22 +16,20 @@
-type main_module() :: module().
-type backend_module() :: module().

-spec init_per_host_type(HostType :: mongooseim:host_type(),
MainModule :: main_module(),
TrackedFuns :: [function_name()]) -> ok.
init_per_host_type(HostType, MainModule, TrackedFuns) ->
init_per_host_type(HostType, MainModule, TrackedFuns, mnesia).

-spec init_per_host_type(HostType :: mongooseim:host_type(),
MainModule :: main_module(),
TrackedFuns :: [function_name()],
DefaultBackend :: atom()) -> ok.
init_per_host_type(HostType, MainModule, TrackedFuns, DefaultBackend) ->
Opts :: gen_mod:module_opts()) -> ok.
init_per_host_type(HostType, MainModule, TrackedFuns, Opts) ->
ensure_backend_metrics(MainModule, TrackedFuns),
Backend = gen_mod:get_backend_module(HostType, MainModule, DefaultBackend),
persist_backend_name(HostType, MainModule, Backend),
Backend = gen_mod:get_opt(backend, Opts, mnesia),
BackendModule = backend_module(MainModule, Backend),
persist_backend_name(HostType, MainModule, BackendModule),
ok.

backend_module(Module, Backend) ->
list_to_atom(atom_to_list(Module) ++ "_" ++ atom_to_list(Backend)).

call_metric(MainModule, FunName) ->
[backends, MainModule, calls, FunName].

Expand Down
2 changes: 1 addition & 1 deletion test/auth_tokens_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ mock_mongoose_metrics() ->

mock_rdbms_backend() ->
meck:new(mod_auth_token_backend, []),
meck:expect(mod_auth_token_backend, start, fun(_) -> ok end),
meck:expect(mod_auth_token_backend, start, fun(_, _) -> ok end),
meck:expect(mod_auth_token_backend, get_valid_sequence_number,
fun (_, _) -> valid_seq_no_threshold() end),
ok.
Expand Down