adding mongoose_subdomain_core

src/domain/mongoose_subdomain_core.erl

@@ -4,7 +4,6 @@
 -behaviour(gen_server).
 
 -include("mongoose_logger.hrl").
-
 %% API
 -export([start/0, stop/0]).
 -export([start_link/0]).
@@ -27,6 +26,14 @@
          code_change/3,
          terminate/2]).
 
+-ifdef(TEST).
+
+-undef(LOG_ERROR).
+-export([log_error/2]).
+-define(LOG_ERROR(Error), ?MODULE:log_error(?FUNCTION_NAME, Error)).
+
+-endif.
+
 -define(SUBDOMAINS_TABLE, ?MODULE).
 -define(REGISTRATION_TABLE, mongoose_subdomain_reg).
 
@@ -67,6 +74,9 @@ start() ->
 stop() ->
     gen_server:stop(?MODULE).
 
+%% this interface is required only to detect errors in unit tests
+log_error(_Function, _Error) -> ok.
+
 -else.
 
 start() ->
@@ -214,7 +224,7 @@ handle_unregister(HostType, SubdomainPattern) ->
 
 -spec handle_add_domain(host_type(), domain()) -> ok.
 handle_add_domain(HostType, Domain) ->
-    mongoose_subdomain_utils:check_domain_name(HostType, Domain),
+    check_domain_name(HostType, Domain),
     %% even if the domain name check fails, it's not easy to solve this
     %% collision. so the best thing we can do is to report it and just keep
     %% the data in both ETS tables (domains and subdomains) for further
@@ -250,23 +260,20 @@ add_subdomains(RegItems, Domain) ->
 -spec add_subdomain(reg_item(), maybe_parent_domain()) -> ok | {error, already_registered}.
 add_subdomain(RegItem, Domain) ->
     #subdomain_item{subdomain = Subdomain} = Item = make_subdomain_item(RegItem, Domain),
-    Info = convert_subdomain_item_to_map(Item),
     case ets:insert_new(?SUBDOMAINS_TABLE, Item) of
         true ->
-            mongoose_subdomain_utils:check_subdomain_name(Info),
+            check_subdomain_name(Item),
             %% even if the subdomain name check fails, it's not easy to solve this
             %% collision. so the best thing we can do is to report it and just keep
             %% the data in both ETS tables (domains and subdomains) for further
             %% troubleshooting.
             ok;
         false ->
-            [ExistingItem] = ets:lookup(?SUBDOMAINS_TABLE, Subdomain),
-            if
-                ExistingItem =:= Item ->
+            case ets:lookup(?SUBDOMAINS_TABLE, Subdomain) of
+                [Item] ->
                     ok; %% exactly the same item is already inserted, it's fine.
-                true ->
-                    ExistingInfo = convert_subdomain_item_to_map(ExistingItem),
-                    mongoose_subdomain_utils:report_subdomains_collision(ExistingInfo, Info),
+                [ExistingItem] ->
+                    report_subdomains_collision(ExistingItem, Item),
                     {error, subdomain_already_exists}
             end
     end.
