Browse Source

perf(topic): use tail recursive to joining a topic

JimMoen 2 years ago
parent
commit
4ea3483083
2 changed files with 21 additions and 16 deletions
  1. 17 15
      apps/emqx/src/emqx_topic.erl
  2. 4 1
      apps/emqx/test/emqx_topic_SUITE.erl

+ 17 - 15
apps/emqx/src/emqx_topic.erl

@@ -39,6 +39,11 @@
 -type word() :: emqx_types:word().
 -type words() :: emqx_types:words().
 
+%% Guards
+-define(MULTI_LEVEL_WILDCARD_NOT_LAST(C, REST),
+    ((C =:= '#' orelse C =:= <<"#">>) andalso REST =/= [])
+).
+
 %%--------------------------------------------------------------------
 %% APIs
 %%--------------------------------------------------------------------
@@ -110,7 +115,8 @@ validate2([]) ->
 % end with '#'
 validate2(['#']) ->
     true;
-validate2(['#' | Words]) when length(Words) > 0 ->
+%% MQTT-5.0 [MQTT-4.7.1-1]
+validate2([C | Words]) when ?MULTI_LEVEL_WILDCARD_NOT_LAST(C, Words) ->
     error('topic_invalid_#');
 validate2(['' | Words]) ->
     validate2(Words);
@@ -213,20 +219,16 @@ feed_var(Var, Val, [W | Words], Acc) ->
 -spec join(list(word())) -> binary().
 join([]) ->
     <<>>;
-join([W]) ->
-    bin(W);
-join(Words) ->
-    {_, Bin} = lists:foldr(
-        fun
-            (W, {true, Tail}) ->
-                {false, <<W/binary, Tail/binary>>};
-            (W, {false, Tail}) ->
-                {false, <<W/binary, "/", Tail/binary>>}
-        end,
-        {true, <<>>},
-        [bin(W) || W <- Words]
-    ),
-    Bin.
+join([Word | Words]) ->
+    do_join(bin(Word), Words).
+
+do_join(TopicAcc, []) ->
+    TopicAcc;
+%% MQTT-5.0 [MQTT-4.7.1-1]
+do_join(_TopicAcc, [C | Words]) when ?MULTI_LEVEL_WILDCARD_NOT_LAST(C, Words) ->
+    error('topic_invalid_#');
+do_join(TopicAcc, [Word | Words]) ->
+    do_join(<<TopicAcc/binary, "/", (bin(Word))/binary>>, Words).
 
 -spec parse(topic() | {topic(), map()}) -> {topic(), #{share => binary()}}.
 parse(TopicFilter) when is_binary(TopicFilter) ->

+ 4 - 1
apps/emqx/test/emqx_topic_SUITE.erl

@@ -199,7 +199,10 @@ t_join(_) ->
     ?assertEqual(<<"+//#">>, join(['+', '', '#'])),
     ?assertEqual(<<"x/y/z/+">>, join([<<"x">>, <<"y">>, <<"z">>, '+'])),
     ?assertEqual(<<"/ab/cd/ef/">>, join(words(<<"/ab/cd/ef/">>))),
-    ?assertEqual(<<"ab/+/#">>, join(words(<<"ab/+/#">>))).
+    ?assertEqual(<<"ab/+/#">>, join(words(<<"ab/+/#">>))),
+    %% MQTT-5.0 [MQTT-4.7.1-1]
+    ?assertError('topic_invalid_#', join(['+', <<"a">>, '#', <<"b">>, '', '+'])),
+    ?assertError('topic_invalid_#', join(['+', <<"c">>, <<"#">>, <<"d">>, '', '+'])).
 
 t_systop(_) ->
     SysTop1 = iolist_to_binary(["$SYS/brokers/", atom_to_list(node()), "/xyz"]),