Improve the 'channel' module and add more test cases

- Rename the 'Client' field to 'ClientInfo'
- Remove the 'expiry_interval' from session record
- Add more test cases for emqx_zone module
- Add more test cases for emqx_banned module
- Add more test cases for emqx_message module
- Remove 'sockname', 'conn_mod' fields from type 'client'

src/emqx_banned.erl

@@ -30,7 +30,7 @@
 -boot_mnesia({mnesia, [boot]}).
 -copy_mnesia({mnesia, [copy]}).
 
--export([start_link/0]).
+-export([start_link/0, stop/0]).
 
 -export([ check/1
         , add/1
@@ -69,6 +69,10 @@ mnesia(copy) ->
 start_link() ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
 
+%% for tests
+-spec(stop() -> ok).
+stop() -> gen_server:stop(?MODULE).
+
 -spec(check(emqx_types:client()) -> boolean()).
 check(#{client_id := ClientId,
         username  := Username,
@@ -105,8 +109,7 @@ handle_cast(Msg, State) ->
     {noreply, State}.
 
 handle_info({timeout, TRef, expire}, State = #{expiry_timer := TRef}) ->
-    mnesia:async_dirty(fun expire_banned_items/1,
-                       [erlang:system_time(second)]),
+    mnesia:async_dirty(fun expire_banned_items/1, [erlang:system_time(second)]),
     {noreply, ensure_expiry_timer(State), hibernate};
 
 handle_info(Info, State) ->
@@ -125,7 +128,7 @@ code_change(_OldVsn, State, _Extra) ->
 
 -ifdef(TEST).
 ensure_expiry_timer(State) ->
-    State#{expiry_timer := emqx_misc:start_timer(timer:seconds(1), expire)}.
+    State#{expiry_timer := emqx_misc:start_timer(10, expire)}.
 -else.
 ensure_expiry_timer(State) ->
     State#{expiry_timer := emqx_misc:start_timer(timer:minutes(1), expire)}.

src/emqx_session.erl

@@ -114,8 +114,6 @@
           max_awaiting_rel :: non_neg_integer(),
           %% Awaiting PUBREL Timeout
           await_rel_timeout :: timeout(),
-          %% Session Expiry Interval
-          expiry_interval :: timeout(),
           %% Enqueue Count
           enqueue_cnt :: non_neg_integer(),
           %% Created at
@@ -127,11 +125,12 @@
 -type(publish() :: {publish, emqx_types:packet_id(), emqx_types:message()}).
 
 -define(DEFAULT_BATCH_N, 1000).
--define(ATTR_KEYS, [expiry_interval, created_at]).
+-define(ATTR_KEYS, [max_inflight, max_mqueue, retry_interval,
+                    max_awaiting_rel, await_rel_timeout, created_at]).
 -define(INFO_KEYS, [subscriptions, max_subscriptions, upgrade_qos, inflight,
                     max_inflight, retry_interval, mqueue_len, max_mqueue,
                     mqueue_dropped, next_pkt_id, awaiting_rel, max_awaiting_rel,
-                    await_rel_timeout, expiry_interval, created_at]).
+                    await_rel_timeout, created_at]).
 -define(STATS_KEYS, [subscriptions_cnt, max_subscriptions, inflight, max_inflight,
                      mqueue_len, max_mqueue, mqueue_dropped, awaiting_rel,
                      max_awaiting_rel, enqueue_cnt]).
@@ -142,8 +141,7 @@
 
 %% @doc Init a session.
 -spec(init(emqx_types:client(), Options :: map()) -> session()).
-init(#{zone := Zone}, #{receive_maximum := MaxInflight,
-                        expiry_interval := ExpiryInterval}) ->
+init(#{zone := Zone}, #{receive_maximum := MaxInflight}) ->
     #session{max_subscriptions = get_env(Zone, max_subscriptions, 0),
              subscriptions     = #{},
              upgrade_qos       = get_env(Zone, upgrade_qos, false),
@@ -154,7 +152,6 @@ init(#{zone := Zone}, #{receive_maximum := MaxInflight,
              awaiting_rel      = #{},
              max_awaiting_rel  = get_env(Zone, max_awaiting_rel, 100),
              await_rel_timeout = get_env(Zone, await_rel_timeout, 3600*1000),
-             expiry_interval   = ExpiryInterval,
              enqueue_cnt       = 0,
              created_at        = os:timestamp()
             }.
@@ -210,8 +207,6 @@ info(max_awaiting_rel, #session{max_awaiting_rel = MaxAwaitingRel}) ->
     MaxAwaitingRel;
 info(await_rel_timeout, #session{await_rel_timeout = Timeout}) ->
     Timeout;
-info(expiry_interval, #session{expiry_interval = Interval}) ->
-    Interval;
 info(enqueue_cnt, #session{enqueue_cnt = Cnt}) ->
     Cnt;
 info(created_at, #session{created_at = CreatedAt}) ->

src/emqx_types.erl

@@ -103,9 +103,7 @@
                       atom()   => term()
                      }).
 -type(client() :: #{zone         := zone(),
-                    conn_mod     := maybe(module()),
                     peerhost     := peerhost(),
-                    sockname     := peername(),
                     client_id    := client_id(),
                     username     := username(),
                     peercert     := esockd_peercert:peercert(),

src/emqx_zone.erl

@@ -30,7 +30,7 @@
 -export([ use_username_as_clientid/1
         , enable_stats/1
         , enable_acl/1
-        , enable_banned/1
+        , enable_ban/1
         , enable_flapping_detect/1
         , ignore_loop_deliver/1
         , server_keepalive/1
@@ -88,9 +88,9 @@ enable_stats(Zone) ->
 enable_acl(Zone) ->
     get_env(Zone, enable_acl, true).
 
--spec(enable_banned(zone()) -> boolean()).
-enable_banned(Zone) ->
-    get_env(Zone, enable_banned, false).
+-spec(enable_ban(zone()) -> boolean()).
+enable_ban(Zone) ->
+    get_env(Zone, enable_ban, false).
 
 -spec(enable_flapping_detect(zone()) -> boolean()).
 enable_flapping_detect(Zone) ->

test/emqx_banned_SUITE.erl

@@ -27,6 +27,8 @@ all() -> emqx_ct:all(?MODULE).
 init_per_suite(Config) ->
     application:load(emqx),
     ok = ekka:start(),
+    %% for coverage
+    ok = emqx_banned:mnesia(copy),
     Config.
 
 end_per_suite(_Config) ->
@@ -80,3 +82,14 @@ t_check(_) ->
     ?assertNot(emqx_banned:check(ClientInfo4)),
     ?assertEqual(0, emqx_banned:info(size)).
 
+t_unused(_) ->
+    {ok, Banned} = emqx_banned:start_link(),
+    ok = emqx_banned:add(#banned{who = {client_id, <<"BannedClient">>},
+                                 until = erlang:system_time(second)
+                                }),
+    ?assertEqual(ignored, gen_server:call(Banned, unexpected_req)),
+    ?assertEqual(ok, gen_server:cast(Banned, unexpected_msg)),
+    ?assertEqual(ok, Banned ! ok),
+    timer:sleep(500), %% expiry timer
+    ok = emqx_banned:stop().
+

test/emqx_cm_SUITE.erl

@@ -0,0 +1,60 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2019 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_cm_SUITE).
+
+-compile(export_all).
+-compile(nowarn_export_all).
+
+-include("emqx.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+all() -> emqx_ct:all(?MODULE).
+
+init_per_suite(Config) ->
+    emqx_ct_helpers:start_apps([]),
+    Config.
+
+end_per_suite(_Config) ->
+    emqx_ct_helpers:stop_apps([]).
+
+t_reg_unreg_channel(_) ->
+    error(not_implemented).
+
+t_get_set_chan_attrs(_) ->
+    error(not_implemented).
+
+t_get_set_chan_stats(_) ->
+    error(not_implemented).
+
+t_open_session(_) ->
+    error(not_implemented).
+
+t_discard_session(_) ->
+    error(not_implemented).
+
+t_takeover_session(_) ->
+    error(not_implemented).
+
+t_lookup_channels(_) ->
+    error(not_implemented).
+
+t_lock_clientid(_) ->
+    error(not_implemented).
+
+t_unlock_clientid(_) ->
+    error(not_implemented).
+

test/emqx_inflight_SUITE.erl

@@ -84,8 +84,9 @@ t_is_empty(_) ->
     ?assert(emqx_inflight:is_empty(Inflight1)).
 
 t_window(_) ->
+    ?assertEqual([], emqx_inflight:window(emqx_inflight:new(0))),
     Inflight = emqx_inflight:insert(
                  b, 2, emqx_inflight:insert(
                          a, 1, emqx_inflight:new(2))),
-    [a, b] = emqx_inflight:window(Inflight).
+    ?assertEqual([a, b], emqx_inflight:window(Inflight)).
 

test/emqx_message_SUITE.erl

@@ -56,6 +56,7 @@ t_get_set_flag(_) ->
     ?assertNot(emqx_message:get_flag(retain, Msg3)),
     Msg4 = emqx_message:unset_flag(dup, Msg3),
     Msg5 = emqx_message:unset_flag(retain, Msg4),
+    Msg5 = emqx_message:unset_flag(badflag, Msg5),
     ?assertEqual(undefined, emqx_message:get_flag(dup, Msg5, undefined)),
     ?assertEqual(undefined, emqx_message:get_flag(retain, Msg5, undefined)),
     Msg6 = emqx_message:set_flags(#{dup => true, retain => true}, Msg5),
@@ -81,7 +82,7 @@ t_get_set_header(_) ->
     ?assertEqual(1, emqx_message:get_header(a, Msg3)),
     ?assertEqual(4, emqx_message:get_header(d, Msg2, 4)),
     Msg4 = emqx_message:remove_header(a, Msg3),
-    Msg4 = emqx_message:remove_header(a, Msg3),
+    Msg4 = emqx_message:remove_header(a, Msg4),
     ?assertEqual(#{b => 2, c => 3}, emqx_message:get_headers(Msg4)).
 
 t_undefined_headers(_) ->
@@ -93,16 +94,24 @@ t_undefined_headers(_) ->
 
 t_format(_) ->
     Msg = emqx_message:make(<<"clientid">>, <<"topic">>, <<"payload">>),
-    io:format("~s", [emqx_message:format(Msg)]).
+    io:format("~s~n", [emqx_message:format(Msg)]),
+    Msg1 = #message{id = <<"id">>,
+                    qos = ?QOS_0,
+                    flags = undefined,
+                    headers = undefined
+                   },
+    io:format("~s~n", [emqx_message:format(Msg1)]).
 
 t_expired(_) ->
     Msg = emqx_message:make(<<"clientid">>, <<"topic">>, <<"payload">>),
+    ?assertNot(emqx_message:is_expired(Msg)),
     Msg1 = emqx_message:set_headers(#{'Message-Expiry-Interval' => 1}, Msg),
     timer:sleep(500),
     ?assertNot(emqx_message:is_expired(Msg1)),
     timer:sleep(600),
     ?assert(emqx_message:is_expired(Msg1)),
     timer:sleep(1000),
+    Msg = emqx_message:update_expiry(Msg),
     Msg2 = emqx_message:update_expiry(Msg1),
     ?assertEqual(1, emqx_message:get_header('Message-Expiry-Interval', Msg2)).
 

test/emqx_zone_SUITE.erl

@@ -21,29 +21,81 @@
 
 -include_lib("eunit/include/eunit.hrl").
 
--define(OPTS, [{enable_acl, true},
-               {enable_banned, false}
+-define(ENVS, [{use_username_as_clientid, false},
+               {server_keepalive, 60},
+               {upgrade_qos, false},
+               {session_expiry_interval, 7200},
+               {retry_interval, 20000},
+               {mqueue_store_qos0, true},
+               {mqueue_priorities, none},
+               {mqueue_default_priority, highest},
+               {max_subscriptions, 0},
+               {max_mqueue_len, 1000},
+               {max_inflight, 32},
+               {max_awaiting_rel, 100},
+               {keepalive_backoff, 0.75},
+               {ignore_loop_deliver, false},
+               {idle_timeout, 15000},
+               {force_shutdown_policy, #{max_heap_size => 838860800,
+                                         message_queue_len => 8000}},
+               {force_gc_policy, #{bytes => 1048576, count => 1000}},
+               {enable_stats, true},
+               {enable_flapping_detect, false},
+               {enable_ban, true},
+               {enable_acl, true},
+               {await_rel_timeout, 300000},
+               {acl_deny_action, ignore}
               ]).
 
 all() -> emqx_ct:all(?MODULE).
 
-t_set_get_env(_) ->
+init_per_suite(Config) ->
     _ = application:load(emqx),
-    application:set_env(emqx, zones, [{external, ?OPTS}]),
-    {ok, _} = emqx_zone:start_link(),
-    ?assert(emqx_zone:get_env(external, enable_acl)),
-    ?assertNot(emqx_zone:get_env(external, enable_banned)),
+    application:set_env(emqx, zone_env, val),
+    application:set_env(emqx, zones, [{zone, ?ENVS}]),
+    Config.
+
+end_per_suite(_Config) ->
+    application:unset_env(emqx, zone_env),
+    application:unset_env(emqx, zones).
+
+t_zone_env_func(_) ->
+    lists:foreach(fun({Env, Val}) ->
+                          case erlang:function_exported(emqx_zone, Env, 1) of
+                              true ->
+                                  ?assertEqual(Val, erlang:apply(emqx_zone, Env, [zone]));
+                              false -> ok
+                          end
+                  end, ?ENVS).
+
+t_get_env(_) ->
+    ?assertEqual(val, emqx_zone:get_env(undefined, zone_env)),
+    ?assertEqual(val, emqx_zone:get_env(undefined, zone_env, def)),
+    ?assert(emqx_zone:get_env(zone, enable_acl)),
+    ?assert(emqx_zone:get_env(zone, enable_ban)),
     ?assertEqual(defval, emqx_zone:get_env(extenal, key, defval)),
     ?assertEqual(undefined, emqx_zone:get_env(external, key)),
     ?assertEqual(undefined, emqx_zone:get_env(internal, key)),
-    ?assertEqual(def, emqx_zone:get_env(internal, key, def)),
-    emqx_zone:stop().
+    ?assertEqual(def, emqx_zone:get_env(internal, key, def)).
+
+t_get_set_env(_) ->
+    ok = emqx_zone:set_env(zone, key, val),
+    ?assertEqual(val, emqx_zone:get_env(zone, key)),
+    true = emqx_zone:unset_env(zone, key),
+    ?assertEqual(undefined, emqx_zone:get_env(zone, key)).
 
 t_force_reload(_) ->
     {ok, _} = emqx_zone:start_link(),
-    application:set_env(emqx, zones, [{zone, [{key, val}]}]),
-    ?assertEqual(undefined, emqx_zone:get_env(zone, key)),
+    ?assertEqual(undefined, emqx_zone:get_env(xzone, key)),
+    application:set_env(emqx, zones, [{xzone, [{key, val}]}]),
     ok = emqx_zone:force_reload(),
-    ?assertEqual(val, emqx_zone:get_env(zone, key)),
+    ?assertEqual(val, emqx_zone:get_env(xzone, key)),
+    emqx_zone:stop().
+
+t_uncovered_func(_) ->
+    {ok, Pid} = emqx_zone:start_link(),
+    ignored = gen_server:call(Pid, unexpected_call),
+    ok = gen_server:cast(Pid, unexpected_cast),
+    ok = Pid ! ok,
     emqx_zone:stop().