@@ -289,3 +296,34 @@ convert_subdomain_item_to_map(#subdomain_item{} = Item) ->
     [_ | Values] = tuple_to_list(Item),
     KVList = lists:zip(Fields, Values),
     maps:from_list(KVList).
+
+-spec check_domain_name(mongooseim:host_type(), mongooseim:domain_name()) ->
+    boolean().
+check_domain_name(_HostType, Domain) ->
+    case mongoose_subdomain_core:get_subdomain_info(Domain) of
+        {error, not_found} -> true;
+        {ok, _Info} ->
+            %% TODO: this is critical collision, and it must be reported properly
+            %% think about adding some metric, so devops can set some alarm for it
+            ?LOG_ERROR(#{what => check_domain_name_failed, domain => Domain}),
+            false
+    end.
+
+-spec check_subdomain_name(subdomain_item()) -> boolean().
+check_subdomain_name(#subdomain_item{subdomain = Subdomain} = _SubdomainItem) ->
+    case mongoose_domain_core:get_host_type(Subdomain) of
+        {error, not_found} -> true;
+        {ok, _HostType} ->
+            %% TODO: this is critical collision, and it must be reported properly
+            %% think about adding some metric, so devops can set some alarm for it
+            ?LOG_ERROR(#{what => check_subdomain_name_failed, subdomain => Subdomain}),
+            false
+    end.
+
+-spec report_subdomains_collision(subdomain_item(), subdomain_item()) -> ok.
+report_subdomains_collision(ExistingSubdomainItem, _NewSubdomainItem) ->
+    #subdomain_item{subdomain = Subdomain} = ExistingSubdomainItem,
+    %% TODO: this is critical collision, and it must be reported properly
+    %% think about adding some metric, so devops can set some alarm for it
+    ?LOG_ERROR(#{what => subdomains_collision, subdomain => Subdomain}),
+    ok.

src/domain/mongoose_subdomain_utils.erl

@@ -1,14 +1,9 @@
 -module(mongoose_subdomain_utils).
 
--include("mongoose_logger.hrl").
-
 %% API
 -export([make_subdomain_pattern/1,
          get_fqdn/2,
          subdomain_type/1,
-         check_domain_name/2,
-         check_subdomain_name/1,
-         report_subdomains_collision/2,
          is_subdomain/3]).
 
 -type subdomain_pattern() :: {fqdn | prefix, binary()}.
@@ -38,38 +33,6 @@ get_fqdn({prefix, Prefix}, Domain)   -> <<Prefix/binary, Domain/binary>>.
 subdomain_type({fqdn, _}) -> fqdn;
 subdomain_type({prefix, _}) -> subdomain.
 
--spec check_domain_name(mongooseim:host_type(), mongooseim:domain_name()) ->
-    boolean().
-check_domain_name(_HostType, Domain) ->
-    case mongoose_subdomain_core:get_subdomain_info(Domain) of
-        {error, not_found} -> true;
-        {ok, _Info} ->
-            %% TODO: this is critical collision, and it must be reported properly
-            %% think about adding some metric, so devops can set some alarm for it
-            ?LOG_ERROR(#{what => check_domain_name_failed, domain => Domain}),
-            false
-    end.
-
--spec check_subdomain_name(mongoose_subdomain_core:subdomain_info()) -> boolean().
-check_subdomain_name(#{subdomain := Subdomain} = _SubdomainInfo) ->
-    case mongoose_domain_core:get_host_type(Subdomain) of
-        {error, not_found} -> true;
-        {ok, _HostType} ->
-            %% TODO: this is critical collision, and it must be reported properly
-            %% think about adding some metric, so devops can set some alarm for it
-            ?LOG_ERROR(#{what => check_subdomain_name_failed, subdomain => Subdomain}),
-            false
-    end.
-
--spec report_subdomains_collision(mongoose_subdomain_core:subdomain_info(),
-                                  mongoose_subdomain_core:subdomain_info()) -> ok.
-report_subdomains_collision(#{subdomain := Subdomain} = _ExistingSubdomainInfo,
-                            _NewSubdomainInfo) ->
-    %% TODO: this is critical collision, and it must be reported properly
-    %% think about adding some metric, so devops can set some alarm for it
-    ?LOG_ERROR(#{what => subdomains_collision, subdomain => Subdomain}),
-    ok.
-
 -spec is_subdomain(subdomain_pattern(),
                    Domain :: mongooseim:domain_name(),
                    Subdomain :: mongooseim:domain_name()) -> boolean().

test/mongoose_subdomain_core_SUITE.erl

@@ -25,7 +25,7 @@ all() ->
 
 init_per_suite(Config) ->
     meck:new(mongoose_hooks, [no_link]),
-    meck:new(mongoose_subdomain_utils, [no_link, passthrough]),
+    meck:new(mongoose_subdomain_core, [no_link, passthrough]),
     meck:expect(mongoose_hooks, disable_domain, fun(_, _) -> ok end),
     meck:expect(mongoose_hooks, disable_subdomain, fun(_, _) -> ok end),
     Config.
@@ -45,7 +45,7 @@ init_per_testcase(_, Config) ->
     ok = mongoose_subdomain_core:start(),
     [mongoose_domain_core:insert(Domain, ?DYNAMIC_HOST_TYPE2, dummy_source)
      || Domain <- ?DYNAMIC_DOMAINS],
-    [meck:reset(M) || M <- [mongoose_hooks, mongoose_subdomain_utils]],
+    [meck:reset(M) || M <- [mongoose_hooks, mongoose_subdomain_core]],
     Config.
 
 end_per_testcase(_, Config) ->
@@ -333,16 +333,16 @@ prevents_subdomain_overriding(_Config) ->
     mongoose_domain_core:insert(<<"domain.test">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
     mongoose_subdomain_core:sync(),
     ?assertEqual(3, get_subdomains_table_size()),
-    ExpectedSubdomainInfo1 =
+    ExpectedSubdomainInfo =
         #{host_type => ?DYNAMIC_HOST_TYPE1, subdomain_pattern => Pattern2,
           parent_domain => <<"test">>, packet_handler => Handler,
           subdomain => <<"sub.domain.test">>},
-    ?assertEqual({ok, ExpectedSubdomainInfo1} ,
+    ?assertEqual({ok, ExpectedSubdomainInfo} ,
                  mongoose_subdomain_core:get_subdomain_info(<<"sub.domain.test">>)),
     %% check that removal of "domain.test" doesn't affect "sub.domain.test" subdomain
     mongoose_domain_core:delete("domain.test"),
     mongoose_subdomain_core:sync(),
-    ?assertEqual({ok, ExpectedSubdomainInfo1},
+    ?assertEqual({ok, ExpectedSubdomainInfo},
                  mongoose_subdomain_core:get_subdomain_info(<<"sub.domain.test">>)),
     %%----------------------------------------------------------------
     %% testing type #2 subdomain names collision + double registration
@@ -357,35 +357,26 @@ prevents_subdomain_overriding(_Config) ->
                                                             Pattern3, Handler)),
 
     ?assertEqual(4, get_subdomains_table_size()),
-    ExpectedSubdomainInfo2 =
-        #{host_type => ?DYNAMIC_HOST_TYPE1, subdomain_pattern => Pattern3,
-          parent_domain => no_parent_domain, packet_handler => Handler,
-          subdomain => <<"sub.domain.fqdn">>},
-    ?assertEqual({ok,ExpectedSubdomainInfo2},
+    ?assertEqual({ok,#{host_type => ?DYNAMIC_HOST_TYPE1, subdomain_pattern => Pattern3,
+                       parent_domain => no_parent_domain, packet_handler => Handler,
+                       subdomain => <<"sub.domain.fqdn">>}},
                  mongoose_subdomain_core:get_subdomain_info(<<"sub.domain.fqdn">>)),
     %%------------------------------------------
     %% testing type #3 subdomain names collision
     %%------------------------------------------
     mongoose_domain_core:insert(<<"fqdn">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
     mongoose_subdomain_core:sync(),
     ?assertEqual(5, get_subdomains_table_size()),
-    ExpectedSubdomainInfo3 =
-        #{host_type => ?DYNAMIC_HOST_TYPE1, subdomain_pattern => Pattern3,
-          parent_domain => no_parent_domain, packet_handler => Handler,
-          subdomain => <<"sub.domain.fqdn">>},
-    ?assertEqual({ok, ExpectedSubdomainInfo3},
+    ?assertEqual({ok, #{host_type => ?DYNAMIC_HOST_TYPE1, subdomain_pattern => Pattern3,
+                        parent_domain => no_parent_domain, packet_handler => Handler,
+                        subdomain => <<"sub.domain.fqdn">>}},
                  mongoose_subdomain_core:get_subdomain_info(<<"sub.domain.fqdn">>)),
     %%---------------------------------------
     %% check that all collisions are detected
     %%---------------------------------------
-    ?assertEqual([[ExpectedSubdomainInfo1,
-                   ExpectedSubdomainInfo1#{subdomain_pattern := Pattern1,
-                                           parent_domain => <<"domain.test">>}],
-                  [ExpectedSubdomainInfo2,
-                   ExpectedSubdomainInfo2#{host_type := ?DYNAMIC_HOST_TYPE2}],
-                  [ExpectedSubdomainInfo3,
-                   ExpectedSubdomainInfo3#{subdomain_pattern := Pattern2,
-                                           parent_domain := <<"fqdn">>}]],
+    ?assertEqual([#{what => subdomains_collision, subdomain => <<"sub.domain.test">>},
+                  #{what => subdomains_collision, subdomain => <<"sub.domain.fqdn">>},
+                  #{what => subdomains_collision, subdomain => <<"sub.domain.fqdn">>}],
                  get_list_of_subdomain_collisions()),
     no_domain_collisions().
 
@@ -439,33 +430,31 @@ detects_domain_subdomain_collisions(_Config) ->
     %%------------------------------------------
     ?assertEqual(ok, mongoose_subdomain_core:register_subdomain(?DYNAMIC_HOST_TYPE1,
                                                                 Pattern1, Handler)),
-    mongoose_domain_core:insert(<<"example.net">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
-    %% without this sync call "subdomain.example.net" collision can be detected twice,
-    %% one time by mongoose_subdomain_utils:check_subdomain_name/1 function and then
-    %% second time by mongoose_subdomain_utils:check_domain_name/2.
+    mongoose_domain_core:insert(<<"test.net">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
+    %% without this sync call "subdomain.example.net" collision can be detected
+    %% twice, one time by check_subdomain_name/1 function and then second time
+    %% by check_domain_name/2.
     mongoose_subdomain_core:sync(),
-    mongoose_domain_core:insert(<<"subdomain.example.net">>, ?DYNAMIC_HOST_TYPE2,
+    mongoose_domain_core:insert(<<"subdomain.test.net">>, ?DYNAMIC_HOST_TYPE2,
                                 dummy_src),
-    mongoose_domain_core:insert(<<"subdomain.example.org">>, ?DYNAMIC_HOST_TYPE2,
+    mongoose_domain_core:insert(<<"subdomain.test.org">>, ?DYNAMIC_HOST_TYPE2,
                                 dummy_src),
-    mongoose_domain_core:insert(<<"example.org">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
+    mongoose_domain_core:insert(<<"test.org">>, ?DYNAMIC_HOST_TYPE1, dummy_src),
     mongoose_subdomain_core:sync(),
-    ?assertEqualLists([<<"subdomain.example.org">>, <<"subdomain.example.net">>],
+    ?assertEqualLists([<<"subdomain.test.org">>, <<"subdomain.test.net">>],
                       get_all_subdomains()),
     ?assertEqualLists(
-        [<<"subdomain.example.org">>, <<"subdomain.example.net">> | ?DYNAMIC_DOMAINS],
+        [<<"subdomain.test.org">>, <<"subdomain.test.net">> | ?DYNAMIC_DOMAINS],
         mongoose_domain_core:get_domains_by_host_type(?DYNAMIC_HOST_TYPE2)),
     no_subdomain_collisions(),
-    {ok, SubdomainInfo1} =
-        mongoose_subdomain_core:get_subdomain_info(<<"subdomain.example.org">>),
     ?assertEqual(
-        [{check_domain_name, [?DYNAMIC_HOST_TYPE2, <<"subdomain.example.net">>]},
-         {check_subdomain_name, [SubdomainInfo1]}],
+        [#{what => check_domain_name_failed, domain => <<"subdomain.test.net">>},
+         #{what => check_subdomain_name_failed, subdomain => <<"subdomain.test.org">>}],
         get_list_of_domain_collisions()),
     %% cleanup
-    meck:reset(mongoose_subdomain_utils),
-    Domains = [<<"subdomain.example.net">>, <<"subdomain.example.org">>,
-               <<"example.net">>, <<"example.org">>],
+    meck:reset(mongoose_subdomain_core),
+    Domains = [<<"subdomain.test.net">>, <<"subdomain.test.org">>,
+               <<"test.net">>, <<"test.org">>],
     [mongoose_domain_core:delete(Domain) || Domain <- Domains],
     %%------------------------------------------
     %% testing type #2 subdomain names collision
@@ -481,11 +470,9 @@ detects_domain_subdomain_collisions(_Config) ->
         [<<"some.fqdn">>, <<"another.fqdn">> | ?DYNAMIC_DOMAINS],
         mongoose_domain_core:get_domains_by_host_type(?DYNAMIC_HOST_TYPE2)),
     no_subdomain_collisions(),
-    {ok, SubdomainInfo2} =
-        mongoose_subdomain_core:get_subdomain_info(<<"another.fqdn">>),
     ?assertEqual(
-        [{check_domain_name, [?DYNAMIC_HOST_TYPE2, <<"some.fqdn">>]},
-         {check_subdomain_name, [SubdomainInfo2]}],
+        [#{what => check_domain_name_failed, domain => <<"some.fqdn">>},
+         #{what => check_subdomain_name_failed, subdomain => <<"another.fqdn">>}],
         get_list_of_domain_collisions()).
 
 %%-------------------------------------------------------------------
@@ -552,27 +539,26 @@ no_collisions() ->
     no_subdomain_collisions().
 
 no_domain_collisions() ->
-    Hist = meck:history(mongoose_subdomain_utils),
-    Calls = [Call || {_Pid, {_Mod, Func, _Args}, false = _Result} = Call <- Hist,
-                     Func =:= check_subdomain_name orelse Func =:= check_domain_name],
-    ?assertEqual([], Calls).
+    Hist = meck:history(mongoose_subdomain_core),
+    Errors = [Call || {_P, {_M, log_error = _F, [From, _] = _A}, _R} = Call <- Hist,
+                      From =:= check_subdomain_name orelse From =:= check_domain_name],
+    ?assertEqual([], Errors).
 
 get_list_of_domain_collisions() ->
-    Hist = meck:history(mongoose_subdomain_utils),
-    [{Func, Args} || {_Pid, {_Mod, Func, Args}, false = _Result} <- Hist,
-                     Func =:= check_subdomain_name orelse Func =:= check_domain_name].
+    Hist = meck:history(mongoose_subdomain_core),
+    [Error || {_Pid, {_Mod, log_error = _Func, [From, Error] = _Args}, _Result} <- Hist,
+               From =:= check_subdomain_name orelse From =:= check_domain_name].
 
 no_subdomain_collisions() ->
-    Hist = meck:history(mongoose_subdomain_utils),
-    Calls = [Call || {_P, {_M, report_subdomains_collision, _A}, _R} = Call <- Hist],
-    ?assertEqual([], Calls).
+    Hist = meck:history(mongoose_subdomain_core),
+    Errors = [Call || {_P, {_M, log_error = _F, [From, _] = _A}, _R} = Call <- Hist,
+                      From =:= report_subdomains_collision],
+    ?assertEqual([], Errors).
 
 get_list_of_subdomain_collisions() ->
-    Hist = meck:history(mongoose_subdomain_utils),
-    %% Args is a list of 2 mongoose_subdomain_core:subdomain_info() elements:
-    %%   * the first one - info map for the existing subdomain.
-    %%   * the second one - info map for the new subdomain that we've failed to add.
-    [Args || {_P, {_M, report_subdomains_collision = _F, Args}, _R} <- Hist].
+    Hist = meck:history(mongoose_subdomain_core),
+    [Error || {_Pid, {_Mod, log_error = _Func, [From, Error] = _Args}, _Result} <- Hist,
+               From =:= report_subdomains_collision].
 
 get_list_of_disabled_subdomains() ->
     History = meck:history(mongoose_hooks),