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

Improved subdomain handling with dedicated registry #1116

Merged
merged 3 commits into from
Dec 13, 2016

Conversation

fenek
Copy link
Member

@fenek fenek commented Dec 12, 2016

This PR addresses most of #911

Proposed changes include:

  • New module, mongoose_subhosts, maintains a table of subdomain->domain mappings.
  • Entries are added when a module registers a subdomain with new API (it is not coupled with route registration or module startup)
  • API is provided in gen_mod to access module options by subdomain
  • Main module table is now created with read_concurrency enabled
  • Clarified naming convention in some places (Host/ServerHost/Server vs. Host/SubHost/MUCHost/etc.)
  • mod_mam_muc and mod_muc_light use new registry instead of custom, private tables

@@ -932,5 +932,6 @@ disable_delivery_test(Config) ->
required_modules() ->
[{mod_pubsub, [
{plugins,[<<"dag">>]},
{nodetree,<<"dag">>}
{nodetree,<<"dag">>},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 935

@@ -137,7 +137,8 @@ required_modules() ->
{mod_pubsub, [
{plugins,[<<"dag">>,<<"pep">>]},
{nodetree,<<"dag">>},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 139

RoomsPerUser = gen_mod:get_module_opt_by_subhost(
MUCServer, mod_muc_light, rooms_per_user, ?DEFAULT_ROOMS_PER_USER),
BlockingQuery = case gen_mod:get_module_opt_by_subhost(
MUCServer, mod_muc_light, blocking, ?DEFAULT_BLOCKING) of
true ->
[{user, FromUS}
| if

Choose a reason for hiding this comment

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

According to Elvis:

Replace the 'if' expression on line 156 with a 'case' expression or function clauses.

@@ -137,7 +137,8 @@ required_modules() ->
{mod_pubsub, [
{plugins,[<<"dag">>,<<"pep">>]},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 138

@@ -776,6 +777,8 @@ room_jid_to_pid(#jid{luser=RoomName, lserver=MucService}) ->
{error, not_found}
end.

-spec default_host() -> list().
default_host() -> "conference.@HOST@".

-spec iq_disco_info(ejabberd:lang()) -> [jlib:xmlel(),...].

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 783

@@ -175,7 +180,7 @@ init([VHost, Opts]) ->
?MODULE,process_sm_iq, IQDisc),

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 180

@@ -137,7 +137,8 @@ required_modules() ->
{mod_pubsub, [
{plugins,[<<"dag">>,<<"pep">>]},
{nodetree,<<"dag">>},
{pep_mapping,[]}
{pep_mapping,[]},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 140

[#subhost_mapping{ host = Host }] -> {ok, Host};
[] -> undefined
end.

Choose a reason for hiding this comment

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

According to Elvis:

Line 65 has 4 trailing whitespace characters.

@@ -137,7 +137,8 @@ required_modules() ->
{mod_pubsub, [
{plugins,[<<"dag">>,<<"pep">>]},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 138

@@ -175,7 +180,7 @@ init([VHost, Opts]) ->
?MODULE,process_sm_iq, IQDisc),
gen_iq_handler:add_iq_handler(ejabberd_local, VHost, ?NS_VCARD,
?MODULE,process_local_iq, IQDisc),

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 182

@@ -461,8 +461,7 @@ process_pattern(Str, {User, Domain}, AttrValues) ->
[{<<"%s">>, V, 1} || V <- AttrValues]).

Choose a reason for hiding this comment

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

According to Elvis:

Line 461 has a tab at column 0.

-spec get_module_opt_subhost(ejabberd:server(), module(), list()) -> ejabberd:server().
get_module_opt_subhost(Host, Module, Default) ->
Subject = get_module_opt(Host, Module, host, Default),
re:replace(Subject, "@HOST@", Host, [global, {return,binary}]).

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 342

@@ -932,5 +932,6 @@ disable_delivery_test(Config) ->
required_modules() ->
[{mod_pubsub, [
{plugins,[<<"dag">>]},

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 934

-spec get_opt_host(ejabberd:server(), list(), _) -> ejabberd:server().
get_opt_host(Host, Opts, Default) ->
-spec get_opt_subhost(ejabberd:server(), list(), list()) -> ejabberd:server().
get_opt_subhost(Host, Opts, Default) ->
Val = get_opt(host, Opts, Default),
re:replace(Val, "@HOST@", Host, [global, {return,binary}]).

Choose a reason for hiding this comment

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

According to Elvis:

Missing space right "," on line 337

@@ -643,7 +605,7 @@ handle_purge_multiple_messages(ArcJID=#jid{},
handle_purge_single_message(ArcJID=#jid{},
IQ=#iq{sub_el = PurgeEl}) ->
Now = mod_mam_utils:now_to_microseconds(now()),

Choose a reason for hiding this comment

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

According to Compiler:

Warning: erlang:now/0: Deprecated BIF. See the "Time and Time Correction in Erlang" chapter of the ERTS User's Guide for more information.

@@ -482,7 +444,7 @@ handle_lookup_messages(
ArcJID=#jid{},
IQ=#iq{xmlns = MamNs, sub_el = QueryEl}) ->
Now = mod_mam_utils:now_to_microseconds(now()),

Choose a reason for hiding this comment

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

According to Compiler:

Warning: erlang:now/0: Deprecated BIF. See the "Time and Time Correction in Erlang" chapter of the ERTS User's Guide for more information.

@@ -537,7 +499,7 @@ handle_set_message_form(
ArcJID=#jid{},
IQ=#iq{xmlns=MamNs, sub_el = QueryEl}) ->
Now = mod_mam_utils:now_to_microseconds(now()),

Choose a reason for hiding this comment

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

According to Compiler:

Warning: erlang:now/0: Deprecated BIF. See the "Time and Time Correction in Erlang" chapter of the ERTS User's Guide for more information.

@@ -623,7 +585,7 @@ handle_get_message_form(_From=#jid{}, _ArcJID=#jid{}, IQ=#iq{}) ->
handle_purge_multiple_messages(ArcJID=#jid{},
IQ=#iq{sub_el = PurgeEl}) ->
Now = mod_mam_utils:now_to_microseconds(now()),

Choose a reason for hiding this comment

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

According to Compiler:

Warning: erlang:now/0: Deprecated BIF. See the "Time and Time Correction in Erlang" chapter of the ERTS User's Guide for more information.

@fenek
Copy link
Member Author

fenek commented Dec 12, 2016

Resolving now() complaints is not a subject of this PR. :)

@@ -235,6 +236,10 @@ stop(Host) ->
ok = supervisor:terminate_child(ejabberd_sup, Proc),
ok = supervisor:delete_child(ejabberd_sup, Proc).

-spec default_host() -> string().
default_host() ->
"pubsub.@HOST@".
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that before the change it was a binary. Why is it changed to plain string now?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. In most places it was still provided as string.
  2. It's set with a string in config file.
  3. It doesn't make a difference for re:replace

init/0,
stop/0,

'register'/2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does compiler complain about function name register without single quotes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't check. But it's annoying to see it highlighted in editor as BIF anyway. :)

@fenek fenek added the WIP 🚧 label Dec 12, 2016
@fenek fenek force-pushed the improved-subdomain-handling-v2 branch from f4637b9 to 55bef7c Compare December 13, 2016 08:53
@fenek fenek force-pushed the improved-subdomain-handling-v2 branch from 55bef7c to 8c8ce77 Compare December 13, 2016 09:01
@fenek fenek added ready and removed WIP 🚧 labels Dec 13, 2016
@michalwski michalwski merged commit 42d5e2b into master Dec 13, 2016
@michalwski michalwski deleted the improved-subdomain-handling-v2 branch December 13, 2016 13:37
This was referenced Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants