From 8a8729f9ea7c6f130769722ffc548012c7dc290e Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sun, 23 Sep 2018 09:04:41 +0200 Subject: [PATCH 01/28] Make rebar3 xref work. Fixed a bad call in emqx_mod_subscription module also commented out dead code for now in emqx_config.erl --- .travis.yml | 1 + Makefile | 3 + src/emqx_config.erl | 100 +++++++++++++++++----------------- src/emqx_mod_subscription.erl | 2 +- 4 files changed, 55 insertions(+), 51 deletions(-) diff --git a/.travis.yml b/.travis.yml index adef0f3cd..2557513d4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ before_install: script: - make dep-vsn-check - make rebar-compile + - make rebar-xref - make rebar-eunit - make rebar-ct - make rebar-cover diff --git a/Makefile b/Makefile index c5033df7b..e061066b4 100644 --- a/Makefile +++ b/Makefile @@ -91,6 +91,9 @@ cuttlefish: rebar-deps mv _build/default/lib/cuttlefish/cuttlefish ./cuttlefish; \ fi +rebar-xref: + @rebar3 xref + rebar-deps: @rebar3 get-deps diff --git a/src/emqx_config.erl b/src/emqx_config.erl index 2b96f88fc..435ebaea7 100644 --- a/src/emqx_config.erl +++ b/src/emqx_config.erl @@ -66,19 +66,19 @@ reload(_App) -> ok. -spec(write(atom(), list(env())) -> ok | {error, term()}). -write(App, Terms) -> - Configs = lists:map(fun({Key, Val}) -> - {cuttlefish_variable:tokenize(binary_to_list(Key)), binary_to_list(Val)} - end, Terms), - Path = lists:concat([code:priv_dir(App), "/", App, ".schema"]), - Schema = cuttlefish_schema:files([Path]), - case cuttlefish_generator:map(Schema, Configs) of - [{App, Configs1}] -> - emqx_cli_config:write_config(App, Configs), - lists:foreach(fun({Key, Val}) -> application:set_env(App, Key, Val) end, Configs1); - _ -> - error - end. +write(_App, _Terms) -> ok. + % Configs = lists:map(fun({Key, Val}) -> + % {cuttlefish_variable:tokenize(binary_to_list(Key)), binary_to_list(Val)} + % end, Terms), + % Path = lists:concat([code:priv_dir(App), "/", App, ".schema"]), + % Schema = cuttlefish_schema:files([Path]), + % case cuttlefish_generator:map(Schema, Configs) of + % [{App, Configs1}] -> + % emqx_cli_config:write_config(App, Configs), + % lists:foreach(fun({Key, Val}) -> application:set_env(App, Key, Val) end, Configs1); + % _ -> + % error + % end. -spec(dump(atom(), list(env())) -> ok | {error, term()}). dump(_App, _Terms) -> @@ -86,47 +86,47 @@ dump(_App, _Terms) -> ok. -spec(set(atom(), list(), list()) -> ok). -set(App, Par, Val) -> - emqx_cli_config:run(["config", - "set", - lists:concat([Par, "=", Val]), - lists:concat(["--app=", App])]). +set(_App, _Par, _Val) -> ok. + % emqx_cli_config:run(["config", + % "set", + % lists:concat([Par, "=", Val]), + % lists:concat(["--app=", App])]). -spec(get(atom(), list()) -> undefined | {ok, term()}). -get(App, Par) -> - case emqx_cli_config:get_cfg(App, Par) of - undefined -> undefined; - Val -> {ok, Val} - end. +get(_App, _Par) -> error(no_impl). + % case emqx_cli_config:get_cfg(App, Par) of + % undefined -> undefined; + % Val -> {ok, Val} + % end. -spec(get(atom(), list(), atom()) -> term()). -get(App, Par, Def) -> - emqx_cli_config:get_cfg(App, Par, Def). +get(_App, _Par, _Def) -> error(no_impl). + % emqx_cli_config:get_cfg(App, Par, Def). -read_(App) -> - Configs = emqx_cli_config:read_config(App), - Path = lists:concat([code:priv_dir(App), "/", App, ".schema"]), - case filelib:is_file(Path) of - false -> - []; - true -> - {_, Mappings, _} = cuttlefish_schema:files([Path]), - OptionalCfg = lists:foldl(fun(Map, Acc) -> - Key = cuttlefish_mapping:variable(Map), - case proplists:get_value(Key, Configs) of - undefined -> - [{cuttlefish_variable:format(Key), "", cuttlefish_mapping:doc(Map), false} | Acc]; - _ -> Acc - end - end, [], Mappings), - RequiredCfg = lists:foldl(fun({Key, Val}, Acc) -> - case lists:keyfind(Key, 2, Mappings) of - false -> Acc; - Map -> - [{cuttlefish_variable:format(Key), Val, cuttlefish_mapping:doc(Map), true} | Acc] - end - end, [], Configs), - RequiredCfg ++ OptionalCfg - end. +read_(_App) -> error(no_impl). + % Configs = emqx_cli_config:read_config(App), + % Path = lists:concat([code:priv_dir(App), "/", App, ".schema"]), + % case filelib:is_file(Path) of + % false -> + % []; + % true -> + % {_, Mappings, _} = cuttlefish_schema:files([Path]), + % OptionalCfg = lists:foldl(fun(Map, Acc) -> + % Key = cuttlefish_mapping:variable(Map), + % case proplists:get_value(Key, Configs) of + % undefined -> + % [{cuttlefish_variable:format(Key), "", cuttlefish_mapping:doc(Map), false} | Acc]; + % _ -> Acc + % end + % end, [], Mappings), + % RequiredCfg = lists:foldl(fun({Key, Val}, Acc) -> + % case lists:keyfind(Key, 2, Mappings) of + % false -> Acc; + % Map -> + % [{cuttlefish_variable:format(Key), Val, cuttlefish_mapping:doc(Map), true} | Acc] + % end + % end, [], Configs), + % RequiredCfg ++ OptionalCfg + % end. diff --git a/src/emqx_mod_subscription.erl b/src/emqx_mod_subscription.erl index 48edac2c4..aed7f5af2 100644 --- a/src/emqx_mod_subscription.erl +++ b/src/emqx_mod_subscription.erl @@ -36,7 +36,7 @@ on_session_created(#{client_id := ClientId}, SessAttrs, Topics) -> emqx_session:subscribe(self(), [{Replace(Topic), #{qos => QoS}} || {Topic, QoS} <- Topics]). unload(_) -> - emqx_hooks:delete('session.created', fun ?MODULE:on_session_created/3). + emqx_hooks:del('session.created', fun ?MODULE:on_session_created/3). %%-------------------------------------------------------------------- %% Internal functions From 9b2629e884e3470fb5e54899e739ab24376c21b9 Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sun, 23 Sep 2018 09:12:39 +0200 Subject: [PATCH 02/28] Fix dependency version discrepancy between Makefile and rebar.config --- rebar.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rebar.config b/rebar.config index aa77f3a02..f94258e6b 100644 --- a/rebar.config +++ b/rebar.config @@ -10,7 +10,7 @@ [{gen_rpc, "2.2.0"}, {ekka, "v0.4.1"}, {clique, "develop"}, - {esockd, "v5.4"}, + {esockd, "v5.4.1"}, {cuttlefish, "emqx30"} ]}. From 0b0ef9bd5484aacd1da0f68862557e55687aea18 Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sun, 23 Sep 2018 10:13:02 +0200 Subject: [PATCH 03/28] Fix default QoS in test cases --- test/emqx_broker_SUITE.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/emqx_broker_SUITE.erl b/test/emqx_broker_SUITE.erl index 8cadbf00d..7baa248f3 100644 --- a/test/emqx_broker_SUITE.erl +++ b/test/emqx_broker_SUITE.erl @@ -100,7 +100,7 @@ t_local_subscribe(_) -> timer:sleep(10), ?assertEqual([{self(), undefined}], emqx:subscribers("$local/topic0")), ?assertEqual([{self(), <<"clientId">>}], emqx:subscribers("$local/topic1")), - ?assertEqual([{<<"$local/topic1">>, #{}}, + ?assertEqual([{<<"$local/topic1">>, #{ qos => 0 }}, {<<"$local/topic2">>, #{ qos => 2 }}], emqx:subscriptions({self(), <<"clientId">>})), ?assertEqual(ok, emqx:unsubscribe("$local/topic0")), @@ -117,9 +117,9 @@ t_shared_subscribe(_) -> timer:sleep(10), ct:log("share subscriptions: ~p~n", [emqx:subscriptions({self(), undefined})]), ?assertEqual([{self(), undefined}], emqx:subscribers(<<"$local/$share/group1/topic1">>)), - ?assertEqual([{<<"$local/$share/group1/topic1">>, #{}}, - {<<"$queue/topic3">>, #{}}, - {<<"$share/group2/topic2">>, #{}}], + ?assertEqual([{<<"$local/$share/group1/topic1">>, #{qos => 0}}, + {<<"$queue/topic3">>, #{qos => 0}}, + {<<"$share/group2/topic2">>, #{qos => 0}}], lists:sort(emqx:subscriptions({self(), undefined}))), emqx:unsubscribe("$local/$share/group1/topic1"), emqx:unsubscribe("$share/group2/topic2"), From c123064afcf186151c29a000266cc1e7e7a89ef0 Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sun, 23 Sep 2018 10:18:25 +0200 Subject: [PATCH 04/28] Upload coveralls data only when sucess This is done by moving coveralls send to after_success section in .travis.yml. having it in after_success should not make the build fail if coveralls send fails due to server being overloaded etc. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 2557513d4..8960ab7c7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,8 @@ script: - make rebar-eunit - make rebar-ct - make rebar-cover + +after_success: - make coveralls sudo: false From db520b9b9427fca0556e396fa1fe1c8d38808520 Mon Sep 17 00:00:00 2001 From: Feng Lee Date: Tue, 25 Sep 2018 22:37:45 +0800 Subject: [PATCH 05/28] Disable the rate_limit of MQTT/TCP listener by default. --- etc/emqx.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/emqx.conf b/etc/emqx.conf index b7ff0d654..10da1a0b9 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -766,7 +766,7 @@ listener.tcp.external.zone = external ## ## Value: rate,burst ## Unit: Bps -listener.tcp.external.rate_limit = 1024,4096 +## listener.tcp.external.rate_limit = 1024,4096 ## The access control rules for the MQTT/TCP listener. ## From d1c72b92aff6fe8fc2dde4598a07b181252789e2 Mon Sep 17 00:00:00 2001 From: HuangDan Date: Thu, 27 Sep 2018 10:46:14 +0800 Subject: [PATCH 06/28] Add 'sticky' dispatch strategy for shared subscription --- etc/emqx.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/emqx.conf b/etc/emqx.conf index 10da1a0b9..60b576379 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -1895,6 +1895,7 @@ broker.session_locking_strategy = quorum ## Value: Enum ## - random ## - round_robbin +## - sticky ## - hash broker.shared_subscription_strategy = random From 96b5d71a67a983e51af0a3b31d2983d3963c04f1 Mon Sep 17 00:00:00 2001 From: HuangDan Date: Wed, 26 Sep 2018 19:11:50 +0800 Subject: [PATCH 07/28] Fix args errors on emqx_hook:run('message.acked') --- src/emqx_session.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/emqx_session.erl b/src/emqx_session.erl index 14ed2a21d..ad3e66cc5 100644 --- a/src/emqx_session.erl +++ b/src/emqx_session.erl @@ -806,7 +806,7 @@ await(PacketId, Msg, State = #state{inflight = Inflight}) -> acked(puback, PacketId, State = #state{client_id = ClientId, inflight = Inflight}) -> case emqx_inflight:lookup(PacketId, Inflight) of {value, {publish, {_, Msg}, _Ts}} -> - emqx_hooks:run('message.acked', [#{client_id =>ClientId}], Msg), + emqx_hooks:run('message.acked', [#{client_id => ClientId}], Msg), State#state{inflight = emqx_inflight:delete(PacketId, Inflight)}; none -> ?LOG(warning, "Duplicated PUBACK PacketId ~w", [PacketId], State), @@ -816,7 +816,7 @@ acked(puback, PacketId, State = #state{client_id = ClientId, inflight = Infligh acked(pubrec, PacketId, State = #state{client_id = ClientId, inflight = Inflight}) -> case emqx_inflight:lookup(PacketId, Inflight) of {value, {publish, {_, Msg}, _Ts}} -> - emqx_hooks:run('message.acked', [ClientId], Msg), + emqx_hooks:run('message.acked', [#{client_id => ClientId}], Msg), State#state{inflight = emqx_inflight:update(PacketId, {pubrel, PacketId, os:timestamp()}, Inflight)}; {value, {pubrel, PacketId, _Ts}} -> ?LOG(warning, "Duplicated PUBREC PacketId ~w", [PacketId], State), From 3bab3cbd2aed89cb20bef259038364d845f9e5bc Mon Sep 17 00:00:00 2001 From: spring2maz Date: Thu, 27 Sep 2018 21:50:23 +0200 Subject: [PATCH 08/28] Shared subscriber should be keyed by SharedName + Topic Prior to this change, if a producer session produces to two or more shared subscriber groups, the previously picked subscriber for sticky strategy may not be a valid member for the next group. --- src/emqx_shared_sub.erl | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/emqx_shared_sub.erl b/src/emqx_shared_sub.erl index 02422fe55..c6fef6d6f 100644 --- a/src/emqx_shared_sub.erl +++ b/src/emqx_shared_sub.erl @@ -89,7 +89,7 @@ dispatch(Group, Topic, Delivery = #delivery{message = Msg, results = Results}) - end. pick(sticky, ClientId, Group, Topic) -> - Sub0 = erlang:get(shared_sub_sticky), + Sub0 = erlang:get({shared_sub_sticky, Group, Topic}), case is_sub_alive(Sub0) of true -> %% the old subscriber is still alive @@ -99,32 +99,33 @@ pick(sticky, ClientId, Group, Topic) -> %% randomly pick one for the first message Sub = do_pick(random, ClientId, Group, Topic), %% stick to whatever pick result - erlang:put(shared_sub_sticky, Sub), + erlang:put({shared_sub_sticky, Group, Topic}, Sub), Sub end; pick(Strategy, ClientId, Group, Topic) -> do_pick(Strategy, ClientId, Group, Topic). do_pick(Strategy, ClientId, Group, Topic) -> - All = subscribers(Group, Topic), - pick_subscriber(Strategy, ClientId, All). + case subscribers(Group, Topic) of + [] -> false; + [Sub] -> Sub; + All -> pick_subscriber(Group, Topic, Strategy, ClientId, All) + end. -pick_subscriber(_, _ClientId, []) -> false; -pick_subscriber(_, _ClientId, [Sub]) -> Sub; -pick_subscriber(Strategy, ClientId, Subs) -> - Nth = do_pick_subscriber(Strategy, ClientId, length(Subs)), +pick_subscriber(Group, Topic, Strategy, ClientId, Subs) -> + Nth = do_pick_subscriber(Group, Topic, Strategy, ClientId, length(Subs)), lists:nth(Nth, Subs). -do_pick_subscriber(random, _ClientId, Count) -> +do_pick_subscriber(_Group, _Topic, random, _ClientId, Count) -> rand:uniform(Count); -do_pick_subscriber(hash, ClientId, Count) -> +do_pick_subscriber(_Group, _Topic, hash, ClientId, Count) -> 1 + erlang:phash2(ClientId) rem Count; -do_pick_subscriber(round_robin, _ClientId, Count) -> - Rem = case erlang:get(shared_sub_round_robin) of +do_pick_subscriber(Group, Topic, round_robin, _ClientId, Count) -> + Rem = case erlang:get({shared_sub_round_robin, Group, Topic}) of undefined -> 0; N -> (N + 1) rem Count end, - _ = erlang:put(shared_sub_round_robin, Rem), + _ = erlang:put({shared_sub_round_robin, Group, Topic}, Rem), Rem + 1. subscribers(Group, Topic) -> From 2a0bbd1c37bee148c9d47be7ee7786ecac1846dd Mon Sep 17 00:00:00 2001 From: HuangDan Date: Thu, 27 Sep 2018 14:43:06 +0800 Subject: [PATCH 09/28] Fixed errors './cuttlefish: command not found' when running make app.config --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index e061066b4..549237e67 100644 --- a/Makefile +++ b/Makefile @@ -73,10 +73,10 @@ etc/gen.emqx.conf: bbmustache etc/emqx.conf ok = file:write_file('etc/gen.emqx.conf', Targ), \ halt(0)." -app.config: etc/gen.emqx.conf +app.config: cuttlefish etc/gen.emqx.conf $(verbose) ./cuttlefish -l info -e etc/ -c etc/gen.emqx.conf -i priv/emqx.schema -d data/ -ct: cuttlefish app.config +ct: app.config rebar-cover: @rebar3 cover @@ -103,7 +103,7 @@ rebar-eunit: cuttlefish rebar-compile: @rebar3 compile -rebar-ct: cuttlefish app.config +rebar-ct: app.config @rebar3 as test compile @ln -s -f '../../../../etc' _build/test/lib/emqx/ @ln -s -f '../../../../data' _build/test/lib/emqx/ From 1bc175e0ce8d40a6ea27f7bf2aa50bf3065f85e1 Mon Sep 17 00:00:00 2001 From: HuangDan Date: Thu, 27 Sep 2018 19:49:13 +0800 Subject: [PATCH 10/28] Add mountpoint config to zone configs --- etc/emqx.conf | 18 ++++++++++++++++++ priv/emqx.schema | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/etc/emqx.conf b/etc/emqx.conf index 60b576379..18a7d6fd6 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -665,6 +665,15 @@ zone.external.max_mqueue_len = 1000 ## Value: false | true zone.external.mqueue_store_qos0 = true +## All the topics will be prefixed with the mountpoint path if this option is enabled. +## +## Variables in mountpoint path: +## - %c: clientid +## - %u: username +## +## Value: String +## zone.external.mountpoint = devicebound/ + ##-------------------------------------------------------------------- ## Internal Zone @@ -715,6 +724,15 @@ zone.internal.max_mqueue_len = 1000 ## Value: false | true zone.internal.mqueue_store_qos0 = true +## All the topics will be prefixed with the mountpoint path if this option is enabled. +## +## Variables in mountpoint path: +## - %c: clientid +## - %u: username +## +## Value: String +## zone.internal.mountpoint = cloudbound/ + ##-------------------------------------------------------------------- ## Listeners ##-------------------------------------------------------------------- diff --git a/priv/emqx.schema b/priv/emqx.schema index 3d9328bf2..2450a6876 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -842,6 +842,10 @@ end}. {datatype, string} ]}. +{mapping, "zone.$name.mountpoint", "emqx.zones", [ + {datatype, string} +]}. + {translation, "emqx.zones", fun(Conf) -> Mapping = fun("retain_available", Val) -> {mqtt_retain_available, Val}; From 064db6520668c23b26b89bfb7deb8eb38bc9eb97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Thu, 27 Sep 2018 09:06:18 +0800 Subject: [PATCH 11/28] improve receive maximum in connect packet --- src/emqx_inflight.erl | 6 +++++- src/emqx_packet.erl | 5 +++++ src/emqx_protocol.erl | 43 +++++++++++++++++++++++++-------------- src/emqx_session.erl | 12 ++++++++--- src/emqx_sm.erl | 3 ++- test/emqx_mock_client.erl | 3 ++- test/emqx_sm_SUITE.erl | 2 +- 7 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/emqx_inflight.erl b/src/emqx_inflight.erl index 3353983d8..876052974 100644 --- a/src/emqx_inflight.erl +++ b/src/emqx_inflight.erl @@ -14,7 +14,7 @@ -module(emqx_inflight). --export([new/1, contain/2, lookup/2, insert/3, update/3, delete/2, values/1, +-export([new/1, contain/2, lookup/2, insert/3, update/3, update_size/2, delete/2, values/1, to_list/1, size/1, max_size/1, is_full/1, is_empty/1, window/1]). -type(max_size() :: pos_integer()). @@ -46,6 +46,10 @@ delete(Key, {?MODULE, MaxSize, Tree}) -> update(Key, Val, {?MODULE, MaxSize, Tree}) -> {?MODULE, MaxSize, gb_trees:update(Key, Val, Tree)}. +-spec(update_size(integer(), inflight()) -> inflight()). +update_size(MaxSize, {?MODULE, _OldMaxSize, Tree}) -> + {?MODULE, MaxSize, Tree}. + -spec(is_full(inflight()) -> boolean()). is_full({?MODULE, 0, _Tree}) -> false; diff --git a/src/emqx_packet.erl b/src/emqx_packet.erl index fc90cf492..c1bbbb6fd 100644 --- a/src/emqx_packet.erl +++ b/src/emqx_packet.erl @@ -61,6 +61,11 @@ validate(?PUBLISH_PACKET(_QoS, Topic, _, Properties, _)) -> ((not emqx_topic:wildcard(Topic)) orelse error(topic_name_invalid)) andalso validate_properties(?PUBLISH, Properties); +validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' := 0}})) -> + error(protocol_error); +validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' := _}})) -> + true; + validate(_Packet) -> true. diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index 15a287116..753dc099b 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -208,11 +208,8 @@ received(Packet = ?PACKET(Type), PState) -> true -> {Packet1, PState1} = preprocess_properties(Packet, PState), process_packet(Packet1, inc_stats(recv, Type, PState1)); - {'EXIT', {topic_filters_invalid, _Stacktrace}} -> - deliver({disconnect, ?RC_PROTOCOL_ERROR}, PState), - {error, topic_filters_invalid, PState}; {'EXIT', {Reason, _Stacktrace}} -> - deliver({disconnect, ?RC_MALFORMED_PACKET}, PState), + deliver({disconnect, rc(Reason)}, PState), {error, Reason, PState} end. @@ -593,17 +590,25 @@ try_open_session(#pstate{zone = Zone, clean_start => CleanStart }, - case emqx_sm:open_session(maps:put(expiry_interval, if - ProtoVer =:= ?MQTT_PROTO_V5 -> - maps:get('Session-Expiry-Interval', ConnProps, 0); - true -> - case CleanStart of - true -> - 0; - false -> - emqx_zone:get_env(Zone, session_expiry_interval, 16#ffffffff) - end - end, SessAttrs)) of + MaxInflight = #{max_inflight => if + ProtoVer =:= ?MQTT_PROTO_V5 -> + maps:get('Receive-Maximum', ConnProps, 65535); + true -> + emqx_zone:get_env(Zone, max_inflight, 65535) + end}, + SessionExpiryInterval = #{expiry_interval => if + ProtoVer =:= ?MQTT_PROTO_V5 -> + maps:get('Session-Expiry-Interval', ConnProps, 0); + true -> + case CleanStart of + true -> + 0; + false -> + emqx_zone:get_env(Zone, session_expiry_interval, 16#ffffffff) + end + end}, + + case emqx_sm:open_session(maps:merge(SessAttrs, maps:merge(MaxInflight, SessionExpiryInterval))) of {ok, SPid} -> {ok, SPid, false}; Other -> Other @@ -782,6 +787,14 @@ start_keepalive(Secs, #pstate{zone = Zone}) when Secs > 0 -> Backoff = emqx_zone:get_env(Zone, keepalive_backoff, 0.75), self() ! {keepalive, start, round(Secs * Backoff)}. +rc(Reason) -> + case Reason of + protocol_error -> ?RC_PROTOCOL_ERROR; + topic_filters_invalid -> ?RC_TOPIC_FILTER_INVALID; + topic_name_invalid -> ?RC_TOPIC_NAME_INVALID; + _ -> ?RC_MALFORMED_PACKET + end. + %%----------------------------------------------------------------------------- %% Parse topic filters %%----------------------------------------------------------------------------- diff --git a/src/emqx_session.erl b/src/emqx_session.erl index ad3e66cc5..4d570bc08 100644 --- a/src/emqx_session.erl +++ b/src/emqx_session.erl @@ -47,7 +47,7 @@ -export([info/1, attrs/1]). -export([stats/1]). -export([resume/2, discard/2]). --export([update_expiry_interval/2]). +-export([update_expiry_interval/2, update_max_inflight/2]). -export([subscribe/2, subscribe/4]). -export([publish/3]). -export([puback/2, puback/3]). @@ -318,6 +318,9 @@ discard(SPid, ByPid) -> update_expiry_interval(SPid, Interval) -> gen_server:cast(SPid, {expiry_interval, Interval * 1000}). +update_max_inflight(SPid, MaxInflight) -> + gen_server:cast(SPid, {max_inflight, MaxInflight}). + -spec(close(spid()) -> ok). close(SPid) -> gen_server:call(SPid, close, infinity). @@ -331,10 +334,10 @@ init([Parent, #{zone := Zone, username := Username, conn_pid := ConnPid, clean_start := CleanStart, - expiry_interval := ExpiryInterval}]) -> + expiry_interval := ExpiryInterval, + max_inflight := MaxInflight}]) -> process_flag(trap_exit, true), true = link(ConnPid), - MaxInflight = get_env(Zone, max_inflight), IdleTimout = get_env(Zone, idle_timeout, 30000), State = #state{idle_timeout = IdleTimout, clean_start = CleanStart, @@ -543,6 +546,9 @@ handle_cast({resume, ConnPid}, State = #state{client_id = ClientId, handle_cast({expiry_interval, Interval}, State) -> {noreply, State#state{expiry_interval = Interval}}; +handle_cast({max_inflight, MaxInflight}, State) -> + {noreply, State#state{inflight = emqx_inflight:update_size(MaxInflight, State#state.inflight)}}; + handle_cast(Msg, State) -> emqx_logger:error("[Session] unexpected cast: ~p", [Msg]), {noreply, State}. diff --git a/src/emqx_sm.erl b/src/emqx_sm.erl index 36d416f3b..98046823f 100644 --- a/src/emqx_sm.erl +++ b/src/emqx_sm.erl @@ -56,10 +56,11 @@ open_session(SessAttrs = #{clean_start := true, client_id := ClientId, conn_pid end, emqx_sm_locker:trans(ClientId, CleanStart); -open_session(SessAttrs = #{clean_start := false, client_id := ClientId, conn_pid := ConnPid}) -> +open_session(SessAttrs = #{clean_start := false, client_id := ClientId, conn_pid := ConnPid, max_inflight := MaxInflight}) -> ResumeStart = fun(_) -> case resume_session(ClientId, ConnPid) of {ok, SPid} -> + emqx_session:update_max_inflight(SPid, MaxInflight), {ok, SPid, true}; {error, not_found} -> emqx_session_sup:start_session(SessAttrs) diff --git a/test/emqx_mock_client.erl b/test/emqx_mock_client.erl index 4528239e6..0aa01458e 100644 --- a/test/emqx_mock_client.erl +++ b/test/emqx_mock_client.erl @@ -51,7 +51,8 @@ handle_call({start_session, ClientPid, ClientId, Zone}, _From, State) -> conn_pid => ClientPid, clean_start => true, username => undefined, - expiry_interval => 0 + expiry_interval => 0, + max_inflight => 0 }, {ok, SessPid} = emqx_sm:open_session(Attrs), {reply, {ok, SessPid}, diff --git a/test/emqx_sm_SUITE.erl b/test/emqx_sm_SUITE.erl index 6f9b92399..1f85d4724 100644 --- a/test/emqx_sm_SUITE.erl +++ b/test/emqx_sm_SUITE.erl @@ -25,7 +25,7 @@ t_open_close_session(_) -> emqx_ct_broker_helpers:run_setup_steps(), {ok, ClientPid} = emqx_mock_client:start_link(<<"client">>), Attrs = #{clean_start => true, client_id => <<"client">>, conn_pid => ClientPid, - zone => internal, username => <<"zhou">>, expiry_interval => 0}, + zone => internal, username => <<"zhou">>, expiry_interval => 0, max_inflight => 0}, {ok, SPid} = emqx_sm:open_session(Attrs), [{<<"client">>, SPid}] = emqx_sm:lookup_session(<<"client">>), SPid = emqx_sm:lookup_session_pid(<<"client">>), From 2d354ca88386ab360e0dc862fc4b6483262758e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Fri, 28 Sep 2018 10:23:37 +0800 Subject: [PATCH 12/28] Improve topic alias maximum in connect packet --- src/emqx_mqtt_caps.erl | 2 +- src/emqx_protocol.erl | 62 ++++++++++++---------- src/emqx_session.erl | 96 ++++++++++++++++++++--------------- src/emqx_sm.erl | 8 ++- test/emqx_mock_client.erl | 3 +- test/emqx_mqtt_caps_SUITE.erl | 3 +- test/emqx_sm_SUITE.erl | 2 +- 7 files changed, 102 insertions(+), 74 deletions(-) diff --git a/src/emqx_mqtt_caps.erl b/src/emqx_mqtt_caps.erl index b8b7a5b3a..5f919d9c0 100644 --- a/src/emqx_mqtt_caps.erl +++ b/src/emqx_mqtt_caps.erl @@ -44,7 +44,7 @@ -define(PUBCAP_KEYS, [max_qos_allowed, mqtt_retain_available, - mqtt_topic_alias + max_topic_alias ]). -define(SUBCAP_KEYS, [max_qos_allowed, max_topic_levels, diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index 753dc099b..88f07138d 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -574,13 +574,11 @@ maybe_assign_client_id(PState = #pstate{client_id = <<>>, ackprops = AckProps}) maybe_assign_client_id(PState) -> PState. -try_open_session(#pstate{zone = Zone, - proto_ver = ProtoVer, - client_id = ClientId, - conn_pid = ConnPid, - conn_props = ConnProps, - username = Username, - clean_start = CleanStart}) -> +try_open_session(PState = #pstate{zone = Zone, + client_id = ClientId, + conn_pid = ConnPid, + username = Username, + clean_start = CleanStart}) -> SessAttrs = #{ zone => Zone, @@ -590,30 +588,42 @@ try_open_session(#pstate{zone = Zone, clean_start => CleanStart }, - MaxInflight = #{max_inflight => if - ProtoVer =:= ?MQTT_PROTO_V5 -> - maps:get('Receive-Maximum', ConnProps, 65535); - true -> - emqx_zone:get_env(Zone, max_inflight, 65535) - end}, - SessionExpiryInterval = #{expiry_interval => if - ProtoVer =:= ?MQTT_PROTO_V5 -> - maps:get('Session-Expiry-Interval', ConnProps, 0); - true -> - case CleanStart of - true -> - 0; - false -> - emqx_zone:get_env(Zone, session_expiry_interval, 16#ffffffff) - end - end}, - - case emqx_sm:open_session(maps:merge(SessAttrs, maps:merge(MaxInflight, SessionExpiryInterval))) of + SessAttrs1 = lists:foldl(fun set_session_attrs/2, SessAttrs, [{max_inflight, PState}, {expiry_interval, PState}, {topic_alias_maximum, PState}]), + case emqx_sm:open_session(SessAttrs1) of {ok, SPid} -> {ok, SPid, false}; Other -> Other end. +set_session_attrs({max_inflight, #pstate{zone = Zone, proto_ver = ProtoVer, conn_props = ConnProps}}, SessAttrs) -> + maps:put(max_inflight, if + ProtoVer =:= ?MQTT_PROTO_V5 -> + maps:get('Receive-Maximum', ConnProps, 65535); + true -> + emqx_zone:get_env(Zone, max_inflight, 65535) + end, SessAttrs); +set_session_attrs({expiry_interval, #pstate{zone = Zone, proto_ver = ProtoVer, conn_props = ConnProps, clean_start = CleanStart}}, SessAttrs) -> + maps:put(expiry_interval, if + ProtoVer =:= ?MQTT_PROTO_V5 -> + maps:get('Session-Expiry-Interval', ConnProps, 0); + true -> + case CleanStart of + true -> 0; + false -> + emqx_zone:get_env(Zone, session_expiry_interval, 16#ffffffff) + end + end, SessAttrs); +set_session_attrs({topic_alias_maximum, #pstate{zone = Zone, proto_ver = ProtoVer, conn_props = ConnProps}}, SessAttrs) -> + maps:put(topic_alias_maximum, if + ProtoVer =:= ?MQTT_PROTO_V5 -> + maps:get('Topic-Alias-Maximum', ConnProps, 0); + true -> + emqx_zone:get_env(Zone, max_topic_alias, 0) + end, SessAttrs); +set_session_attrs({_, #pstate{}}, SessAttrs) -> + SessAttrs. + + authenticate(Credentials, Password) -> case emqx_access_control:authenticate(Credentials, Password) of ok -> {ok, false}; diff --git a/src/emqx_session.erl b/src/emqx_session.erl index 4d570bc08..4514debcb 100644 --- a/src/emqx_session.erl +++ b/src/emqx_session.erl @@ -47,7 +47,7 @@ -export([info/1, attrs/1]). -export([stats/1]). -export([resume/2, discard/2]). --export([update_expiry_interval/2, update_max_inflight/2]). +-export([update_expiry_interval/2, update_misc/2]). -export([subscribe/2, subscribe/4]). -export([publish/3]). -export([puback/2, puback/3]). @@ -145,7 +145,9 @@ enqueue_stats = 0, %% Created at - created_at :: erlang:timestamp() + created_at :: erlang:timestamp(), + + topic_alias_maximum :: pos_integer() }). -type(spid() :: pid()). @@ -318,8 +320,8 @@ discard(SPid, ByPid) -> update_expiry_interval(SPid, Interval) -> gen_server:cast(SPid, {expiry_interval, Interval * 1000}). -update_max_inflight(SPid, MaxInflight) -> - gen_server:cast(SPid, {max_inflight, MaxInflight}). +update_misc(SPid, Misc) -> + gen_server:cast(SPid, {update_misc, Misc}). -spec(close(spid()) -> ok). close(SPid) -> @@ -329,36 +331,38 @@ close(SPid) -> %% gen_server callbacks %%------------------------------------------------------------------------------ -init([Parent, #{zone := Zone, - client_id := ClientId, - username := Username, - conn_pid := ConnPid, - clean_start := CleanStart, - expiry_interval := ExpiryInterval, - max_inflight := MaxInflight}]) -> +init([Parent, #{zone := Zone, + client_id := ClientId, + username := Username, + conn_pid := ConnPid, + clean_start := CleanStart, + expiry_interval := ExpiryInterval, + max_inflight := MaxInflight, + topic_alias_maximum := TopicAliasMaximum}]) -> process_flag(trap_exit, true), true = link(ConnPid), IdleTimout = get_env(Zone, idle_timeout, 30000), - State = #state{idle_timeout = IdleTimout, - clean_start = CleanStart, - binding = binding(ConnPid), - client_id = ClientId, - username = Username, - conn_pid = ConnPid, - subscriptions = #{}, - max_subscriptions = get_env(Zone, max_subscriptions, 0), - upgrade_qos = get_env(Zone, upgrade_qos, false), - inflight = emqx_inflight:new(MaxInflight), - mqueue = init_mqueue(Zone), - retry_interval = get_env(Zone, retry_interval, 0), - awaiting_rel = #{}, - await_rel_timeout = get_env(Zone, await_rel_timeout), - max_awaiting_rel = get_env(Zone, max_awaiting_rel), - expiry_interval = ExpiryInterval, - enable_stats = get_env(Zone, enable_stats, true), - deliver_stats = 0, - enqueue_stats = 0, - created_at = os:timestamp() + State = #state{idle_timeout = IdleTimout, + clean_start = CleanStart, + binding = binding(ConnPid), + client_id = ClientId, + username = Username, + conn_pid = ConnPid, + subscriptions = #{}, + max_subscriptions = get_env(Zone, max_subscriptions, 0), + upgrade_qos = get_env(Zone, upgrade_qos, false), + inflight = emqx_inflight:new(MaxInflight), + mqueue = init_mqueue(Zone), + retry_interval = get_env(Zone, retry_interval, 0), + awaiting_rel = #{}, + await_rel_timeout = get_env(Zone, await_rel_timeout), + max_awaiting_rel = get_env(Zone, max_awaiting_rel), + expiry_interval = ExpiryInterval, + enable_stats = get_env(Zone, enable_stats, true), + deliver_stats = 0, + enqueue_stats = 0, + created_at = os:timestamp(), + topic_alias_maximum = TopicAliasMaximum }, emqx_sm:register_session(ClientId, attrs(State)), emqx_sm:set_session_stats(ClientId, stats(State)), @@ -546,8 +550,9 @@ handle_cast({resume, ConnPid}, State = #state{client_id = ClientId, handle_cast({expiry_interval, Interval}, State) -> {noreply, State#state{expiry_interval = Interval}}; -handle_cast({max_inflight, MaxInflight}, State) -> - {noreply, State#state{inflight = emqx_inflight:update_size(MaxInflight, State#state.inflight)}}; +handle_cast({update_misc, #{max_inflight := MaxInflight, topic_alias_maximum := TopicAliasMaximum}}, State) -> + {noreply, State#state{inflight = emqx_inflight:update_size(MaxInflight, State#state.inflight), + topic_alias_maximum = TopicAliasMaximum}}; handle_cast(Msg, State) -> emqx_logger:error("[Session] unexpected cast: ~p", [Msg]), @@ -560,15 +565,22 @@ handle_info({dispatch, Topic, Msgs}, State) when is_list(Msgs) -> end, State, Msgs)}; %% Dispatch message -handle_info({dispatch, Topic, Msg}, State = #state{subscriptions = SubMap}) when is_record(Msg, message) -> - noreply(case maps:find(Topic, SubMap) of - {ok, #{nl := Nl, qos := QoS, rap := Rap, subid := SubId}} -> - run_dispatch_steps([{nl, Nl}, {qos, QoS}, {rap, Rap}, {subid, SubId}], Msg, State); - {ok, #{nl := Nl, qos := QoS, rap := Rap}} -> - run_dispatch_steps([{nl, Nl}, {qos, QoS}, {rap, Rap}], Msg, State); - error -> - dispatch(emqx_message:unset_flag(dup, Msg), State) - end); +handle_info({dispatch, Topic, Msg = #message{headers = Headers}}, + State = #state{subscriptions = SubMap, topic_alias_maximum = TopicAliasMaximum}) when is_record(Msg, message) -> + TopicAlias = maps:get('Topic-Alias', Headers, undefined), + if + TopicAlias =:= undefined orelse TopicAlias =< TopicAliasMaximum -> + noreply(case maps:find(Topic, SubMap) of + {ok, #{nl := Nl, qos := QoS, rap := Rap, subid := SubId}} -> + run_dispatch_steps([{nl, Nl}, {qos, QoS}, {rap, Rap}, {subid, SubId}], Msg, State); + {ok, #{nl := Nl, qos := QoS, rap := Rap}} -> + run_dispatch_steps([{nl, Nl}, {qos, QoS}, {rap, Rap}], Msg, State); + error -> + dispatch(emqx_message:unset_flag(dup, Msg), State) + end); + true -> + noreply(State) + end; %% Do nothing if the client has been disconnected. handle_info({timeout, Timer, retry_delivery}, State = #state{conn_pid = undefined, retry_timer = Timer}) -> diff --git a/src/emqx_sm.erl b/src/emqx_sm.erl index 98046823f..1fa0b488b 100644 --- a/src/emqx_sm.erl +++ b/src/emqx_sm.erl @@ -56,11 +56,15 @@ open_session(SessAttrs = #{clean_start := true, client_id := ClientId, conn_pid end, emqx_sm_locker:trans(ClientId, CleanStart); -open_session(SessAttrs = #{clean_start := false, client_id := ClientId, conn_pid := ConnPid, max_inflight := MaxInflight}) -> +open_session(SessAttrs = #{clean_start := false, + client_id := ClientId, + conn_pid := ConnPid, + max_inflight := MaxInflight, + topic_alias_maximum := TopicAliasMaximum}) -> ResumeStart = fun(_) -> case resume_session(ClientId, ConnPid) of {ok, SPid} -> - emqx_session:update_max_inflight(SPid, MaxInflight), + emqx_session:update_misc(SPid, #{max_inflight => MaxInflight, topic_alias_maximum => TopicAliasMaximum}), {ok, SPid, true}; {error, not_found} -> emqx_session_sup:start_session(SessAttrs) diff --git a/test/emqx_mock_client.erl b/test/emqx_mock_client.erl index 0aa01458e..4a49a1fc5 100644 --- a/test/emqx_mock_client.erl +++ b/test/emqx_mock_client.erl @@ -52,7 +52,8 @@ handle_call({start_session, ClientPid, ClientId, Zone}, _From, State) -> clean_start => true, username => undefined, expiry_interval => 0, - max_inflight => 0 + max_inflight => 0, + topic_alias_maximum => 0 }, {ok, SessPid} = emqx_sm:open_session(Attrs), {reply, {ok, SessPid}, diff --git a/test/emqx_mqtt_caps_SUITE.erl b/test/emqx_mqtt_caps_SUITE.erl index d5751f9bb..919be5218 100644 --- a/test/emqx_mqtt_caps_SUITE.erl +++ b/test/emqx_mqtt_caps_SUITE.erl @@ -44,7 +44,8 @@ t_get_set_caps(_) -> end, PubCaps = #{ max_qos_allowed => ?QOS_2, - mqtt_retain_available => true + mqtt_retain_available => true, + max_topic_alias => 0 }, PubCaps = emqx_mqtt_caps:get_caps(zone, publish), NewPubCaps = PubCaps#{max_qos_allowed => ?QOS_1}, diff --git a/test/emqx_sm_SUITE.erl b/test/emqx_sm_SUITE.erl index 1f85d4724..110e13026 100644 --- a/test/emqx_sm_SUITE.erl +++ b/test/emqx_sm_SUITE.erl @@ -25,7 +25,7 @@ t_open_close_session(_) -> emqx_ct_broker_helpers:run_setup_steps(), {ok, ClientPid} = emqx_mock_client:start_link(<<"client">>), Attrs = #{clean_start => true, client_id => <<"client">>, conn_pid => ClientPid, - zone => internal, username => <<"zhou">>, expiry_interval => 0, max_inflight => 0}, + zone => internal, username => <<"zhou">>, expiry_interval => 0, max_inflight => 0, topic_alias_maximum => 0}, {ok, SPid} = emqx_sm:open_session(Attrs), [{<<"client">>, SPid}] = emqx_sm:lookup_session(<<"client">>), SPid = emqx_sm:lookup_session_pid(<<"client">>), From 59798762a937f215f8cfd1adbef565429c4010bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Sat, 29 Sep 2018 09:19:33 +0800 Subject: [PATCH 13/28] Remove will message when received disconnect packet with reason code 0x00 --- src/emqx_protocol.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index 88f07138d..1fb80c0ce 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -412,12 +412,14 @@ process_packet(?DISCONNECT_PACKET(?RC_SUCCESS, #{'Session-Expiry-Interval' := In case Interval =/= 0 andalso OldInterval =:= 0 of true -> deliver({disconnect, ?RC_PROTOCOL_ERROR}, PState), - {error, protocol_error, PState}; + {error, protocol_error, PState#pstate{will_msg = undefined}}; false -> emqx_session:update_expiry_interval(SPid, Interval), %% Clean willmsg {stop, normal, PState#pstate{will_msg = undefined}} end; +process_packet(?DISCONNECT_PACKET(?RC_SUCCESS), PState) -> + {stop, normal, PState#pstate{will_msg = undefined}}; process_packet(?DISCONNECT_PACKET(_), PState) -> {stop, normal, PState}. From a77f8d28f9b50fcc0f98a41a6ec765e2d8e25a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Sat, 29 Sep 2018 10:10:46 +0800 Subject: [PATCH 14/28] Improve coverage --- test/emqx_packet_SUITE.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/emqx_packet_SUITE.erl b/test/emqx_packet_SUITE.erl index bd1b0cb4a..ec4205957 100644 --- a/test/emqx_packet_SUITE.erl +++ b/test/emqx_packet_SUITE.erl @@ -46,7 +46,12 @@ packet_type_name(_) -> packet_validate(_) -> ?assertEqual(true, emqx_packet:validate(?SUBSCRIBE_PACKET(15, #{'Subscription-Identifier' => 1}, [{<<"topic">>, #{qos => ?QOS0}}]))), ?assertEqual(true, emqx_packet:validate(?UNSUBSCRIBE_PACKET(89, [<<"topic">>]))), - ?assertEqual(true, emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{}))). + ?assertEqual(true, emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{}))), + ?assertEqual(true, emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' => 1}}))), + case catch emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' => 0}})) of + {'EXIT', {protocol_error, _}} -> ?assertEqual(true, true); + true -> ?assertEqual(true, false) + end. packet_message(_) -> Pkt = #mqtt_packet{header = #mqtt_packet_header{type = ?PUBLISH, From 1638aff0f9c744a46e2b485bad2e77135e89befd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Sat, 29 Sep 2018 14:30:48 +0800 Subject: [PATCH 15/28] Remove duplicate match --- src/emqx_packet.erl | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/emqx_packet.erl b/src/emqx_packet.erl index c1bbbb6fd..1d1e29c6a 100644 --- a/src/emqx_packet.erl +++ b/src/emqx_packet.erl @@ -63,8 +63,6 @@ validate(?PUBLISH_PACKET(_QoS, Topic, _, Properties, _)) -> validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' := 0}})) -> error(protocol_error); -validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' := _}})) -> - true; validate(_Packet) -> true. From cb85c5fea019c3cead216b89de14f85c6ea186d1 Mon Sep 17 00:00:00 2001 From: HuangDan Date: Sat, 29 Sep 2018 18:58:25 +0800 Subject: [PATCH 16/28] Upgrade the esockd library to v5.4.2 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 549237e67..368aff6a9 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ dep_jsx = git https://github.com/talentdeficit/jsx 2.9.0 dep_gproc = git https://github.com/uwiger/gproc 0.8.0 dep_gen_rpc = git https://github.com/emqx/gen_rpc 2.2.0 dep_lager = git https://github.com/erlang-lager/lager 3.6.5 -dep_esockd = git https://github.com/emqx/esockd v5.4.1 +dep_esockd = git https://github.com/emqx/esockd v5.4.2 dep_ekka = git https://github.com/emqx/ekka v0.4.1 dep_cowboy = git https://github.com/ninenines/cowboy 2.4.0 dep_clique = git https://github.com/emqx/clique develop From 1705f25db638aa60c5a5da8c95a1cd8acee3cab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Sat, 29 Sep 2018 21:28:28 +0800 Subject: [PATCH 17/28] Fix bug in session expiry interval --- src/emqx_session.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/emqx_session.erl b/src/emqx_session.erl index 4514debcb..9a5689401 100644 --- a/src/emqx_session.erl +++ b/src/emqx_session.erl @@ -127,7 +127,7 @@ await_rel_timer :: reference() | undefined, %% Session Expiry Interval - expiry_interval = 7200000 :: timeout(), + expiry_interval = 7200 :: timeout(), %% Expired Timer expiry_timer :: reference() | undefined, @@ -318,7 +318,7 @@ discard(SPid, ByPid) -> -spec(update_expiry_interval(spid(), timeout()) -> ok). update_expiry_interval(SPid, Interval) -> - gen_server:cast(SPid, {expiry_interval, Interval * 1000}). + gen_server:cast(SPid, {expiry_interval, Interval}). update_misc(SPid, Misc) -> gen_server:cast(SPid, {update_misc, Misc}). From e3f2ae8db8753c6e21c3a32773e13ddc931321f2 Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sat, 22 Sep 2018 08:53:49 +0200 Subject: [PATCH 18/28] Change from customized total heap size check to set process flag The `max_heap_size` process flag can be used to limit total heap size of a process, and it gives much more detailed crash log if the limit is hit. --- priv/emqx.schema | 6 +++--- src/emqx_connection.erl | 2 +- src/emqx_misc.erl | 30 ++++++++++++++++-------------- src/emqx_session.erl | 2 +- test/emqx_misc_tests.erl | 16 ++++++---------- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/priv/emqx.schema b/priv/emqx.schema index 2450a6876..42b894401 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -835,7 +835,6 @@ end}. %% connection/session process. %% Message queue here is the Erlang process mailbox, but not the number %% of queued MQTT messages of QoS 1 and 2. -%% Total heap size is the in Erlang 'words' not in 'bytes'. %% Zero or negative is to disable. {mapping, "zone.$name.force_shutdown_policy", "emqx.zones", [ {default, "0 | 0MB"}, @@ -868,7 +867,8 @@ end}. {error, Reason} -> error(Reason); Bytes1 -> - #{bytes => Bytes1, count => list_to_integer(Count)} + #{bytes => Bytes1, + count => list_to_integer(Count)} end, {force_gc_policy, GcPolicy}; ("force_shutdown_policy", Val) -> @@ -878,7 +878,7 @@ end}. error(Reason); Siz1 -> #{message_queue_len => list_to_integer(Len), - total_heap_size => Siz1} + max_heap_size => Siz1} end, {force_shutdown_policy, ShutdownPolicy}; (Opt, Val) -> diff --git a/src/emqx_connection.erl b/src/emqx_connection.erl index 3176ad443..ccb5f59fa 100644 --- a/src/emqx_connection.erl +++ b/src/emqx_connection.erl @@ -152,7 +152,7 @@ init([Transport, RawSocket, Options]) -> }), GcPolicy = emqx_zone:get_env(Zone, force_gc_policy, false), ok = emqx_gc:init(GcPolicy), - erlang:put(force_shutdown_policy, emqx_zone:get_env(Zone, force_shutdown_policy)), + ok = emqx_misc:init_proc_mng_policy(Zone), gen_server:enter_loop(?MODULE, [{hibernate_after, IdleTimout}], State, self(), IdleTimout); {error, Reason} -> diff --git a/src/emqx_misc.erl b/src/emqx_misc.erl index 656b0fca3..03c42510c 100644 --- a/src/emqx_misc.erl +++ b/src/emqx_misc.erl @@ -15,7 +15,9 @@ -module(emqx_misc). -export([merge_opts/2, start_timer/2, start_timer/3, cancel_timer/1, - proc_name/2, proc_stats/0, proc_stats/1, conn_proc_mng_policy/1]). + proc_name/2, proc_stats/0, proc_stats/1]). + +-export([init_proc_mng_policy/1, conn_proc_mng_policy/1]). %% @doc Merge options -spec(merge_opts(list(), list()) -> list()). @@ -60,32 +62,35 @@ proc_stats(Pid) -> -define(DISABLED, 0). +init_proc_mng_policy(Zone) -> + #{max_heap_size := MaxHeapSizeInBytes} = ShutdownPolicy = + emqx_zone:get_env(Zone, force_shutdown_policy), + MaxHeapSize = MaxHeapSizeInBytes div erlang:system_info(wordsize), + _ = erlang:process_flag(max_heap_size, MaxHeapSize), % zero is discarded + erlang:put(force_shutdown_policy, ShutdownPolicy), + ok. + %% @doc Check self() process status against connection/session process management policy, %% return `continue | hibernate | {shutdown, Reason}' accordingly. %% `continue': There is nothing out of the ordinary. %% `hibernate': Nothing to process in my mailbox, and since this check is triggered %% by a timer, we assume it is a fat chance to continue idel, hence hibernate. -%% `shutdown': Some numbers (message queue length or heap size have hit the limit), +%% `shutdown': Some numbers (message queue length hit the limit), %% hence shutdown for greater good (system stability). --spec(conn_proc_mng_policy(#{message_queue_len := integer(), - total_heap_size := integer() - } | undefined) -> continue | hibernate | {shutdown, _}). -conn_proc_mng_policy(#{message_queue_len := MaxMsgQueueLen, - total_heap_size := MaxTotalHeapSize - }) -> +-spec(conn_proc_mng_policy(#{message_queue_len => integer()} | false) -> + continue | hibernate | {shutdown, _}). +conn_proc_mng_policy(#{message_queue_len := MaxMsgQueueLen}) -> Qlength = proc_info(message_queue_len), Checks = [{fun() -> is_message_queue_too_long(Qlength, MaxMsgQueueLen) end, {shutdown, message_queue_too_long}}, - {fun() -> is_heap_size_too_large(MaxTotalHeapSize) end, - {shutdown, total_heap_size_too_large}}, {fun() -> Qlength > 0 end, continue}, {fun() -> true end, hibernate} ], check(Checks); conn_proc_mng_policy(_) -> %% disable by default - conn_proc_mng_policy(#{message_queue_len => 0, total_heap_size => 0}). + conn_proc_mng_policy(#{message_queue_len => 0}). check([{Pred, Result} | Rest]) -> case Pred() of @@ -96,9 +101,6 @@ check([{Pred, Result} | Rest]) -> is_message_queue_too_long(Qlength, Max) -> is_enabled(Max) andalso Qlength > Max. -is_heap_size_too_large(Max) -> - is_enabled(Max) andalso proc_info(total_heap_size) > Max. - is_enabled(Max) -> is_integer(Max) andalso Max > ?DISABLED. proc_info(Key) -> diff --git a/src/emqx_session.erl b/src/emqx_session.erl index 9a5689401..d327723f5 100644 --- a/src/emqx_session.erl +++ b/src/emqx_session.erl @@ -369,7 +369,7 @@ init([Parent, #{zone := Zone, emqx_hooks:run('session.created', [#{client_id => ClientId}, info(State)]), GcPolicy = emqx_zone:get_env(Zone, force_gc_policy, false), ok = emqx_gc:init(GcPolicy), - erlang:put(force_shutdown_policy, emqx_zone:get_env(Zone, force_shutdown_policy)), + ok = emqx_misc:init_proc_mng_policy(Zone), ok = proc_lib:init_ack(Parent, {ok, self()}), gen_server:enter_loop(?MODULE, [{hibernate_after, IdleTimout}], State). diff --git a/test/emqx_misc_tests.erl b/test/emqx_misc_tests.erl index 40a9aae87..50513ee86 100644 --- a/test/emqx_misc_tests.erl +++ b/test/emqx_misc_tests.erl @@ -24,23 +24,19 @@ timer_cancel_flush_test() -> shutdown_disabled_test() -> self() ! foo, - ?assertEqual(continue, conn_proc_mng_policy(0, 0)), + ?assertEqual(continue, conn_proc_mng_policy(0)), receive foo -> ok end, - ?assertEqual(hibernate, conn_proc_mng_policy(0, 0)). + ?assertEqual(hibernate, conn_proc_mng_policy(0)). message_queue_too_long_test() -> self() ! foo, self() ! bar, ?assertEqual({shutdown, message_queue_too_long}, - conn_proc_mng_policy(1, 0)), + conn_proc_mng_policy(1)), receive foo -> ok end, - ?assertEqual(continue, conn_proc_mng_policy(1, 0)), + ?assertEqual(continue, conn_proc_mng_policy(1)), receive bar -> ok end. -total_heap_size_too_large_test() -> - ?assertEqual({shutdown, total_heap_size_too_large}, - conn_proc_mng_policy(0, 1)). +conn_proc_mng_policy(L) -> + emqx_misc:conn_proc_mng_policy(#{message_queue_len => L}). -conn_proc_mng_policy(L, S) -> - emqx_misc:conn_proc_mng_policy(#{message_queue_len => L, - total_heap_size => S}). From e5f977d808f3d9a77c4d564056ab2586352a630e Mon Sep 17 00:00:00 2001 From: HuangDan Date: Sat, 29 Sep 2018 22:16:54 +0800 Subject: [PATCH 19/28] Upgrade the esockd library --- rebar.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rebar.config b/rebar.config index f94258e6b..63b3a7595 100644 --- a/rebar.config +++ b/rebar.config @@ -10,7 +10,7 @@ [{gen_rpc, "2.2.0"}, {ekka, "v0.4.1"}, {clique, "develop"}, - {esockd, "v5.4.1"}, + {esockd, "v5.4.2"}, {cuttlefish, "emqx30"} ]}. From b25dbd866b6200e5bfedd3a431c978e4f1ab95f5 Mon Sep 17 00:00:00 2001 From: Feng Lee Date: Thu, 4 Oct 2018 12:17:28 +0800 Subject: [PATCH 20/28] Update README.md --- README.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b45ac510d..f8872e8a0 100644 --- a/README.md +++ b/README.md @@ -5,19 +5,18 @@ Starting from 3.0 release, *EMQ X* broker fully supports MQTT V5.0 protocol specifications and backward compatible with MQTT V3.1 and V3.1.1, as well as other communication protocols such as MQTT-SN, CoAP, LwM2M, WebSocket and STOMP. The 3.0 release of the *EMQ X* broker can scaled to 10+ million concurrent MQTT connections on one cluster. -- For full list of new features, please read *EMQ X* broker 3.0 [release notes](https://github.com/emqtt/emqttd/releases/). +- For full list of new features, please read *EMQ X* broker 3.0 [release notes](https://github.com/emqx/emqx/releases/). - For more information, please visit [EMQ X homepage](http://emqtt.io). - ## Installation The *EMQ X* broker is cross-platform, which can be deployed on Linux, Unix, Mac, Windows and even Raspberry Pi. Download the binary package for your platform from [here](http://emqtt.io/downloads). -- [Single Node Install](http://emqtt.io/docs/v2/install.html) -- [Multi Node Install](http://emqtt.io/docs/v2/cluster.html) +- [Single Node Install](http://emqtt.io/docs/v3/install.html) +- [Multi Node Install](http://emqtt.io/docs/v3/cluster.html) ## Build From Source @@ -49,7 +48,7 @@ cd _rel/emqx && ./bin/emqx console ## Roadmap -The [EMQX roadmap uses Github milestones](https://github.com/emqtt/emqttd/milestones) to track the progress of the project. +The [EMQ X Roadmap uses Github milestones](https://github.com/emqx/emqx/milestones) to track the progress of the project. ## Community, discussion, contribution, and support @@ -62,7 +61,7 @@ You can reach the EMQ community and developers via the following channels: - [Forum](https://groups.google.com/d/forum/emqtt) - [Blog](https://medium.com/@emqtt) -Please submit any bugs, issues, and feature requests to [emqtt/emqttd](//github.com/emqtt/emqttd/issues). +Please submit any bugs, issues, and feature requests to [emqx/emqx](https://github.com/emqx/emqx/issues). ## License From d36a34fb5988dbad7a205fcf6eac08ea919f539c Mon Sep 17 00:00:00 2001 From: Gilbert Wong Date: Tue, 9 Oct 2018 17:45:40 +0800 Subject: [PATCH 21/28] Fix topic_name validation bug Prior to this change, Prior to this change, the validation for the mqtt5.0 publish packet which both contains zero-length topic name and topic alias is wrong. --- src/emqx_packet.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/emqx_packet.erl b/src/emqx_packet.erl index 1d1e29c6a..1db586cea 100644 --- a/src/emqx_packet.erl +++ b/src/emqx_packet.erl @@ -55,6 +55,8 @@ validate(?UNSUBSCRIBE_PACKET(PacketId, TopicFilters)) -> validate_packet_id(PacketId) andalso ok == lists:foreach(fun emqx_topic:validate/1, TopicFilters); +validate(?PUBLISH_PACKET(_QoS, <<>>, _, #{'Topic-Alias':= _I}, _)) -> + true; validate(?PUBLISH_PACKET(_QoS, <<>>, _, _, _)) -> error(topic_name_invalid); validate(?PUBLISH_PACKET(_QoS, Topic, _, Properties, _)) -> From 4082f3ade2f83f4f3766b1d0e8b5ac18ef28b060 Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sat, 13 Oct 2018 11:41:19 +0200 Subject: [PATCH 22/28] Improve stats test Before this change, the stats callback provided by emqx_broker_helper was an anonymous function with module local context. This commit changes it to a full fun M:F/A style callback for: 1. More robust to hot beam reload 2. Faster/smaller variable to construct 3. Easier test --- Makefile | 2 +- src/emqx_broker_helper.erl | 21 +++++---- src/emqx_stats.erl | 29 +++++++++---- test/emqx_stats_SUITE.erl | 56 ------------------------ test/emqx_stats_tests.erl | 89 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 74 deletions(-) delete mode 100644 test/emqx_stats_SUITE.erl create mode 100644 test/emqx_stats_tests.erl diff --git a/Makefile b/Makefile index 368aff6a9..de4a2c8c0 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ EUNIT_OPTS = verbose CT_SUITES = emqx emqx_zone emqx_banned emqx_connection emqx_session emqx_access emqx_broker emqx_cm emqx_frame emqx_guid emqx_inflight \ emqx_json emqx_keepalive emqx_lib emqx_metrics emqx_misc emqx_mod emqx_mqtt_caps \ emqx_mqtt_compat emqx_mqtt_props emqx_mqueue emqx_net emqx_pqueue emqx_router emqx_sm \ - emqx_stats emqx_tables emqx_time emqx_topic emqx_trie emqx_vm \ + emqx_tables emqx_time emqx_topic emqx_trie emqx_vm \ emqx_mountpoint emqx_listeners emqx_protocol emqx_pool emqx_shared_sub CT_NODE_NAME = emqxct@127.0.0.1 diff --git a/src/emqx_broker_helper.erl b/src/emqx_broker_helper.erl index fecf98a7b..e597a233e 100644 --- a/src/emqx_broker_helper.erl +++ b/src/emqx_broker_helper.erl @@ -19,6 +19,9 @@ -export([start_link/0]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). +%% internal export +-export([stats_fun/0]). + -define(HELPER, ?MODULE). -record(state, {}). @@ -32,7 +35,9 @@ start_link() -> %%------------------------------------------------------------------------------ init([]) -> - emqx_stats:update_interval(broker_stats, stats_fun()), + %% Use M:F/A for callback, not anonymous function because + %% fun M:F/A is small, also no badfun risk during hot beam reload + emqx_stats:update_interval(broker_stats, fun ?MODULE:stats_fun/0), {ok, #state{}, hibernate}. handle_call(Req, _From, State) -> @@ -58,14 +63,12 @@ code_change(_OldVsn, State, _Extra) -> %%------------------------------------------------------------------------------ stats_fun() -> - fun() -> - safe_update_stats(emqx_subscriber, - 'subscribers/count', 'subscribers/max'), - safe_update_stats(emqx_subscription, - 'subscriptions/count', 'subscriptions/max'), - safe_update_stats(emqx_suboptions, - 'suboptions/count', 'suboptions/max') - end. + safe_update_stats(emqx_subscriber, + 'subscribers/count', 'subscribers/max'), + safe_update_stats(emqx_subscription, + 'subscriptions/count', 'subscriptions/max'), + safe_update_stats(emqx_suboptions, + 'suboptions/count', 'suboptions/max'). safe_update_stats(Tab, Stat, MaxStat) -> case ets:info(Tab, size) of diff --git a/src/emqx_stats.erl b/src/emqx_stats.erl index 510b2d91a..61ff6cbc3 100644 --- a/src/emqx_stats.erl +++ b/src/emqx_stats.erl @@ -18,7 +18,7 @@ -include("emqx.hrl"). --export([start_link/0]). +-export([start_link/0, start_link/1, stop/0]). %% Stats API. -export([getstats/0, getstat/1]). @@ -31,7 +31,8 @@ code_change/3]). -record(update, {name, countdown, interval, func}). --record(state, {timer, updates :: [#update{}]}). +-record(state, {timer, updates :: [#update{}], + tick_ms :: timeout()}). -type(stats() :: list({atom(), non_neg_integer()})). @@ -77,10 +78,20 @@ -define(TAB, ?MODULE). -define(SERVER, ?MODULE). +-type opts() :: #{tick_ms := timeout()}. + %% @doc Start stats server -spec(start_link() -> emqx_types:startlink_ret()). start_link() -> - gen_server:start_link({local, ?SERVER}, ?MODULE, [], []). + start_link(#{tick_ms => timer:seconds(1)}). + +-spec(start_link(opts()) -> emqx_types:startlink_ret()). +start_link(Opts) -> + gen_server:start_link({local, ?SERVER}, ?MODULE, Opts, []). + +-spec(stop() -> ok). +stop() -> + gen_server:call(?SERVER, stop, infinity). %% @doc Generate stats fun -spec(statsfun(Stat :: atom()) -> fun()). @@ -140,16 +151,18 @@ cast(Msg) -> %% gen_server callbacks %%------------------------------------------------------------------------------ -init([]) -> +init(#{tick_ms := TickMs}) -> _ = emqx_tables:new(?TAB, [set, public, {write_concurrency, true}]), Stats = lists:append([?CONNECTION_STATS, ?SESSION_STATS, ?PUBSUB_STATS, ?ROUTE_STATS, ?RETAINED_STATS]), true = ets:insert(?TAB, [{Name, 0} || Name <- Stats]), - {ok, start_timer(#state{updates = []}), hibernate}. + {ok, start_timer(#state{updates = [], tick_ms = TickMs}), hibernate}. -start_timer(State) -> - State#state{timer = emqx_misc:start_timer(timer:seconds(1), tick)}. +start_timer(#state{tick_ms = Ms} = State) -> + State#state{timer = emqx_misc:start_timer(Ms, tick)}. +handle_call(stop, _From, State) -> + {stop, normal, _Reply = ok, State}; handle_call(Req, _From, State) -> emqx_logger:error("[Stats] unexpected call: ~p", [Req]), {reply, ignored, State}. @@ -201,7 +214,7 @@ handle_info(Info, State) -> {noreply, State}. terminate(_Reason, #state{timer = TRef}) -> - timer:cancel(TRef). + emqx_misc:cancel_timer(TRef). code_change(_OldVsn, State, _Extra) -> {ok, State}. diff --git a/test/emqx_stats_SUITE.erl b/test/emqx_stats_SUITE.erl deleted file mode 100644 index 5c7254468..000000000 --- a/test/emqx_stats_SUITE.erl +++ /dev/null @@ -1,56 +0,0 @@ -%% Copyright (c) 2018 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_stats_SUITE). - --compile(export_all). --compile(nowarn_export_all). - --include_lib("common_test/include/ct.hrl"). - -all() -> [t_set_get_state, t_update_interval]. - -t_set_get_state(_) -> - emqx_stats:start_link(), - SetConnsCount = emqx_stats:statsfun('connections/count'), - SetConnsCount(1), - 1 = emqx_stats:getstat('connections/count'), - emqx_stats:setstat('connections/count', 2), - 2 = emqx_stats:getstat('connections/count'), - emqx_stats:setstat('connections/count', 'connections/max', 3), - timer:sleep(100), - 3 = emqx_stats:getstat('connections/count'), - 3 = emqx_stats:getstat('connections/max'), - emqx_stats:setstat('connections/count', 'connections/max', 2), - timer:sleep(100), - 2 = emqx_stats:getstat('connections/count'), - 3 = emqx_stats:getstat('connections/max'), - SetConns = emqx_stats:statsfun('connections/count', 'connections/max'), - SetConns(4), - timer:sleep(100), - 4 = emqx_stats:getstat('connections/count'), - 4 = emqx_stats:getstat('connections/max'), - Conns = emqx_stats:getstats(), - 4 = proplists:get_value('connections/count', Conns), - 4 = proplists:get_value('connections/max', Conns). - -t_update_interval(_) -> - emqx_stats:start_link(), - emqx_stats:cancel_update(cm_stats), - ok = emqx_stats:update_interval(stats_test, fun update_stats/0), - timer:sleep(2500), - 1 = emqx_stats:getstat('connections/count'). - -update_stats() -> - emqx_stats:setstat('connections/count', 1). diff --git a/test/emqx_stats_tests.erl b/test/emqx_stats_tests.erl new file mode 100644 index 000000000..5c3a9c803 --- /dev/null +++ b/test/emqx_stats_tests.erl @@ -0,0 +1,89 @@ +%% Copyright (c) 2018 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_stats_tests). + +-include_lib("eunit/include/eunit.hrl"). + +get_state_test() -> + with_proc(fun() -> + SetConnsCount = emqx_stats:statsfun('connections/count'), + SetConnsCount(1), + 1 = emqx_stats:getstat('connections/count'), + emqx_stats:setstat('connections/count', 2), + 2 = emqx_stats:getstat('connections/count'), + emqx_stats:setstat('connections/count', 'connections/max', 3), + timer:sleep(100), + 3 = emqx_stats:getstat('connections/count'), + 3 = emqx_stats:getstat('connections/max'), + emqx_stats:setstat('connections/count', 'connections/max', 2), + timer:sleep(100), + 2 = emqx_stats:getstat('connections/count'), + 3 = emqx_stats:getstat('connections/max'), + SetConns = emqx_stats:statsfun('connections/count', 'connections/max'), + SetConns(4), + timer:sleep(100), + 4 = emqx_stats:getstat('connections/count'), + 4 = emqx_stats:getstat('connections/max'), + Conns = emqx_stats:getstats(), + 4 = proplists:get_value('connections/count', Conns), + 4 = proplists:get_value('connections/max', Conns) + end). + +update_interval_test() -> + TickMs = 100, + with_proc(fun() -> + SleepMs = TickMs * 2 + TickMs div 2, %% sleep for 2.5 ticks + emqx_stats:cancel_update(cm_stats), + UpdFun = fun() -> emqx_stats:setstat('connections/count', 1) end, + ok = emqx_stats:update_interval(stats_test, UpdFun), + timer:sleep(SleepMs), + ?assertEqual(1, emqx_stats:getstat('connections/count')) + end, TickMs). + +emqx_broker_helpe_test() -> + TickMs = 100, + with_proc(fun() -> + SleepMs = TickMs + TickMs div 2, %% sleep for 1.5 ticks + Ref = make_ref(), + Tester = self(), + UpdFun = + fun() -> + emqx_broker_helper:stats_fun(), + Tester ! Ref, + ok + end, + ok = emqx_stats:update_interval(stats_test, UpdFun), + timer:sleep(SleepMs), + receive Ref -> ok after 2000 -> error(timeout) end + end, TickMs). + +with_proc(F) -> + {ok, _Pid} = emqx_stats:start_link(), + with_stop(F). + +with_proc(F, TickMs) -> + {ok, _Pid} = emqx_stats:start_link(#{tick_ms => TickMs}), + with_stop(F). + +with_stop(F) -> + try + %% make a synced call to the gen_server so we know it has + %% started running, hence it is safe to continue with less risk of race condition + ?assertEqual(ignored, gen_server:call(emqx_stats, ignored)), + F() + after + ok = emqx_stats:stop() + end. + From 7e7d99fbadbf299180c78255888628357f4a024d Mon Sep 17 00:00:00 2001 From: spring2maz Date: Sat, 13 Oct 2018 12:14:29 +0200 Subject: [PATCH 23/28] Change more stats callbacks to full M:F/A Including emqx_sm emqx_cm emqx_router_helper --- src/emqx_cm.erl | 5 ++++- src/emqx_router_helper.erl | 17 +++++++------- src/emqx_sm.erl | 11 ++++----- test/emqx_stats_tests.erl | 46 ++++++++++++++++++++++++-------------- 4 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/emqx_cm.erl b/src/emqx_cm.erl index 3e9958939..19892b386 100644 --- a/src/emqx_cm.erl +++ b/src/emqx_cm.erl @@ -30,6 +30,9 @@ -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). +%% internal export +-export([update_conn_stats/0]). + -define(CM, ?MODULE). %% ETS Tables. @@ -125,7 +128,7 @@ init([]) -> _ = emqx_tables:new(?CONN_TAB, [{read_concurrency, true} | TabOpts]), _ = emqx_tables:new(?CONN_ATTRS_TAB, TabOpts), _ = emqx_tables:new(?CONN_STATS_TAB, TabOpts), - ok = emqx_stats:update_interval(cm_stats, fun update_conn_stats/0), + ok = emqx_stats:update_interval(cm_stats, fun ?MODULE:update_conn_stats/0), {ok, #{conn_pmon => emqx_pmon:new()}}. handle_call(Req, _From, State) -> diff --git a/src/emqx_router_helper.erl b/src/emqx_router_helper.erl index d0d0e8075..5431c9900 100644 --- a/src/emqx_router_helper.erl +++ b/src/emqx_router_helper.erl @@ -31,6 +31,9 @@ -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). +%% internal export +-export([stats_fun/0]). + -record(routing_node, {name, const = unused}). -record(state, {nodes = []}). @@ -90,7 +93,7 @@ init([]) -> [Node | Acc] end end, [], mnesia:dirty_all_keys(?ROUTING_NODE)), - emqx_stats:update_interval(route_stats, stats_fun()), + emqx_stats:update_interval(route_stats, fun ?MODULE:stats_fun/0), {ok, #state{nodes = Nodes}, hibernate}. handle_call(Req, _From, State) -> @@ -143,13 +146,11 @@ code_change(_OldVsn, State, _Extra) -> %%------------------------------------------------------------------------------ stats_fun() -> - fun() -> - case ets:info(?ROUTE, size) of - undefined -> ok; - Size -> - emqx_stats:setstat('routes/count', 'routes/max', Size), - emqx_stats:setstat('topics/count', 'topics/max', Size) - end + case ets:info(?ROUTE, size) of + undefined -> ok; + Size -> + emqx_stats:setstat('routes/count', 'routes/max', Size), + emqx_stats:setstat('topics/count', 'topics/max', Size) end. cleanup_routes(Node) -> diff --git a/src/emqx_sm.erl b/src/emqx_sm.erl index 1fa0b488b..d45548a78 100644 --- a/src/emqx_sm.erl +++ b/src/emqx_sm.erl @@ -31,6 +31,9 @@ %% Internal functions for rpc -export([dispatch/3]). +%% Internal function for stats +-export([stats_fun/0]). + %% gen_server callbacks -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). @@ -210,7 +213,7 @@ init([]) -> _ = emqx_tables:new(?SESSION_P_TAB, TabOpts), _ = emqx_tables:new(?SESSION_ATTRS_TAB, TabOpts), _ = emqx_tables:new(?SESSION_STATS_TAB, TabOpts), - emqx_stats:update_interval(sm_stats, stats_fun()), + emqx_stats:update_interval(sm_stats, fun ?MODULE:stats_fun/0), {ok, #{session_pmon => emqx_pmon:new()}}. handle_call(Req, _From, State) -> @@ -251,10 +254,8 @@ code_change(_OldVsn, State, _Extra) -> %%------------------------------------------------------------------------------ stats_fun() -> - fun() -> - safe_update_stats(?SESSION_TAB, 'sessions/count', 'sessions/max'), - safe_update_stats(?SESSION_P_TAB, 'sessions/persistent/count', 'sessions/persistent/max') - end. + safe_update_stats(?SESSION_TAB, 'sessions/count', 'sessions/max'), + safe_update_stats(?SESSION_P_TAB, 'sessions/persistent/count', 'sessions/persistent/max'). safe_update_stats(Tab, Stat, MaxStat) -> case ets:info(Tab, size) of diff --git a/test/emqx_stats_tests.erl b/test/emqx_stats_tests.erl index 5c3a9c803..dd9733a88 100644 --- a/test/emqx_stats_tests.erl +++ b/test/emqx_stats_tests.erl @@ -42,7 +42,7 @@ get_state_test() -> end). update_interval_test() -> - TickMs = 100, + TickMs = 200, with_proc(fun() -> SleepMs = TickMs * 2 + TickMs div 2, %% sleep for 2.5 ticks emqx_stats:cancel_update(cm_stats), @@ -52,22 +52,34 @@ update_interval_test() -> ?assertEqual(1, emqx_stats:getstat('connections/count')) end, TickMs). -emqx_broker_helpe_test() -> - TickMs = 100, - with_proc(fun() -> - SleepMs = TickMs + TickMs div 2, %% sleep for 1.5 ticks - Ref = make_ref(), - Tester = self(), - UpdFun = - fun() -> - emqx_broker_helper:stats_fun(), - Tester ! Ref, - ok - end, - ok = emqx_stats:update_interval(stats_test, UpdFun), - timer:sleep(SleepMs), - receive Ref -> ok after 2000 -> error(timeout) end - end, TickMs). +helper_test_() -> + TickMs = 200, + TestF = + fun(CbModule, CbFun) -> + SleepMs = TickMs + TickMs div 2, %% sleep for 1.5 ticks + Ref = make_ref(), + Tester = self(), + UpdFun = + fun() -> + CbModule:CbFun(), + Tester ! Ref, + ok + end, + ok = emqx_stats:update_interval(stats_test, UpdFun), + timer:sleep(SleepMs), + receive Ref -> ok after 2000 -> error(timeout) end + end, + MkTestFun = + fun(CbModule, CbFun) -> + fun() -> + with_proc(fun() -> TestF(CbModule, CbFun) end, TickMs) + end + end, + [{"emqx_broker_helper", MkTestFun(emqx_broker_helper, stats_fun)}, + {"emqx_sm", MkTestFun(emqx_sm, stats_fun)}, + {"emqx_router_helper", MkTestFun(emqx_router_helper, stats_fun)}, + {"emqx_cm", MkTestFun(emqx_cm, update_conn_stats)} + ]. with_proc(F) -> {ok, _Pid} = emqx_stats:start_link(), From 0adee194aa4c0b97563dfd15a9fb2aed179a23c1 Mon Sep 17 00:00:00 2001 From: Gilbert Wong Date: Wed, 17 Oct 2018 10:29:34 +0800 Subject: [PATCH 24/28] Fix the refered link Prior to this change, the refered wiki link in acl.conf has been deprecated. This change replace the deprecated link with the doc link in official site. --- etc/acl.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/acl.conf b/etc/acl.conf index fb85f3f20..b60ea5e7a 100644 --- a/etc/acl.conf +++ b/etc/acl.conf @@ -1,6 +1,6 @@ %%-------------------------------------------------------------------- %% -%% [ACL](https://github.com/emqtt/emqttd/wiki/ACL) +%% [ACL](http://emqtt.io/docs/v2/config.html#allow-anonymous-and-acl-file) %% %% -type who() :: all | binary() | %% {ipaddr, esockd_access:cidr()} | From 30d986c31825076cf588f0ecb49d4e27c1453b4d Mon Sep 17 00:00:00 2001 From: Gilbert Wong Date: Sat, 29 Sep 2018 19:17:58 +0800 Subject: [PATCH 25/28] Add warning log for unauthored subscribe Prior to this change, there is no log for unauthored log, it is difficult to find the problem when subscription error occured. --- src/emqx_protocol.erl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index 1fb80c0ce..861fc68a8 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -747,7 +747,11 @@ check_sub_acl(TopicFilters, PState) -> fun({Topic, SubOpts}, {Ok, Acc}) -> case emqx_access_control:check_acl(Credentials, subscribe, Topic) of allow -> {Ok, [{Topic, SubOpts}|Acc]}; - deny -> {error, [{Topic, SubOpts#{rc := ?RC_NOT_AUTHORIZED}}|Acc]} + deny -> + emqx_logger:warning([{client, PState#pstate.client_id}], + "ACL(~s) Cannot SUBSCRIBE ~p for ACL Deny", + [PState#pstate.client_id, Topic]), + {error, [{Topic, SubOpts#{rc := ?RC_NOT_AUTHORIZED}}|Acc]} end end, {ok, []}, TopicFilters). From 4c40f75f4b6957b7d5747aec2ffe5e64bf02334f Mon Sep 17 00:00:00 2001 From: Gilbert Date: Fri, 19 Oct 2018 01:21:05 +0800 Subject: [PATCH 26/28] Request & Response (broker and client) (#1819) Add request & response support for CONNECT & CONNACK Prior to this change, there is no validate and specified process for Request-Response-Information and Response-Information Also added basic Request/Response functionality to emqx_client implementation --- Makefile | 12 +- etc/emqx.conf | 6 + include/emqx.hrl | 3 - include/emqx_mqtt.hrl | 11 +- priv/emqx.schema | 6 +- src/emqx_broker.erl | 15 +- src/emqx_client.erl | 336 +++++++++++++++--- src/emqx_packet.erl | 21 +- src/emqx_protocol.erl | 120 ++++--- src/emqx_topic.erl | 6 +- test/emqx_SUITE.erl | 21 +- ...compat_SUITE.erl => emqx_client_SUITE.erl} | 117 ++++-- test/emqx_packet_SUITE.erl | 67 +++- test/emqx_protocol_SUITE.erl | 279 ++++++++++----- test/emqx_topic_SUITE.erl | 19 +- test/rfc6455_client.erl | 2 +- test/ws_client.erl | 2 - 17 files changed, 771 insertions(+), 272 deletions(-) rename test/{emqx_mqtt_compat_SUITE.erl => emqx_client_SUITE.erl} (60%) diff --git a/Makefile b/Makefile index de4a2c8c0..9a114399c 100644 --- a/Makefile +++ b/Makefile @@ -35,11 +35,12 @@ EUNIT_OPTS = verbose # CT_SUITES = emqx_frame ## emqx_trie emqx_router emqx_frame emqx_mqtt_compat -CT_SUITES = emqx emqx_zone emqx_banned emqx_connection emqx_session emqx_access emqx_broker emqx_cm emqx_frame emqx_guid emqx_inflight \ - emqx_json emqx_keepalive emqx_lib emqx_metrics emqx_misc emqx_mod emqx_mqtt_caps \ - emqx_mqtt_compat emqx_mqtt_props emqx_mqueue emqx_net emqx_pqueue emqx_router emqx_sm \ - emqx_tables emqx_time emqx_topic emqx_trie emqx_vm \ - emqx_mountpoint emqx_listeners emqx_protocol emqx_pool emqx_shared_sub +CT_SUITES = emqx emqx_client emqx_zone emqx_banned emqx_connection emqx_session \ + emqx_access emqx_broker emqx_cm emqx_frame emqx_guid emqx_inflight emqx_json \ + emqx_keepalive emqx_lib emqx_metrics emqx_misc emqx_mod emqx_mqtt_caps \ + emqx_mqtt_props emqx_mqueue emqx_net emqx_pqueue emqx_router emqx_sm \ + emqx_tables emqx_time emqx_topic emqx_trie emqx_vm emqx_mountpoint \ + emqx_listeners emqx_protocol emqx_pool emqx_shared_sub CT_NODE_NAME = emqxct@127.0.0.1 CT_OPTS = -cover test/ct.cover.spec -erl_args -name $(CT_NODE_NAME) @@ -138,4 +139,3 @@ dep-vsn-check: {[], []} -> halt(0); \ {Rebar, Mk} -> erlang:error({deps_version_discrepancy, [{rebar, Rebar}, {mk, Mk}]}) \ end." - diff --git a/etc/emqx.conf b/etc/emqx.conf index 18a7d6fd6..874a4c560 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -459,6 +459,12 @@ acl_cache_ttl = 1m ## MQTT Protocol ##-------------------------------------------------------------------- +## Response Topic Prefix +## +## Value: String +## Default: emqxrspv1 +mqtt.response_topic_prefix = emqxrspv1 + ## Maximum MQTT packet size allowed. ## ## Value: Bytes diff --git a/include/emqx.hrl b/include/emqx.hrl index bca6fe519..984f4c9e2 100644 --- a/include/emqx.hrl +++ b/include/emqx.hrl @@ -37,9 +37,6 @@ %% Queue topic -define(QUEUE, <<"$queue/">>). -%% Shared topic --define(SHARE, <<"$share/">>). - %%-------------------------------------------------------------------- %% Message and Delivery %%-------------------------------------------------------------------- diff --git a/include/emqx_mqtt.hrl b/include/emqx_mqtt.hrl index 74bfc1120..b9bf6b55e 100644 --- a/include/emqx_mqtt.hrl +++ b/include/emqx_mqtt.hrl @@ -65,9 +65,9 @@ end). -define(IS_QOS_NAME(I), - (I =:= qos0; I =:= at_most_once; - I =:= qos1; I =:= at_least_once; - I =:= qos2; I =:= exactly_once)). + (I =:= qos0 orelse I =:= at_most_once orelse + I =:= qos1 orelse I =:= at_least_once orelse + I =:= qos2 orelse I =:= exactly_once)). %%-------------------------------------------------------------------- %% Maximum ClientId Length. @@ -527,5 +527,8 @@ -define(PACKET(Type), #mqtt_packet{header = #mqtt_packet_header{type = Type}}). --endif. +-define(SHARE, "$share"). +-define(SHARE(Group, Topic), emqx_topic:join([<>, Group, Topic])). +-define(IS_SHARE(Topic), case Topic of <> -> true; _ -> false end). +-endif. diff --git a/priv/emqx.schema b/priv/emqx.schema index 42b894401..becb4bff4 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -597,6 +597,11 @@ end}. %% MQTT Protocol %%-------------------------------------------------------------------- +%% @doc Response Topic Prefix +{mapping, "mqtt.response_topic_prefix", "emqx.response_topic_prefix",[ + {datatype, string} +]}. + %% @doc Max Packet Size Allowed, 1MB by default. {mapping, "mqtt.max_packet_size", "emqx.max_packet_size", [ {default, "1MB"}, @@ -1797,4 +1802,3 @@ end}. {busy_port, cuttlefish:conf_get("sysmon.busy_port", Conf)}, {busy_dist_port, cuttlefish:conf_get("sysmon.busy_dist_port", Conf)}] end}. - diff --git a/src/emqx_broker.erl b/src/emqx_broker.erl index 35e8276d2..f0946e92d 100644 --- a/src/emqx_broker.erl +++ b/src/emqx_broker.erl @@ -317,13 +317,6 @@ handle_call(Req, _From, State) -> emqx_logger:error("[Broker] unexpected call: ~p", [Req]), {reply, ignored, State}. -resubscribe(From, {Subscriber, SubOpts, Topic}, State) -> - {SubPid, _} = Subscriber, - Group = maps:get(share, SubOpts, undefined), - true = do_subscribe(Group, Topic, Subscriber, SubOpts), - emqx_shared_sub:subscribe(Group, Topic, SubPid), - emqx_router:add_route(From, Topic, dest(Group)), - {noreply, monitor_subscriber(Subscriber, State)}. handle_cast({From, #subscribe{topic = Topic, subpid = SubPid, subid = SubId, subopts = SubOpts}}, State) -> @@ -385,6 +378,14 @@ code_change(_OldVsn, State, _Extra) -> %% Internal functions %%------------------------------------------------------------------------------ +resubscribe(From, {Subscriber, SubOpts, Topic}, State) -> + {SubPid, _} = Subscriber, + Group = maps:get(share, SubOpts, undefined), + true = do_subscribe(Group, Topic, Subscriber, SubOpts), + emqx_shared_sub:subscribe(Group, Topic, SubPid), + emqx_router:add_route(From, Topic, dest(Group)), + {noreply, monitor_subscriber(Subscriber, State)}. + insert_subscriber(Group, Topic, Subscriber) -> Subscribers = subscribers(Topic), case lists:member(Subscriber, Subscribers) of diff --git a/src/emqx_client.erl b/src/emqx_client.erl index 7ef9c5968..22c37d26f 100644 --- a/src/emqx_client.erl +++ b/src/emqx_client.erl @@ -19,7 +19,8 @@ -include("emqx_mqtt.hrl"). -export([start_link/0, start_link/1]). - +-export([request/5, request/6, request_async/7, receive_response/3]). +-export([set_request_handler/2, sub_request_topic/3, sub_request_topic/4]). -export([subscribe/2, subscribe/3, subscribe/4]). -export([publish/2, publish/3, publish/4, publish/5]). -export([unsubscribe/2, unsubscribe/3]). @@ -37,8 +38,34 @@ -export([initialized/3, waiting_for_connack/3, connected/3]). -export([init/1, callback_mode/0, handle_event/4, terminate/3, code_change/4]). +-export_type([client/0, properties/0, payload/0, + pubopt/0, subopt/0, request_input/0, + response_payload/0, request_handler/0, + corr_data/0]). +-export_type([host/0, option/0]). + +%% Default timeout +-define(DEFAULT_KEEPALIVE, 60000). +-define(DEFAULT_ACK_TIMEOUT, 30000). +-define(DEFAULT_CONNECT_TIMEOUT, 60000). + +-define(PROPERTY(Name, Val), #state{properties = #{Name := Val}}). + +-define(WILL_MSG(QoS, Retain, Topic, Props, Payload), + #mqtt_msg{qos = QoS, retain = Retain, topic = Topic, props = Props, payload = Payload}). + +-define(RESPONSE_TIMEOUT_SECONDS, timer:seconds(5)). + +-define(NO_HANDLER, undefined). + +-define(NO_GROUP, <<>>). + +-define(NO_CLIENT_ID, <<>>). + -type(host() :: inet:ip_address() | inet:hostname()). +-type corr_data() :: binary(). + -type(option() :: {name, atom()} | {owner, pid()} | {host, host()} @@ -57,6 +84,7 @@ | {keepalive, non_neg_integer()} | {max_inflight, pos_integer()} | {retry_interval, timeout()} + | {request_handler, request_handler()} | {will_topic, iodata()} | {will_payload, iodata()} | {will_retain, boolean()} @@ -67,8 +95,6 @@ | {force_ping, boolean()} | {properties, properties()}). --export_type([host/0, option/0]). - -record(mqtt_msg, {qos = ?QOS0, retain = false, dup = false, packet_id, topic, props, payload}). @@ -106,6 +132,7 @@ ack_timer :: reference(), retry_interval :: pos_integer(), retry_timer :: reference(), + request_handler :: request_handler(), session_present :: boolean(), last_packet_id :: packet_id(), parse_state :: emqx_frame:state()}). @@ -124,7 +151,7 @@ -type(qos() :: emqx_mqtt_types:qos_name() | emqx_mqtt_types:qos()). --type(pubopt() :: {retain, boolean()} | {qos, qos()}). +-type(pubopt() :: {retain, boolean()} | {qos, qos()} | {timeout, timeout()}). -type(subopt() :: {rh, 0 | 1 | 2} | {rap, boolean()} @@ -135,23 +162,35 @@ -type(subscribe_ret() :: {ok, properties(), [reason_code()]} | {error, term()}). --export_type([client/0, topic/0, qos/0, properties/0, payload/0, - packet_id/0, pubopt/0, subopt/0, reason_code/0]). +-type(request_input() :: binary()). -%% Default timeout --define(DEFAULT_KEEPALIVE, 60000). --define(DEFAULT_ACK_TIMEOUT, 30000). --define(DEFAULT_CONNECT_TIMEOUT, 60000). +-type(response_payload() :: binary()). --define(PROPERTY(Name, Val), #state{properties = #{Name := Val}}). +-type(request_handler() :: fun((request_input()) -> response_payload())). --define(WILL_MSG(QoS, Retain, Topic, Props, Payload), - #mqtt_msg{qos = QoS, retain = Retain, topic = Topic, props = Props, payload = Payload}). +-type(group() :: binary()). %%------------------------------------------------------------------------------ %% API %%------------------------------------------------------------------------------ +%% @doc Swap in a new request handler on the fly. +-spec(set_request_handler(client(), request_handler()) -> ok). +set_request_handler(Responser, RequestHandler) -> + gen_statem:call(Responser, {set_request_handler, RequestHandler}). + +%% @doc Subscribe to request topic. +-spec(sub_request_topic(client(), qos(), topic()) -> ok). +sub_request_topic(Client, QoS, Topic) -> + sub_request_topic(Client, QoS, Topic, ?NO_GROUP). + +%% @doc Share-subscribe to request topic. +-spec(sub_request_topic(client(), qos(), topic(), group()) -> ok). +sub_request_topic(Client, QoS, Topic, Group) -> + Properties = get_properties(Client), + NewTopic = make_req_rsp_topic(Properties, Topic, Group), + subscribe_req_rsp_topic(Client, QoS, NewTopic). + -spec(start_link() -> gen_statem:start_ret()). start_link() -> start_link([]). @@ -248,12 +287,82 @@ parse_subopt([{nl, false} | Opts], Result) -> parse_subopt([{qos, QoS} | Opts], Result) -> parse_subopt(Opts, Result#{qos := ?QOS_I(QoS)}). +-spec(request(client(), topic(), topic(), payload(), qos() | [pubopt()]) + -> ok | {ok, packet_id()} | {error, term()}). +request(Client, ResponseTopic, RequestTopic, Payload, QoS) when is_binary(ResponseTopic), is_atom(QoS) -> + request(Client, ResponseTopic, RequestTopic, Payload, [{qos, ?QOS_I(QoS)}]); +request(Client, ResponseTopic, RequestTopic, Payload, QoS) when is_binary(ResponseTopic), ?IS_QOS(QoS) -> + request(Client, ResponseTopic, RequestTopic, Payload, [{qos, QoS}]); +request(Client, ResponseTopic, RequestTopic, Payload, Opts) when is_binary(ResponseTopic), is_list(Opts) -> + request(Client, ResponseTopic, RequestTopic, Payload, Opts, _Properties = #{}). + +%% @doc Send a request to request topic and wait for response. +-spec(request(client(), topic(), topic(), payload(), [pubopt()], properties()) + -> {ok, response_payload()} | {error, term()}). +request(Client, ResponseTopic, RequestTopic, Payload, Opts, Properties) -> + CorrData = make_corr_data(), + case request_async(Client, ResponseTopic, RequestTopic, + Payload, Opts, Properties, CorrData) of + ok -> receive_response(Client, CorrData, Opts); + {error, Reason} -> {error, Reason} + end. + +%% @doc Get client properties. +-spec(get_properties(client()) -> properties()). +get_properties(Client) -> gen_statem:call(Client, get_properties, infinity). + +%% @doc Send a request, but do not wait for response. +%% The caller should expect a `{publish, Response}' message, +%% or call `receive_response/3' to receive the message. +-spec(request_async(client(), topic(), topic(), payload(), + [pubopt()], properties(), corr_data()) -> ok | {error, any()}). +request_async(Client, ResponseTopic, RequestTopic, Payload, Opts, Properties, CorrData) + when is_binary(ResponseTopic), + is_binary(RequestTopic), + is_map(Properties), + is_list(Opts) -> + ok = emqx_mqtt_props:validate(Properties), + Retain = proplists:get_bool(retain, Opts), + QoS = ?QOS_I(proplists:get_value(qos, Opts, ?QOS_0)), + ClientProperties = get_properties(Client), + NewResponseTopic = make_req_rsp_topic(ClientProperties, ResponseTopic), + NewRequestTopic = make_req_rsp_topic(ClientProperties, RequestTopic), + %% This is perhaps not optimal to subscribe the response topic for + %% each and every request even though the response topic is always the same + ok = sub_response_topic(Client, QoS, NewResponseTopic), + NewProperties = maps:merge(Properties, #{'Response-Topic' => NewResponseTopic, + 'Correlation-Data' => CorrData}), + case publish(Client, #mqtt_msg{qos = QoS, + retain = Retain, + topic = NewRequestTopic, + props = NewProperties, + payload = iolist_to_binary(Payload)}) of + ok -> ok; + {ok, _PacketId} -> ok; %% assume auto_ack + {error, Reason} -> {error, Reason} + end. + +%% @doc Block wait the response for a request sent earlier. +-spec(receive_response(client(), corr_data(), [pubopt()]) + -> {ok, response_payload()} | {error, any()}). +receive_response(Client, CorrData, Opts) -> + TimeOut = proplists:get_value(timeout, Opts, ?RESPONSE_TIMEOUT_SECONDS), + MRef = erlang:monitor(process, Client), + TRef = erlang:start_timer(TimeOut, self(), response), + try + receive_response(Client, CorrData, TRef, MRef) + after + erlang:cancel_timer(TRef), + receive {timeout, TRef, _} -> ok after 0 -> ok end, + erlang:demonitor(MRef, [flush]) + end. + -spec(publish(client(), topic(), payload()) -> ok | {error, term()}). publish(Client, Topic, Payload) when is_binary(Topic) -> publish(Client, #mqtt_msg{topic = Topic, qos = ?QOS_0, payload = iolist_to_binary(Payload)}). -spec(publish(client(), topic(), payload(), qos() | [pubopt()]) - -> ok | {ok, packet_id()} | {error, term()}). + -> ok | {ok, packet_id()} | {error, term()}). publish(Client, Topic, Payload, QoS) when is_binary(Topic), is_atom(QoS) -> publish(Client, Topic, Payload, [{qos, ?QOS_I(QoS)}]); publish(Client, Topic, Payload, QoS) when is_binary(Topic), ?IS_QOS(QoS) -> @@ -369,7 +478,7 @@ init([Options]) -> process_flag(trap_exit, true), ClientId = case {proplists:get_value(proto_ver, Options, v4), proplists:get_value(client_id, Options)} of - {v5, undefined} -> <<>>; + {v5, undefined} -> ?NO_CLIENT_ID; {_ver, undefined} -> random_client_id(); {_ver, Id} -> iolist_to_binary(Id) end, @@ -396,6 +505,7 @@ init([Options]) -> auto_ack = true, ack_timeout = ?DEFAULT_ACK_TIMEOUT, retry_interval = 0, + request_handler = ?NO_HANDLER, connect_timeout = ?DEFAULT_CONNECT_TIMEOUT, last_packet_id = 1}), {ok, initialized, init_parse_state(State)}. @@ -488,6 +598,8 @@ init([{auto_ack, AutoAck} | Opts], State) when is_boolean(AutoAck) -> init(Opts, State#state{auto_ack = AutoAck}); init([{retry_interval, I} | Opts], State) -> init(Opts, State#state{retry_interval = timer:seconds(I)}); +init([{request_handler, Handler} | Opts], State) -> + init(Opts, State#state{request_handler = Handler}); init([{bridge_mode, Mode} | Opts], State) when is_boolean(Mode) -> init(Opts, State#state{bridge_mode = Mode}); init([_Opt | Opts], State) -> @@ -562,7 +674,8 @@ mqtt_connect(State = #state{client_id = ClientId, waiting_for_connack(cast, ?CONNACK_PACKET(?RC_SUCCESS, SessPresent, Properties), - State = #state{properties = AllProps}) -> + State = #state{properties = AllProps, + client_id = ClientId}) -> case take_call(connect, State) of {value, #call{from = From}, State1} -> AllProps1 = case Properties of @@ -570,7 +683,8 @@ waiting_for_connack(cast, ?CONNACK_PACKET(?RC_SUCCESS, _ -> maps:merge(AllProps, Properties) end, Reply = {ok, self(), Properties}, - State2 = State1#state{properties = AllProps1, + State2 = State1#state{client_id = assign_id(ClientId, AllProps1), + properties = AllProps1, session_present = SessPresent}, {next_state, connected, ensure_keepalive_timer(State2), [{reply, From, Reply}]}; @@ -616,6 +730,15 @@ connected({call, From}, resume, State) -> connected({call, From}, stop, _State) -> {stop_and_reply, normal, [{reply, From, ok}]}; +connected({call, From}, get_properties, State = #state{properties = Properties}) -> + {keep_state, State, [{reply, From, Properties}]}; + +connected({call, From}, client_id, State = #state{client_id = ClientId}) -> + {keep_state, State, [{reply, From, ClientId}]}; + +connected({call, From}, {set_request_handler, RequestHandler}, State) -> + {keep_state, State#state{request_handler = RequestHandler}, [{reply, From, ok}]}; + connected({call, From}, SubReq = {subscribe, Properties, Topics}, State = #state{last_packet_id = PacketId, subscriptions = Subscriptions}) -> case send(?SUBSCRIBE_PACKET(PacketId, Properties, Topics), State) of @@ -695,29 +818,30 @@ connected(cast, {pubrel, PacketId, ReasonCode, Properties}, State) -> connected(cast, {pubcomp, PacketId, ReasonCode, Properties}, State) -> send_puback(?PUBCOMP_PACKET(PacketId, ReasonCode, Properties), State); -connected(cast, Packet = ?PUBLISH_PACKET(?QOS_0, _PacketId), State) -> - {keep_state, deliver(packet_to_msg(Packet), State)}; - connected(cast, ?PUBLISH_PACKET(_QoS, _PacketId), State = #state{paused = true}) -> {keep_state, State}; -connected(cast, Packet = ?PUBLISH_PACKET(?QOS_1, PacketId), - State = #state{auto_ack = AutoAck}) -> +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_0, _Topic, _PacketId, Properties, Payload), + State) when Properties =/= undefined -> + NewState = response_publish(Properties, State, ?QOS_0, Payload), + {keep_state, deliver(packet_to_msg(Packet), NewState)}; +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_0, _PacketId), State) -> + {keep_state, deliver(packet_to_msg(Packet), State)}; - _ = deliver(packet_to_msg(Packet), State), - case AutoAck of - true -> send_puback(?PUBACK_PACKET(PacketId), State); - false -> {keep_state, State} - end; +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_1, _Topic, _PacketId, Properties, Payload), State) + when Properties =/= undefined -> + NewState = response_publish(Properties, State, ?QOS_1, Payload), + publish_process(?QOS_1, Packet, NewState); -connected(cast, Packet = ?PUBLISH_PACKET(?QOS_2, PacketId), - State = #state{awaiting_rel = AwaitingRel}) -> - case send_puback(?PUBREC_PACKET(PacketId), State) of - {keep_state, NewState} -> - AwaitingRel1 = maps:put(PacketId, Packet, AwaitingRel), - {keep_state, NewState#state{awaiting_rel = AwaitingRel1}}; - Stop -> Stop - end; +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_1, _PacketId), State) -> + publish_process(?QOS_1, Packet, State); + +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_2, _Topic, _PacketId, Properties, Payload), State) + when Properties =/= undefined -> + NewState = response_publish(Properties, State, ?QOS_2, Payload), + publish_process(?QOS_2, Packet, NewState); +connected(cast, Packet = ?PUBLISH_PACKET(?QOS_2, _PacketId), State) -> + publish_process(?QOS_2, Packet, State); connected(cast, ?PUBACK_PACKET(PacketId, ReasonCode, Properties), State = #state{owner = Owner, inflight = Inflight}) -> @@ -899,6 +1023,132 @@ code_change(_Vsn, State, Data, _Extra) -> %% Internal functions %%------------------------------------------------------------------------------ +%% Subscribe to response topic. +-spec(sub_response_topic(client(), qos(), topic()) -> ok). +sub_response_topic(Client, QoS, Topic) when is_binary(Topic) -> + subscribe_req_rsp_topic(Client, QoS, Topic). + +receive_response(Client, CorrData, TRef, MRef) -> + receive + {publish, Response} -> + {ok, Properties} = maps:find(properties, Response), + case maps:find('Correlation-Data', Properties) of + {ok, CorrData} -> + maps:find(payload, Response); + _ -> + emqx_logger:debug("Discarded stale response: ~p", [Response]), + receive_response(Client, CorrData, TRef, MRef) + end; + {timeout, TRef, response} -> + {error, timeout}; + {'DOWN', MRef, process, _, _} -> + {error, client_down} + end. + +%% Make a unique correlation data for each request. +%% It has to be unique because stale responses should be discarded. +make_corr_data() -> term_to_binary(make_ref()). + +%% Shared function for request and response topic subscription. +subscribe_req_rsp_topic(Client, QoS, Topic) -> + %% It is a Protocol Error to set the No Local bit to 1 on a Shared Subscription + {ok, _Props, _QoS} = subscribe(Client, [{Topic, [{rh, 2}, {rap, false}, + {nl, not ?IS_SHARE(Topic)}, + {qos, QoS}]}]), + emqx_logger:debug("Subscribed to topic ~s", [Topic]), + ok. + +%% Make a request or response topic. +make_req_rsp_topic(Properties, Topic) -> + make_req_rsp_topic(Properties, Topic, ?NO_GROUP). + +%% Same as make_req_rsp_topic/2, but allow shared subscription (for request topics) +make_req_rsp_topic(Properties, Topic, Group) -> + case maps:find('Response-Information', Properties) of + {ok, ResponseInformation} when ResponseInformation =/= <<>> -> + emqx_topic:join([req_rsp_topic_prefix(Group, ResponseInformation), Topic]); + _ -> + erlang:error(no_response_information) + end. + +req_rsp_topic_prefix(?NO_GROUP, Prefix) -> Prefix; +req_rsp_topic_prefix(Group, Prefix) -> ?SHARE(Group, Prefix). + +assign_id(?NO_CLIENT_ID, Props) -> + case maps:find('Assigned-Client-Identifier', Props) of + {ok, Value} -> + Value; + _ -> + error(bad_client_id) + end; +assign_id(Id, _Props) -> + Id. + +publish_process(?QOS_1, Packet = ?PUBLISH_PACKET(?QOS_1, PacketId), State = #state{auto_ack = AutoAck}) -> + _ = deliver(packet_to_msg(Packet), State), + case AutoAck of + true -> send_puback(?PUBACK_PACKET(PacketId), State); + false -> {keep_state, State} + end; +publish_process(?QOS_2, Packet = ?PUBLISH_PACKET(?QOS_2, PacketId), + State = #state{awaiting_rel = AwaitingRel}) -> + case send_puback(?PUBREC_PACKET(PacketId), State) of + {keep_state, NewState} -> + AwaitingRel1 = maps:put(PacketId, Packet, AwaitingRel), + {keep_state, NewState#state{awaiting_rel = AwaitingRel1}}; + Stop -> Stop + end. + +response_publish(undefined, State, _QoS, _Payload) -> + State; +response_publish(Properties, State = #state{request_handler = RequestHandler}, QoS, Payload) -> + case maps:find('Response-Topic', Properties) of + {ok, ResponseTopic} -> + case RequestHandler of + ?NO_HANDLER -> State; + _ -> do_publish(ResponseTopic, Properties, State, QoS, Payload) + end; + _ -> + State + end. + +do_publish(ResponseTopic, Properties, State = #state{request_handler = RequestHandler}, ?QOS_0, Payload) -> + Msg = #mqtt_msg{qos = ?QOS_0, + retain = false, + topic = ResponseTopic, + props = Properties, + payload = RequestHandler(Payload) + }, + case send(Msg, State) of + {ok, NewState} -> NewState; + _Error -> State + end; +do_publish(ResponseTopic, Properties, State = #state{request_handler = RequestHandler, + inflight = Inflight, + last_packet_id = PacketId}, + QoS, Payload) + when (QoS =:= ?QOS_1); (QoS =:= ?QOS_2)-> + case emqx_inflight:is_full(Inflight) of + true -> + emqx_logger:error("Inflight is full"), + State; + false -> + Msg = #mqtt_msg{packet_id = PacketId, + qos = QoS, + retain = false, + topic = ResponseTopic, + props = Properties, + payload = RequestHandler(Payload)}, + case send(Msg, State) of + {ok, NewState} -> + Inflight1 = emqx_inflight:insert(PacketId, {publish, Msg, os:timestamp()}, Inflight), + ensure_retry_timer(NewState#state{inflight = Inflight1}); + {error, Reason} -> + emqx_logger:error("Send failed: ~p", [Reason]), + State + end + end. + ensure_keepalive_timer(State = ?PROPERTY('Server-Keep-Alive', Secs)) -> ensure_keepalive_timer(timer:seconds(Secs), State); ensure_keepalive_timer(State = #state{keepalive = 0}) -> @@ -986,10 +1236,15 @@ retry_send(pubrel, PacketId, Now, State = #state{inflight = Inflight}) -> deliver(#mqtt_msg{qos = QoS, dup = Dup, retain = Retain, packet_id = PacketId, topic = Topic, props = Props, payload = Payload}, - State = #state{owner = Owner}) -> - Owner ! {publish, #{qos => QoS, dup => Dup, retain => Retain, packet_id => PacketId, - topic => Topic, properties => Props, payload => Payload, - client_pid => self()}}, + State = #state{owner = Owner, request_handler = RequestHandler}) -> + case RequestHandler of + ?NO_HANDLER -> + Owner ! {publish, #{qos => QoS, dup => Dup, retain => Retain, packet_id => PacketId, + topic => Topic, properties => Props, payload => Payload, + client_pid => self()}}; + _ -> + ok + end, State. packet_to_msg(#mqtt_packet{header = #mqtt_packet_header{type = ?PUBLISH, @@ -1001,7 +1256,7 @@ packet_to_msg(#mqtt_packet{header = #mqtt_packet_header{type = ?PUBLISH, properties = Props}, payload = Payload}) -> #mqtt_msg{qos = QoS, retain = R, dup = Dup, packet_id = PacketId, - topic = Topic, props = Props, payload = Payload}. + topic = Topic, props = Props, payload = Payload}. msg_to_packet(#mqtt_msg{qos = QoS, dup = Dup, retain = Retain, packet_id = PacketId, topic = Topic, props = Props, payload = Payload}) -> @@ -1070,7 +1325,6 @@ receive_loop(Bytes, State = #state{parse_state = ParseState}) -> {error, Reason} -> {stop, Reason}; {'EXIT', Error} -> - io:format("client stop"), {stop, Error} end. diff --git a/src/emqx_packet.erl b/src/emqx_packet.erl index 1db586cea..2040e595e 100644 --- a/src/emqx_packet.erl +++ b/src/emqx_packet.erl @@ -63,8 +63,8 @@ validate(?PUBLISH_PACKET(_QoS, Topic, _, Properties, _)) -> ((not emqx_topic:wildcard(Topic)) orelse error(topic_name_invalid)) andalso validate_properties(?PUBLISH, Properties); -validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' := 0}})) -> - error(protocol_error); +validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = Properties})) -> + validate_properties(?CONNECT, Properties); validate(_Packet) -> true. @@ -82,11 +82,24 @@ validate_properties(?PUBLISH, #{'Topic-Alias':= I}) error(topic_alias_invalid); validate_properties(?PUBLISH, #{'Subscription-Identifier' := _I}) -> error(protocol_error); +validate_properties(?PUBLISH, #{'Response-Topic' := ResponseTopic}) -> + case emqx_topic:wildcard(ResponseTopic) of + true -> + error(protocol_error); + false -> + true + end; +validate_properties(?CONNECT, #{'Receive-Maximum' := 0}) -> + error(protocol_error); +validate_properties(?CONNECT, #{'Request-Response-Information' := ReqRespInfo}) + when ReqRespInfo =/= 0, ReqRespInfo =/= 1 -> + error(protocol_error); +validate_properties(?CONNECT, #{'Request-Problem-Information' := ReqProInfo}) + when ReqProInfo =/= 0, ReqProInfo =/= 1 -> + error(protocol_error); validate_properties(_, _) -> true. - - validate_subscription({Topic, #{qos := QoS}}) -> emqx_topic:validate(filter, Topic) andalso validate_qos(QoS). diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index 861fc68a8..c3f0689fa 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -82,27 +82,27 @@ -spec(init(map(), list()) -> state()). init(#{peername := Peername, peercert := Peercert, sendfun := SendFun}, Options) -> Zone = proplists:get_value(zone, Options), - #pstate{zone = Zone, - sendfun = SendFun, - peername = Peername, - peercert = Peercert, - proto_ver = ?MQTT_PROTO_V4, - proto_name = <<"MQTT">>, - client_id = <<>>, - is_assigned = false, - conn_pid = self(), - username = init_username(Peercert, Options), - is_super = false, - clean_start = false, - topic_aliases = #{}, - packet_size = emqx_zone:get_env(Zone, max_packet_size), - mountpoint = emqx_zone:get_env(Zone, mountpoint), - is_bridge = false, - enable_ban = emqx_zone:get_env(Zone, enable_ban, false), - enable_acl = emqx_zone:get_env(Zone, enable_acl), - recv_stats = #{msg => 0, pkt => 0}, - send_stats = #{msg => 0, pkt => 0}, - connected = false}. + #pstate{zone = Zone, + sendfun = SendFun, + peername = Peername, + peercert = Peercert, + proto_ver = ?MQTT_PROTO_V4, + proto_name = <<"MQTT">>, + client_id = <<>>, + is_assigned = false, + conn_pid = self(), + username = init_username(Peercert, Options), + is_super = false, + clean_start = false, + topic_aliases = #{}, + packet_size = emqx_zone:get_env(Zone, max_packet_size), + mountpoint = emqx_zone:get_env(Zone, mountpoint), + is_bridge = false, + enable_ban = emqx_zone:get_env(Zone, enable_ban, false), + enable_acl = emqx_zone:get_env(Zone, enable_acl), + recv_stats = #{msg => 0, pkt => 0}, + send_stats = #{msg => 0, pkt => 0}, + connected = false}. init_username(Peercert, Options) -> case proplists:get_value(peer_cert_as_username, Options) of @@ -194,6 +194,13 @@ parser(#pstate{packet_size = Size, proto_ver = Ver}) -> %% Packet Received %%------------------------------------------------------------------------------ +set_protover(?CONNECT_PACKET(#mqtt_packet_connect{ + proto_ver = ProtoVer}), + PState) -> + PState#pstate{ proto_ver = ProtoVer }; +set_protover(_Packet, PState) -> + PState. + -spec(received(emqx_mqtt_types:packet(), state()) -> {ok, state()} | {error, term()} | {error, term(), state()} | {stop, term(), state()}). received(?PACKET(Type), PState = #pstate{connected = false}) when Type =/= ?CONNECT -> @@ -203,14 +210,31 @@ received(?PACKET(?CONNECT), PState = #pstate{connected = true}) -> {error, proto_unexpected_connect, PState}; received(Packet = ?PACKET(Type), PState) -> - trace(recv, Packet, PState), - case catch emqx_packet:validate(Packet) of + PState1 = set_protover(Packet, PState), + trace(recv, Packet, PState1), + try emqx_packet:validate(Packet) of true -> - {Packet1, PState1} = preprocess_properties(Packet, PState), - process_packet(Packet1, inc_stats(recv, Type, PState1)); - {'EXIT', {Reason, _Stacktrace}} -> - deliver({disconnect, rc(Reason)}, PState), - {error, Reason, PState} + {Packet1, PState2} = preprocess_properties(Packet, PState1), + process_packet(Packet1, inc_stats(recv, Type, PState2)) + catch + error : protocol_error -> + deliver({disconnect, ?RC_PROTOCOL_ERROR}, PState1), + {error, protocol_error, PState}; + error : subscription_identifier_invalid -> + deliver({disconnect, ?RC_SUBSCRIPTION_IDENTIFIERS_NOT_SUPPORTED}, PState1), + {error, subscription_identifier_invalid, PState1}; + error : topic_alias_invalid -> + deliver({disconnect, ?RC_TOPIC_ALIAS_INVALID}, PState1), + {error, topic_alias_invalid, PState1}; + error : topic_filters_invalid -> + deliver({disconnect, ?RC_TOPIC_FILTER_INVALID}, PState1), + {error, topic_filters_invalid, PState1}; + error : topic_name_invalid -> + deliver({disconnect, ?RC_TOPIC_FILTER_INVALID}, PState1), + {error, topic_filters_invalid, PState1}; + error : Reason -> + deliver({disconnect, ?RC_MALFORMED_PACKET}, PState1), + {error, Reason, PState1} end. %%------------------------------------------------------------------------------ @@ -299,7 +323,7 @@ process_packet(?CONNECT_PACKET( {error, Error} -> ?LOG(error, "Failed to open session: ~p", [Error], PState1), {?RC_UNSPECIFIED_ERROR, PState1} - end; + end; {error, Reason} -> ?LOG(error, "Username '~s' login failed for ~p", [Username, Reason], PState2), {?RC_NOT_AUTHORIZED, PState1} @@ -407,13 +431,13 @@ process_packet(?UNSUBSCRIBE_PACKET(PacketId, Properties, RawTopicFilters), process_packet(?PACKET(?PINGREQ), PState) -> send(?PACKET(?PINGRESP), PState); -process_packet(?DISCONNECT_PACKET(?RC_SUCCESS, #{'Session-Expiry-Interval' := Interval}), +process_packet(?DISCONNECT_PACKET(?RC_SUCCESS, #{'Session-Expiry-Interval' := Interval}), PState = #pstate{session = SPid, conn_props = #{'Session-Expiry-Interval' := OldInterval}}) -> case Interval =/= 0 andalso OldInterval =:= 0 of - true -> + true -> deliver({disconnect, ?RC_PROTOCOL_ERROR}, PState), {error, protocol_error, PState#pstate{will_msg = undefined}}; - false -> + false -> emqx_session:update_expiry_interval(SPid, Interval), %% Clean willmsg {stop, normal, PState#pstate{will_msg = undefined}} @@ -481,7 +505,14 @@ deliver({connack, ReasonCode}, PState) -> deliver({connack, ?RC_SUCCESS, SP}, PState = #pstate{zone = Zone, proto_ver = ?MQTT_PROTO_V5, client_id = ClientId, + conn_props = ConnProps, is_assigned = IsAssigned}) -> + ResponseInformation = case maps:find('Request-Response-Information', ConnProps) of + {ok, 1} -> + iolist_to_binary(emqx_config:get_env(response_topic_prefix)); + _ -> + <<>> + end, #{max_packet_size := MaxPktSize, max_qos_allowed := MaxQoS, mqtt_retain_available := Retain, @@ -493,18 +524,21 @@ deliver({connack, ?RC_SUCCESS, SP}, PState = #pstate{zone = Zone, 'Topic-Alias-Maximum' => MaxAlias, 'Wildcard-Subscription-Available' => flag(Wildcard), 'Subscription-Identifier-Available' => 1, + 'Response-Information' => ResponseInformation, + 'Shared-Subscription-Available' => flag(Shared)}, - Props1 = if - MaxQoS =:= ?QOS_2 -> + Props1 = if + MaxQoS =:= ?QOS_2 -> Props; true -> maps:put('Maximum-QoS', MaxQoS, Props) end, - + Props2 = if IsAssigned -> Props1#{'Assigned-Client-Identifier' => ClientId}; true -> Props1 + end, Props3 = case emqx_zone:get_env(Zone, server_keepalive) of @@ -601,17 +635,17 @@ set_session_attrs({max_inflight, #pstate{zone = Zone, proto_ver = ProtoVer, conn maps:put(max_inflight, if ProtoVer =:= ?MQTT_PROTO_V5 -> maps:get('Receive-Maximum', ConnProps, 65535); - true -> + true -> emqx_zone:get_env(Zone, max_inflight, 65535) end, SessAttrs); set_session_attrs({expiry_interval, #pstate{zone = Zone, proto_ver = ProtoVer, conn_props = ConnProps, clean_start = CleanStart}}, SessAttrs) -> maps:put(expiry_interval, if ProtoVer =:= ?MQTT_PROTO_V5 -> maps:get('Session-Expiry-Interval', ConnProps, 0); - true -> + true -> case CleanStart of true -> 0; - false -> + false -> emqx_zone:get_env(Zone, session_expiry_interval, 16#ffffffff) end end, SessAttrs); @@ -619,7 +653,7 @@ set_session_attrs({topic_alias_maximum, #pstate{zone = Zone, proto_ver = ProtoVe maps:put(topic_alias_maximum, if ProtoVer =:= ?MQTT_PROTO_V5 -> maps:get('Topic-Alias-Maximum', ConnProps, 0); - true -> + true -> emqx_zone:get_env(Zone, max_topic_alias, 0) end, SessAttrs); set_session_attrs({_, #pstate{}}, SessAttrs) -> @@ -803,14 +837,6 @@ start_keepalive(Secs, #pstate{zone = Zone}) when Secs > 0 -> Backoff = emqx_zone:get_env(Zone, keepalive_backoff, 0.75), self() ! {keepalive, start, round(Secs * Backoff)}. -rc(Reason) -> - case Reason of - protocol_error -> ?RC_PROTOCOL_ERROR; - topic_filters_invalid -> ?RC_TOPIC_FILTER_INVALID; - topic_name_invalid -> ?RC_TOPIC_NAME_INVALID; - _ -> ?RC_MALFORMED_PACKET - end. - %%----------------------------------------------------------------------------- %% Parse topic filters %%----------------------------------------------------------------------------- diff --git a/src/emqx_topic.erl b/src/emqx_topic.erl index 6540bbef2..f7e229a13 100644 --- a/src/emqx_topic.erl +++ b/src/emqx_topic.erl @@ -34,6 +34,8 @@ -define(MAX_TOPIC_LEN, 4096). +-include("emqx_mqtt.hrl"). + %% @doc Is wildcard topic? -spec(wildcard(topic() | words()) -> true | false). wildcard(Topic) when is_binary(Topic) -> @@ -180,11 +182,11 @@ parse(Topic) when is_binary(Topic) -> parse(Topic = <<"$queue/", _/binary>>, #{share := _Group}) -> error({invalid_topic, Topic}); -parse(Topic = <<"$share/", _/binary>>, #{share := _Group}) -> +parse(Topic = <>, #{share := _Group}) -> error({invalid_topic, Topic}); parse(<<"$queue/", Topic1/binary>>, Options) -> parse(Topic1, maps:put(share, <<"$queue">>, Options)); -parse(Topic = <<"$share/", Topic1/binary>>, Options) -> +parse(Topic = <>, Options) -> case binary:split(Topic1, <<"/">>) of [<<>>] -> error({invalid_topic, Topic}); [_] -> error({invalid_topic, Topic}); diff --git a/test/emqx_SUITE.erl b/test/emqx_SUITE.erl index 166d64a30..e82f70395 100644 --- a/test/emqx_SUITE.erl +++ b/test/emqx_SUITE.erl @@ -27,7 +27,6 @@ -record(ssl_socket, {tcp, ssl}). --type(socket() :: inet:socket() | #ssl_socket{}). -define(CLIENT, ?CONNECT_PACKET(#mqtt_packet_connect{ client_id = <<"mqtt_client">>, @@ -104,7 +103,7 @@ mqtt_connect_with_tcp(_) -> %% Issue #599 %% Empty clientId and clean_session = false {ok, Sock} = emqx_client_sock:connect({127,0,0,1}, 1883, [binary, {packet, raw}, {active, false}], 3000), - Packet = raw_send_serialise(?CLIENT2), + Packet = raw_send_serialize(?CLIENT2), emqx_client_sock:send(Sock, Packet), {ok, Data} = gen_tcp:recv(Sock, 0), {ok, ?CONNACK_PACKET(?CONNACK_INVALID_ID), _} = raw_recv_pase(Data), @@ -117,7 +116,7 @@ mqtt_connect_with_ssl_oneway(_) -> ClientSsl = emqx_ct_broker_helpers:client_ssl(), {ok, #ssl_socket{tcp = _Sock1, ssl = SslSock} = Sock} = emqx_client_sock:connect("127.0.0.1", 8883, [{ssl_opts, ClientSsl}], 3000), - Packet = raw_send_serialise(?CLIENT), + Packet = raw_send_serialize(?CLIENT), emqx_client_sock:setopts(Sock, [{active, once}]), emqx_client_sock:send(Sock, Packet), ?assert( @@ -135,7 +134,7 @@ mqtt_connect_with_ssl_twoway(_Config) -> ClientSsl = emqx_ct_broker_helpers:client_ssl_twoway(), {ok, #ssl_socket{tcp = _Sock1, ssl = SslSock} = Sock} = emqx_client_sock:connect("127.0.0.1", 8883, [{ssl_opts, ClientSsl}], 3000), - Packet = raw_send_serialise(?CLIENT), + Packet = raw_send_serialize(?CLIENT), emqx_client_sock:setopts(Sock, [{active, once}]), emqx_client_sock:send(Sock, Packet), timer:sleep(500), @@ -145,6 +144,7 @@ mqtt_connect_with_ssl_twoway(_Config) -> after 1000 -> false end), + ssl:close(SslSock), emqx_client_sock:close(Sock). mqtt_connect_with_ws(_Config) -> @@ -152,19 +152,19 @@ mqtt_connect_with_ws(_Config) -> {ok, _} = rfc6455_client:open(WS), %% Connect Packet - Packet = raw_send_serialise(?CLIENT), + Packet = raw_send_serialize(?CLIENT), ok = rfc6455_client:send_binary(WS, Packet), {binary, CONACK} = rfc6455_client:recv(WS), {ok, ?CONNACK_PACKET(?CONNACK_ACCEPT), _} = raw_recv_pase(CONACK), %% Sub Packet - SubPacket = raw_send_serialise(?SUBPACKET), + SubPacket = raw_send_serialize(?SUBPACKET), rfc6455_client:send_binary(WS, SubPacket), {binary, SubAck} = rfc6455_client:recv(WS), {ok, ?SUBACK_PACKET(?PACKETID, ?SUBCODE), _} = raw_recv_pase(SubAck), %% Pub Packet QoS 1 - PubPacket = raw_send_serialise(?PUBPACKET), + PubPacket = raw_send_serialize(?PUBPACKET), rfc6455_client:send_binary(WS, PubPacket), {binary, PubAck} = rfc6455_client:recv(WS), {ok, ?PUBACK_PACKET(?PACKETID), _} = raw_recv_pase(PubAck), @@ -174,22 +174,21 @@ mqtt_connect_with_ws(_Config) -> %%issue 1811 packet_size(_Config) -> {ok, Sock} = emqx_client_sock:connect({127,0,0,1}, 1883, [binary, {packet, raw}, {active, false}], 3000), - Packet = raw_send_serialise(?CLIENT), + Packet = raw_send_serialize(?CLIENT), emqx_client_sock:send(Sock, Packet), {ok, Data} = gen_tcp:recv(Sock, 0), {ok, ?CONNACK_PACKET(?CONNACK_ACCEPT), _} = raw_recv_pase(Data), %% Pub Packet QoS 1 - PubPacket = raw_send_serialise(?BIG_PUBPACKET), + PubPacket = raw_send_serialize(?BIG_PUBPACKET), emqx_client_sock:send(Sock, PubPacket), {ok, Data1} = gen_tcp:recv(Sock, 0), {ok, ?PUBACK_PACKET(?PACKETID), _} = raw_recv_pase(Data1), emqx_client_sock:close(Sock). -raw_send_serialise(Packet) -> +raw_send_serialize(Packet) -> emqx_frame:serialize(Packet). raw_recv_pase(P) -> emqx_frame:parse(P, {none, #{max_packet_size => ?MAX_PACKET_SIZE, version => ?MQTT_PROTO_V4} }). - diff --git a/test/emqx_mqtt_compat_SUITE.erl b/test/emqx_client_SUITE.erl similarity index 60% rename from test/emqx_mqtt_compat_SUITE.erl rename to test/emqx_client_SUITE.erl index af2583678..2ad3dbaf7 100644 --- a/test/emqx_mqtt_compat_SUITE.erl +++ b/test/emqx_client_SUITE.erl @@ -12,7 +12,7 @@ %% See the License for the specific language governing permissions and %% limitations under the License. --module(emqx_mqtt_compat_SUITE). +-module(emqx_client_SUITE). -compile(export_all). -compile(nowarn_export_all). @@ -32,15 +32,24 @@ <<"+/+">>, <<"TopicA/#">>]). all() -> - [basic_test, - will_message_test, - zero_length_clientid_test, - offline_message_queueing_test, - overlapping_subscriptions_test, - %% keepalive_test, - redelivery_on_reconnect_test, - %% subscribe_failure_test, - dollar_topics_test]. + [ {group, mqttv4}, + {group, mqttv5} + ]. + +groups() -> + [{mqttv4, [non_parallel_tests], + [basic_test, + will_message_test, + offline_message_queueing_test, + overlapping_subscriptions_test, + %% keepalive_test, + redelivery_on_reconnect_test, + %% subscribe_failure_test, + dollar_topics_test]}, + {mqttv5, [non_parallel_tests], + [request_response, + share_sub_request_topic]} +]. init_per_suite(Config) -> emqx_ct_broker_helpers:run_setup_steps(), @@ -49,6 +58,77 @@ init_per_suite(Config) -> end_per_suite(_Config) -> emqx_ct_broker_helpers:run_teardown_steps(). +request_response_exception(QoS) -> + {ok, Client, _} = emqx_client:start_link([{proto_ver, v5}, + {properties, #{ 'Request-Response-Information' => 0 }}]), + ?assertError(no_response_information, + emqx_client:sub_request_topic(Client, QoS, <<"request_topic">>)), + ok = emqx_client:disconnect(Client). + +request_response_per_qos(QoS) -> + {ok, Requester, _} = emqx_client:start_link([{proto_ver, v5}, + {client_id, <<"requester">>}, + {properties, #{ 'Request-Response-Information' => 1}}]), + {ok, Responser, _} = emqx_client:start_link([{proto_ver, v5}, + {client_id, <<"responser">>}, + {properties, #{ 'Request-Response-Information' => 1}}, + {request_handler, fun(_Req) -> <<"ResponseTest">> end}]), + ok = emqx_client:sub_request_topic(Responser, QoS, <<"request_topic">>), + {ok, <<"ResponseTest">>} = emqx_client:request(Requester, <<"response_topic">>, <<"request_topic">>, <<"request_payload">>, QoS), + ok = emqx_client:set_request_handler(Responser, fun(<<"request_payload">>) -> + <<"ResponseFunctionTest">>; + (_) -> + <<"404">> + end), + {ok, <<"ResponseFunctionTest">>} = emqx_client:request(Requester, <<"response_topic">>, <<"request_topic">>, <<"request_payload">>, QoS), + {ok, <<"404">>} = emqx_client:request(Requester, <<"response_topic">>, <<"request_topic">>, <<"invalid_request">>, QoS), + ok = emqx_client:disconnect(Responser), + ok = emqx_client:disconnect(Requester). + +request_response(_Config) -> + request_response_per_qos(?QOS_2), + request_response_per_qos(?QOS_1), + request_response_per_qos(?QOS_0), + request_response_exception(?QOS_0), + request_response_exception(?QOS_1), + request_response_exception(?QOS_2). + +share_sub_request_topic(_Config) -> + share_sub_request_topic_per_qos(?QOS_2), + share_sub_request_topic_per_qos(?QOS_1), + share_sub_request_topic_per_qos(?QOS_0). + +share_sub_request_topic_per_qos(QoS) -> + application:set_env(?APPLICATION, shared_subscription_strategy, random), + ReqTopic = <<"request-topic">>, + RspTopic = <<"response-topic">>, + Group = <<"g1">>, + Properties = #{ 'Request-Response-Information' => 1}, + Opts = fun(ClientId) -> [{proto_ver, v5}, + {client_id, atom_to_binary(ClientId, utf8)}, + {properties, Properties} + ] end, + {ok, Requester, _} = emqx_client:start_link(Opts(requester)), + {ok, Responser1, _} = emqx_client:start_link([{request_handler, fun(Req) -> <<"1-", Req/binary>> end} | Opts(requester1)]), + {ok, Responser2, _} = emqx_client:start_link([{request_handler, fun(Req) -> <<"2-", Req/binary>> end} | Opts(requester2)]), + ok = emqx_client:sub_request_topic(Responser1, QoS, ReqTopic, Group), + ok = emqx_client:sub_request_topic(Responser2, QoS, ReqTopic, Group), + %% Send a request, wait for response, validate response then return responser ID + ReqFun = fun(Req) -> + {ok, Rsp} = emqx_client:request(Requester, RspTopic, ReqTopic, Req, QoS), + case Rsp of + <<"1-", Req/binary>> -> 1; + <<"2-", Req/binary>> -> 2 + end + end, + Ids = lists:map(fun(I) -> ReqFun(integer_to_binary(I)) end, lists:seq(1, 100)), + %% we are testing with random shared-dispatch strategy, + %% fail if not all responsers got a chance to handle requests + ?assertEqual([1, 2], lists:usort(Ids)), + ok = emqx_client:disconnect(Responser1), + ok = emqx_client:disconnect(Responser2), + ok = emqx_client:disconnect(Requester). + receive_messages(Count) -> receive_messages(Count, []). @@ -59,7 +139,7 @@ receive_messages(Count, Msgs) -> {publish, Msg} -> receive_messages(Count-1, [Msg|Msgs]); Other -> - ct:log("~p~n", [Other]), + ct:log("~p~n", [Other]), receive_messages(Count, Msgs) after 10 -> Msgs @@ -90,18 +170,6 @@ will_message_test(_Config) -> ok = emqx_client:disconnect(C2), ct:print("Will message test succeeded"). -zero_length_clientid_test(_Config) -> - ct:print("Zero length clientid test starting"), - - %% TODO: There are some controversies on the situation when - %% clean_start flag is true and clientid is zero length. - - %% {error, _} = emqx_client:start_link([{clean_start, false}, - %% {client_id, <<>>}]), - {ok, _, _} = emqx_client:start_link([{clean_start, true}, - {client_id, <<>>}]), - ct:print("Zero length clientid test succeeded"). - offline_message_queueing_test(_) -> {ok, C1, _} = emqx_client:start_link([{clean_start, false}, {client_id, <<"c1">>}]), @@ -109,7 +177,7 @@ offline_message_queueing_test(_) -> ok = emqx_client:disconnect(C1), {ok, C2, _} = emqx_client:start_link([{clean_start, true}, {client_id, <<"c2">>}]), - + ok = emqx_client:publish(C2, nth(2, ?TOPICS), <<"qos 0">>, 0), {ok, _} = emqx_client:publish(C2, nth(3, ?TOPICS), <<"qos 1">>, 1), {ok, _} = emqx_client:publish(C2, nth(4, ?TOPICS), <<"qos 2">>, 2), @@ -196,4 +264,3 @@ dollar_topics_test(_) -> ?assertEqual(0, length(receive_messages(1))), ok = emqx_client:disconnect(C), ct:print("$ topics test succeeded"). - diff --git a/test/emqx_packet_SUITE.erl b/test/emqx_packet_SUITE.erl index ec4205957..dda27c45b 100644 --- a/test/emqx_packet_SUITE.erl +++ b/test/emqx_packet_SUITE.erl @@ -44,14 +44,49 @@ packet_type_name(_) -> ?assertEqual('UNSUBSCRIBE', emqx_packet:type_name(?UNSUBSCRIBE)). packet_validate(_) -> - ?assertEqual(true, emqx_packet:validate(?SUBSCRIBE_PACKET(15, #{'Subscription-Identifier' => 1}, [{<<"topic">>, #{qos => ?QOS0}}]))), - ?assertEqual(true, emqx_packet:validate(?UNSUBSCRIBE_PACKET(89, [<<"topic">>]))), - ?assertEqual(true, emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{}))), - ?assertEqual(true, emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' => 1}}))), - case catch emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' => 0}})) of - {'EXIT', {protocol_error, _}} -> ?assertEqual(true, true); - true -> ?assertEqual(true, false) - end. + ?assert(emqx_packet:validate(?SUBSCRIBE_PACKET(15, #{'Subscription-Identifier' => 1}, [{<<"topic">>, #{qos => ?QOS0}}]))), + ?assert(emqx_packet:validate(?UNSUBSCRIBE_PACKET(89, [<<"topic">>]))), + ?assert(emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{}))), + ?assert(emqx_packet:validate(?PUBLISH_PACKET(1, <<"topic">>, 1, #{'Response-Topic' => <<"responsetopic">>, 'Topic-Alias' => 1}, <<"payload">>))), + ?assert(emqx_packet:validate(?CONNECT_PACKET(#mqtt_packet_connect{properties = #{'Receive-Maximum' => 1}}))), + ?assertError(subscription_identifier_invalid, + emqx_packet:validate( + ?SUBSCRIBE_PACKET(15, #{'Subscription-Identifier' => -1}, + [{<<"topic">>, #{qos => ?QOS0}}]))), + ?assertError(topic_filters_invalid, + emqx_packet:validate(?UNSUBSCRIBE_PACKET(1,[]))), + ?assertError(topic_name_invalid, + emqx_packet:validate(?PUBLISH_PACKET(1,<<>>,1,#{},<<"payload">>))), + ?assertError(topic_name_invalid, + emqx_packet:validate(?PUBLISH_PACKET + (1, <<"+/+">>, 1, #{}, <<"payload">>))), + ?assertError(topic_alias_invalid, + emqx_packet:validate( + ?PUBLISH_PACKET + (1, <<"topic">>, 1, #{'Topic-Alias' => 0}, <<"payload">>))), + ?assertError(protocol_error, + emqx_packet:validate( + ?PUBLISH_PACKET(1, <<"topic">>, 1, + #{'Subscription-Identifier' => 10}, <<"payload">>))), + ?assertError(protocol_error, + emqx_packet:validate( + ?PUBLISH_PACKET(1, <<"topic">>, 1, + #{'Response-Topic' => <<"+/+">>}, <<"payload">>))), + ?assertError(protocol_error, + emqx_packet:validate( + ?CONNECT_PACKET(#mqtt_packet_connect{ + properties = + #{'Request-Response-Information' => -1}}))), + ?assertError(protocol_error, + emqx_packet:validate( + ?CONNECT_PACKET(#mqtt_packet_connect{ + properties = + #{'Request-Problem-Information' => 2}}))), + ?assertError(protocol_error, + emqx_packet:validate( + ?CONNECT_PACKET(#mqtt_packet_connect{ + properties = + #{'Receive-Maximum' => 0}}))). packet_message(_) -> Pkt = #mqtt_packet{header = #mqtt_packet_header{type = ?PUBLISH, @@ -83,16 +118,14 @@ packet_format(_) -> io:format("~s", [emqx_packet:format(?UNSUBACK_PACKET(90))]). packet_will_msg(_) -> - Pkt = #mqtt_packet_connect{ will_flag = true, - client_id = <<"clientid">>, - username = "test", - will_retain = true, - will_qos = ?QOS2, - will_topic = <<"topic">>, - will_props = #{}, + Pkt = #mqtt_packet_connect{ will_flag = true, + client_id = <<"clientid">>, + username = "test", + will_retain = true, + will_qos = ?QOS2, + will_topic = <<"topic">>, + will_props = #{}, will_payload = <<"payload">>}, Msg = emqx_packet:will_msg(Pkt), ?assertEqual(<<"clientid">>, Msg#message.from), ?assertEqual(<<"topic">>, Msg#message.topic). - - diff --git a/test/emqx_protocol_SUITE.erl b/test/emqx_protocol_SUITE.erl index 2323519b1..f97f475f8 100644 --- a/test/emqx_protocol_SUITE.erl +++ b/test/emqx_protocol_SUITE.erl @@ -19,114 +19,211 @@ -compile(export_all). -compile(nowarn_export_all). --include("emqx.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +-include_lib("common_test/include/ct.hrl"). -include("emqx_mqtt.hrl"). --include_lib("eunit/include/eunit.hrl"). +-define(TOPICS, [<<"TopicA">>, <<"TopicA/B">>, <<"Topic/C">>, <<"TopicA/C">>, + <<"/TopicA">>]). --import(emqx_serializer, [serialize/1]). +-define(CLIENT2, ?CONNECT_PACKET(#mqtt_packet_connect{ + username = <<"admin">>, + clean_start = false, + password = <<"public">>})). all() -> - [%% {group, parser}, - %% {group, serializer}, - {group, packet}, - {group, message}]. + [ + {group, mqttv4}, + {group, mqttv5}]. groups() -> - [%% {parser, [], - %% [ - %% parse_connect, - %% parse_bridge, - %% parse_publish, - %% parse_puback, - %% parse_pubrec, - %% parse_pubrel, - %% parse_pubcomp, - %% parse_subscribe, - %% parse_unsubscribe, - %% parse_pingreq, - %% parse_disconnect]}, - %% {serializer, [], - %% [serialize_connect, - %% serialize_connack, - %% serialize_publish, - %% serialize_puback, - %% serialize_pubrel, - %% serialize_subscribe, - %% serialize_suback, - %% serialize_unsubscribe, - %% serialize_unsuback, - %% serialize_pingreq, - %% serialize_pingresp, - %% serialize_disconnect]}, - {packet, [], - [packet_proto_name, - packet_type_name, - packet_format]}, - {message, [], - [message_make - %% message_from_packet - ]} - ]. + [{mqttv4, + [sequence], + [ + connect_v4, + subscribe_v4 + ]}, + {mqttv5, + [sequence], + [ + connect_v5, + subscribe_v5 + ] + }]. + +init_per_suite(Config) -> + emqx_ct_broker_helpers:run_setup_steps(), + Config. + +end_per_suite(_Config) -> + emqx_ct_broker_helpers:run_teardown_steps(). + +with_connection(DoFun) -> + {ok, Sock} = emqx_client_sock:connect({127, 0, 0, 1}, 1883, + [binary, {packet, raw}, + {active, false}], 3000), + try + DoFun(Sock) + after + emqx_client_sock:close(Sock) + end. + +connect_v4(_) -> + with_connection(fun(Sock) -> + emqx_client_sock:send(Sock, raw_send_serialize(?PACKET(?PUBLISH))), + {error, closed} =gen_tcp:recv(Sock, 0) + end), + with_connection(fun(Sock) -> + ConnectPacket = raw_send_serialize(?CONNECT_PACKET + (#mqtt_packet_connect{ + client_id = <<"mqttv4_client">>, + username = <<"admin">>, + password = <<"public">>, + proto_ver = ?MQTT_PROTO_V4 + })), + emqx_client_sock:send(Sock, ConnectPacket), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?CONNACK_PACKET(?CONNACK_ACCEPT), _} = raw_recv_parse(Data, ?MQTT_PROTO_V4), + + emqx_client_sock:send(Sock, ConnectPacket), + {error, closed} = gen_tcp:recv(Sock, 0) + end), + ok. +connect_v5(_) -> + with_connection(fun(Sock) -> + emqx_client_sock:send(Sock, + raw_send_serialize( + ?CONNECT_PACKET(#mqtt_packet_connect{ + proto_ver = ?MQTT_PROTO_V5, + properties = + #{'Request-Response-Information' => -1}}))), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?DISCONNECT_PACKET(?RC_PROTOCOL_ERROR), _} = raw_recv_parse(Data, ?MQTT_PROTO_V5) + end), -%%-------------------------------------------------------------------- -%% Packet Cases -%%-------------------------------------------------------------------- + with_connection(fun(Sock) -> + emqx_client_sock:send(Sock, + raw_send_serialize( + ?CONNECT_PACKET( + #mqtt_packet_connect{ + proto_ver = ?MQTT_PROTO_V5, + properties = + #{'Request-Problem-Information' => 2}}))), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?DISCONNECT_PACKET(?RC_PROTOCOL_ERROR), _} = raw_recv_parse(Data, ?MQTT_PROTO_V5) + end), -packet_proto_name(_) -> - ?assertEqual(<<"MQIsdp">>, emqx_packet:protocol_name(3)), - ?assertEqual(<<"MQTT">>, emqx_packet:protocol_name(4)). + with_connection(fun(Sock) -> + emqx_client_sock:send(Sock, + raw_send_serialize( + ?CONNECT_PACKET( + #mqtt_packet_connect{ + proto_ver = ?MQTT_PROTO_V5, + properties = + #{'Request-Response-Information' => 1}}) + )), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?CONNACK_PACKET(?RC_SUCCESS, 0, + #{'Response-Information' := _RespInfo}), _} = + raw_recv_parse(Data, ?MQTT_PROTO_V5) + end), + ok. -packet_type_name(_) -> - ?assertEqual('CONNECT', emqx_packet:type_name(?CONNECT)), - ?assertEqual('UNSUBSCRIBE', emqx_packet:type_name(?UNSUBSCRIBE)). +do_connect(Sock, ProtoVer) -> + emqx_client_sock:send(Sock, raw_send_serialize( + ?CONNECT_PACKET( + #mqtt_packet_connect{ + client_id = <<"mqtt_client">>, + proto_ver = ProtoVer + }))), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?CONNACK_PACKET(?CONNACK_ACCEPT), _} = raw_recv_parse(Data, ProtoVer). -%% packet_connack_name(_) -> -%% ?assertEqual('CONNACK_ACCEPT', emqx_packet:connack_name(?CONNACK_ACCEPT)), -%% ?assertEqual('CONNACK_PROTO_VER', emqx_packet:connack_name(?CONNACK_PROTO_VER)), -%% ?assertEqual('CONNACK_INVALID_ID', emqx_packet:connack_name(?CONNACK_INVALID_ID)), -%% ?assertEqual('CONNACK_SERVER', emqx_packet:connack_name(?CONNACK_SERVER)), -%% ?assertEqual('CONNACK_CREDENTIALS', emqx_packet:connack_name(?CONNACK_CREDENTIALS)), -%% ?assertEqual('CONNACK_AUTH', emqx_packet:connack_name(?CONNACK_AUTH)). +subscribe_v4(_) -> + with_connection(fun(Sock) -> + do_connect(Sock, ?MQTT_PROTO_V4), + SubPacket = raw_send_serialize( + ?SUBSCRIBE_PACKET(15, + [{<<"topic">>, #{rh => 1, + qos => ?QOS_2, + rap => 0, + nl => 0, + rc => 0}}])), + emqx_client_sock:send(Sock, SubPacket), + {ok, Data} = gen_tcp:recv(Sock, 0), + {ok, ?SUBACK_PACKET(15, _), _} = raw_recv_parse(Data, ?MQTT_PROTO_V4) + end), + ok. -packet_format(_) -> - io:format("~s", [emqx_packet:format(?CONNECT_PACKET(#mqtt_packet_connect{}))]), - io:format("~s", [emqx_packet:format(?CONNACK_PACKET(?CONNACK_SERVER))]), - io:format("~s", [emqx_packet:format(?PUBLISH_PACKET(?QOS_1, 1))]), - io:format("~s", [emqx_packet:format(?PUBLISH_PACKET(?QOS_2, <<"topic">>, 10, <<"payload">>))]), - io:format("~s", [emqx_packet:format(?PUBACK_PACKET(?PUBACK, 98))]), - io:format("~s", [emqx_packet:format(?PUBREL_PACKET(99))]), - io:format("~s", [emqx_packet:format(?SUBSCRIBE_PACKET(15, [{<<"topic">>, ?QOS0}, {<<"topic1">>, ?QOS1}]))]), - io:format("~s", [emqx_packet:format(?SUBACK_PACKET(40, [?QOS0, ?QOS1]))]), - io:format("~s", [emqx_packet:format(?UNSUBSCRIBE_PACKET(89, [<<"t">>, <<"t2">>]))]), - io:format("~s", [emqx_packet:format(?UNSUBACK_PACKET(90))]). +subscribe_v5(_) -> + with_connection(fun(Sock) -> + do_connect(Sock, ?MQTT_PROTO_V5), + SubPacket = raw_send_serialize(?SUBSCRIBE_PACKET(15, #{'Subscription-Identifier' => -1},[]), + #{version => ?MQTT_PROTO_V5}), + emqx_client_sock:send(Sock, SubPacket), + {ok, DisConnData} = gen_tcp:recv(Sock, 0), + {ok, ?DISCONNECT_PACKET(?RC_TOPIC_FILTER_INVALID), _} = + raw_recv_parse(DisConnData, ?MQTT_PROTO_V5) + end), + with_connection(fun(Sock) -> + do_connect(Sock, ?MQTT_PROTO_V5), + SubPacket = raw_send_serialize( + ?SUBSCRIBE_PACKET(0, #{}, [{<<"TopicQos0">>, + #{rh => 1, qos => ?QOS_2, + rap => 0, nl => 0, + rc => 0}}]), + #{version => ?MQTT_PROTO_V5}), + emqx_client_sock:send(Sock, SubPacket), + {ok, DisConnData} = gen_tcp:recv(Sock, 0), + {ok, ?DISCONNECT_PACKET(?RC_MALFORMED_PACKET), _} = + raw_recv_parse(DisConnData, ?MQTT_PROTO_V5) + end), + with_connection(fun(Sock) -> + do_connect(Sock, ?MQTT_PROTO_V5), + SubPacket = raw_send_serialize( + ?SUBSCRIBE_PACKET(1, #{'Subscription-Identifier' => 0}, + [{<<"TopicQos0">>, + #{rh => 1, qos => ?QOS_2, + rap => 0, nl => 0, + rc => 0}}]), + #{version => ?MQTT_PROTO_V5}), + emqx_client_sock:send(Sock, SubPacket), + {ok, DisConnData} = gen_tcp:recv(Sock, 0), + {ok, ?DISCONNECT_PACKET(?RC_SUBSCRIPTION_IDENTIFIERS_NOT_SUPPORTED), _} = + raw_recv_parse(DisConnData, ?MQTT_PROTO_V5) + end), + with_connection(fun(Sock) -> + do_connect(Sock, ?MQTT_PROTO_V5), + SubPacket = raw_send_serialize( + ?SUBSCRIBE_PACKET(1, #{'Subscription-Identifier' => 1}, + [{<<"TopicQos0">>, + #{rh => 1, qos => ?QOS_2, + rap => 0, nl => 0, + rc => 0}}]), + #{version => ?MQTT_PROTO_V5}), + emqx_client_sock:send(Sock, SubPacket), + {ok, SubData} = gen_tcp:recv(Sock, 0), + {ok, ?SUBACK_PACKET(1, #{}, [2]), _} = + raw_recv_parse(SubData, ?MQTT_PROTO_V5) + end), + ok. -%%-------------------------------------------------------------------- -%% Message Cases -%%-------------------------------------------------------------------- +publish_v4(_) -> + ok. -message_make(_) -> - Msg = emqx_message:make(<<"clientid">>, <<"topic">>, <<"payload">>), - ?assertEqual(0, Msg#message.qos), - Msg1 = emqx_message:make(<<"clientid">>, qos2, <<"topic">>, <<"payload">>), - ?assert(is_binary(Msg1#message.id)), - ?assertEqual(qos2, Msg1#message.qos). +publish_v5(_) -> + ok. -%% message_from_packet(_) -> -%% Msg = emqx_message:from_packet(?PUBLISH_PACKET(1, <<"topic">>, 10, <<"payload">>)), -%% ?assertEqual(1, Msg#message.qos), -%% %% ?assertEqual(10, Msg#message.pktid), -%% ?assertEqual(<<"topic">>, Msg#message.topic), -%% WillMsg = emqx_message:from_packet(#mqtt_packet_connect{will_flag = true, -%% will_topic = <<"WillTopic">>, -%% will_payload = <<"WillMsg">>}), -%% ?assertEqual(<<"WillTopic">>, WillMsg#message.topic), -%% ?assertEqual(<<"WillMsg">>, WillMsg#message.payload). - - %% Msg2 = emqx_message:fomat_packet(<<"username">>, <<"clientid">>, - %% ?PUBLISH_PACKET(1, <<"topic">>, 20, <<"payload">>)), +raw_send_serialize(Packet) -> + emqx_frame:serialize(Packet). +raw_send_serialize(Packet, Opts) -> + emqx_frame:serialize(Packet, Opts). +raw_recv_parse(P, ProtoVersion) -> + emqx_frame:parse(P, {none, #{max_packet_size => ?MAX_PACKET_SIZE, + version => ProtoVersion}}). diff --git a/test/emqx_topic_SUITE.erl b/test/emqx_topic_SUITE.erl index 816579ebc..8ce3faf79 100644 --- a/test/emqx_topic_SUITE.erl +++ b/test/emqx_topic_SUITE.erl @@ -21,7 +21,7 @@ -compile(nowarn_export_all). -import(emqx_topic, [wildcard/1, match/2, validate/1, triples/1, join/1, - words/1, systop/1, feed_var/3, parse/1, parse/2]). + words/1, systop/1, feed_var/3, parse/1]). -define(N, 10000). @@ -57,10 +57,10 @@ t_match(_) -> true = match(<<"abc">>, <<"+">>), true = match(<<"a/b/c">>, <<"a/b/c">>), false = match(<<"a/b/c">>, <<"a/c/d">>), - false = match(<<"$shared/x/y">>, <<"+">>), - false = match(<<"$shared/x/y">>, <<"+/x/y">>), - false = match(<<"$shared/x/y">>, <<"#">>), - false = match(<<"$shared/x/y">>, <<"+/+/#">>), + false = match(<<"$share/x/y">>, <<"+">>), + false = match(<<"$share/x/y">>, <<"+/x/y">>), + false = match(<<"$share/x/y">>, <<"#">>), + false = match(<<"$share/x/y">>, <<"+/+/#">>), false = match(<<"house/1/sensor/0">>, <<"house/+">>), false = match(<<"house">>, <<"house/+">>). @@ -77,10 +77,10 @@ t_match2(_) -> true = match(<<"abc">>, <<"+">>), true = match(<<"a/b/c">>, <<"a/b/c">>), false = match(<<"a/b/c">>, <<"a/c/d">>), - false = match(<<"$shared/x/y">>, <<"+">>), - false = match(<<"$shared/x/y">>, <<"+/x/y">>), - false = match(<<"$shared/x/y">>, <<"#">>), - false = match(<<"$shared/x/y">>, <<"+/+/#">>), + false = match(<<"$share/x/y">>, <<"+">>), + false = match(<<"$share/x/y">>, <<"+/x/y">>), + false = match(<<"$share/x/y">>, <<"#">>), + false = match(<<"$share/x/y">>, <<"+/+/#">>), false = match(<<"house/1/sensor/0">>, <<"house/+">>). t_match3(_) -> @@ -208,4 +208,3 @@ t_parse(_) -> ?assertEqual({<<"$local/$queue/topic">>, #{}}, parse(<<"$local/$queue/topic">>)), ?assertEqual({<<"$local/$share/group/a/b/c">>, #{}}, parse(<<"$local/$share/group/a/b/c">>)), ?assertEqual({<<"$fastlane/topic">>, #{}}, parse(<<"$fastlane/topic">>)). - diff --git a/test/rfc6455_client.erl b/test/rfc6455_client.erl index 4696f7ab3..f5d8f1ef4 100644 --- a/test/rfc6455_client.erl +++ b/test/rfc6455_client.erl @@ -153,7 +153,7 @@ do_recv2(State = #state{phase = Phase, socket = Socket, ppid = PPid}, R) -> ok end, die(Socket, PPid, WsReason, normal); - {_, _, _, Rest2} -> + {_, _, _, _Rest2} -> io:format("Unknown frame type~n"), die(Socket, PPid, {1006, "Unknown frame type"}, normal) end. diff --git a/test/ws_client.erl b/test/ws_client.erl index f049e3256..25f38164d 100644 --- a/test/ws_client.erl +++ b/test/ws_client.erl @@ -1,7 +1,5 @@ -module(ws_client). --behaviour(websocket_client_handler). - -export([ start_link/0, start_link/1, From 73658b39536f595eea4031b0aa28a409be58452f Mon Sep 17 00:00:00 2001 From: huangdan Date: Fri, 19 Oct 2018 15:46:25 +0800 Subject: [PATCH 27/28] Merged emqx_misc_SUITE to emqx_misc_tests (#1890) * Merged emqx_misc_SUITE to emqx_misc_tests --- Makefile | 3 ++- test/emqx_misc_SUITE.erl | 45 ---------------------------------------- test/emqx_misc_tests.erl | 25 +++++++++++++++++++++- 3 files changed, 26 insertions(+), 47 deletions(-) delete mode 100644 test/emqx_misc_SUITE.erl diff --git a/Makefile b/Makefile index 9a114399c..f9c8bd201 100644 --- a/Makefile +++ b/Makefile @@ -35,9 +35,10 @@ EUNIT_OPTS = verbose # CT_SUITES = emqx_frame ## emqx_trie emqx_router emqx_frame emqx_mqtt_compat + CT_SUITES = emqx emqx_client emqx_zone emqx_banned emqx_connection emqx_session \ emqx_access emqx_broker emqx_cm emqx_frame emqx_guid emqx_inflight emqx_json \ - emqx_keepalive emqx_lib emqx_metrics emqx_misc emqx_mod emqx_mqtt_caps \ + emqx_keepalive emqx_lib emqx_metrics emqx_mod emqx_mqtt_caps \ emqx_mqtt_props emqx_mqueue emqx_net emqx_pqueue emqx_router emqx_sm \ emqx_tables emqx_time emqx_topic emqx_trie emqx_vm emqx_mountpoint \ emqx_listeners emqx_protocol emqx_pool emqx_shared_sub diff --git a/test/emqx_misc_SUITE.erl b/test/emqx_misc_SUITE.erl deleted file mode 100644 index 766691869..000000000 --- a/test/emqx_misc_SUITE.erl +++ /dev/null @@ -1,45 +0,0 @@ -%% Copyright (c) 2018 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_misc_SUITE). - --include_lib("eunit/include/eunit.hrl"). - --compile(export_all). --compile(nowarn_export_all). - --define(SOCKOPTS, [binary, - {packet, raw}, - {reuseaddr, true}, - {backlog, 512}, - {nodelay, true}]). - -all() -> [t_merge_opts]. - -t_merge_opts(_) -> - Opts = emqx_misc:merge_opts(?SOCKOPTS, [raw, - binary, - {backlog, 1024}, - {nodelay, false}, - {max_clients, 1024}, - {acceptors, 16}]), - ?assertEqual(1024, proplists:get_value(backlog, Opts)), - ?assertEqual(1024, proplists:get_value(max_clients, Opts)), - [binary, raw, - {acceptors, 16}, - {backlog, 1024}, - {max_clients, 1024}, - {nodelay, false}, - {packet, raw}, - {reuseaddr, true}] = lists:sort(Opts). diff --git a/test/emqx_misc_tests.erl b/test/emqx_misc_tests.erl index 50513ee86..92b0973e8 100644 --- a/test/emqx_misc_tests.erl +++ b/test/emqx_misc_tests.erl @@ -15,6 +15,30 @@ -module(emqx_misc_tests). -include_lib("eunit/include/eunit.hrl"). +-define(SOCKOPTS, [binary, + {packet, raw}, + {reuseaddr, true}, + {backlog, 512}, + {nodelay, true}]). + + +t_merge_opts_test() -> + Opts = emqx_misc:merge_opts(?SOCKOPTS, [raw, + binary, + {backlog, 1024}, + {nodelay, false}, + {max_clients, 1024}, + {acceptors, 16}]), + ?assertEqual(1024, proplists:get_value(backlog, Opts)), + ?assertEqual(1024, proplists:get_value(max_clients, Opts)), + [binary, raw, + {acceptors, 16}, + {backlog, 1024}, + {max_clients, 1024}, + {nodelay, false}, + {packet, raw}, + {reuseaddr, true}] = lists:sort(Opts). + timer_cancel_flush_test() -> Timer = emqx_misc:start_timer(0, foo), ok = emqx_misc:cancel_timer(Timer), @@ -39,4 +63,3 @@ message_queue_too_long_test() -> conn_proc_mng_policy(L) -> emqx_misc:conn_proc_mng_policy(#{message_queue_len => L}). - From 873a08dc94b061747a9e6282ce5ffcb858e34467 Mon Sep 17 00:00:00 2001 From: tigercl Date: Fri, 19 Oct 2018 16:21:43 +0800 Subject: [PATCH 28/28] Improve coverage for emqx_hooks, and add test case for emqx_mod_sup (#1892) Improve coverage for emqx_hooks, and add test case for emqx_mod_sup --- Makefile | 3 +-- test/emqx_hooks_SUITE.erl | 19 ++++++++++++++++++- test/emqx_mod_sup_SUITE.erl | 28 ++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 test/emqx_mod_sup_SUITE.erl diff --git a/Makefile b/Makefile index f9c8bd201..dbd503864 100644 --- a/Makefile +++ b/Makefile @@ -35,10 +35,9 @@ EUNIT_OPTS = verbose # CT_SUITES = emqx_frame ## emqx_trie emqx_router emqx_frame emqx_mqtt_compat - CT_SUITES = emqx emqx_client emqx_zone emqx_banned emqx_connection emqx_session \ emqx_access emqx_broker emqx_cm emqx_frame emqx_guid emqx_inflight emqx_json \ - emqx_keepalive emqx_lib emqx_metrics emqx_mod emqx_mqtt_caps \ + emqx_keepalive emqx_lib emqx_metrics emqx_mod emqx_mod_sup emqx_mqtt_caps \ emqx_mqtt_props emqx_mqueue emqx_net emqx_pqueue emqx_router emqx_sm \ emqx_tables emqx_time emqx_topic emqx_trie emqx_vm emqx_mountpoint \ emqx_listeners emqx_protocol emqx_pool emqx_shared_sub diff --git a/test/emqx_hooks_SUITE.erl b/test/emqx_hooks_SUITE.erl index b5b278e31..1961d0e02 100644 --- a/test/emqx_hooks_SUITE.erl +++ b/test/emqx_hooks_SUITE.erl @@ -43,7 +43,7 @@ add_delete_hook(_) -> {callback, {?MODULE, hook_fun2, []}, undefined, 8}], ?assertEqual(Callbacks2, emqx_hooks:lookup(emqx_hook)), ok = emqx:unhook(emqx_hook, {?MODULE, hook_fun1, []}), - ok = emqx:unhook(emqx_hook, {?MODULE, hook_fun2, []}), + ok = emqx:unhook(emqx_hook, {?MODULE, hook_fun2}), timer:sleep(1000), ?assertEqual([], emqx_hooks:lookup(emqx_hook)), ok = emqx_hooks:stop(). @@ -62,6 +62,17 @@ run_hooks(_) -> ok = emqx:hook(foreach_hook, fun ?MODULE:hook_fun7/2, [initArg]), ok = emqx:hook(foreach_hook, fun ?MODULE:hook_fun8/2, [initArg]), stop = emqx:run_hooks(foreach_hook, [arg]), + + ok = emqx:hook(foldl_hook2, fun ?MODULE:hook_fun9/2), + ok = emqx:hook(foldl_hook2, {?MODULE, hook_fun10, []}), + {stop, []} = emqx:run_hooks(foldl_hook2, [arg], []), + + ok = emqx:hook(filter1_hook, {?MODULE, hook_fun1, []}, {?MODULE, hook_filter1, []}, 0), + ok = emqx:run_hooks(filter1_hook, [arg]), + + ok = emqx:hook(filter2_hook, {?MODULE, hook_fun2, []}, {?MODULE, hook_filter2, []}), + {ok, []} = emqx:run_hooks(filter2_hook, [arg], []), + ok = emqx_hooks:stop(). hook_fun1([]) -> ok. @@ -75,3 +86,9 @@ hook_fun6(arg, initArg) -> ok. hook_fun7(arg, initArg) -> any. hook_fun8(arg, initArg) -> stop. +hook_fun9(arg, _Acc) -> any. +hook_fun10(arg, _Acc) -> stop. + +hook_filter1(arg) -> true. +hook_filter2(arg, _Acc) -> true. + diff --git a/test/emqx_mod_sup_SUITE.erl b/test/emqx_mod_sup_SUITE.erl new file mode 100644 index 000000000..87245e351 --- /dev/null +++ b/test/emqx_mod_sup_SUITE.erl @@ -0,0 +1,28 @@ +%% Copyright (c) 2018 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_mod_sup_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include("emqx.hrl"). + +all() -> [t_child_all]. + +t_child_all(_) -> + {ok, _Pid} = emqx_mod_sup:start_link(), + {ok, _Child} = emqx_mod_sup:start_child(emqx_banned, worker), + timer:sleep(10), + ok = emqx_mod_sup:stop_child(emqx_banned).