Преглед на файлове

test: refine tests for lots of List HTTP API

JianBo He преди 3 години
родител
ревизия
8a0c468b01

+ 1 - 1
apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl

@@ -213,7 +213,7 @@ t_list_users(_) ->
 
     #{
         data := [#{is_superuser := false, user_id := <<"u3">>}],
-        meta := #{page := 1, limit := 20, count := 1}
+        meta := #{page := 1, limit := 20, count := 0}
     } = emqx_authn_mnesia:list_users(
         #{
             <<"page">> => 1,

+ 46 - 37
apps/emqx_management/src/emqx_mgmt_api.erl

@@ -21,6 +21,7 @@
 -elvis([{elvis_style, dont_repeat_yourself, #{min_complexity => 100}}]).
 
 -define(FRESH_SELECT, fresh_select).
+-define(LONG_QUERY_TIMEOUT, 50000).
 
 -export([
     paginate/3,
@@ -35,6 +36,7 @@
 ]).
 
 -export([do_query/5]).
+-export([parse_qstring/2]).
 
 paginate(Tables, Params, {Module, FormatFun}) ->
     Qh = query_handle(Tables),
@@ -236,25 +238,30 @@ do_cluster_query(
 
 maybe_collect_total_from_tail_nodes([], _Tab, _QString, _MsFun, ResultAcc) ->
     ResultAcc;
-maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc = #{total := TotalAcc}) ->
+maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc) ->
     {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]),
-    case is_countable_total(Ms, FuzzyFun) of
-        true ->
-            %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node
-            case rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun]) of
-                {_, [Node | _]} ->
-                    {error, Node, {badrpc, badnode}};
-                {ResL0, []} ->
-                    ResL = lists:zip(Nodes, ResL0),
-                    case lists:filter(fun({_, I}) -> not is_integer(I) end, ResL) of
-                        [{Node, {badrpc, Reason}} | _] ->
-                            {error, Node, {badrpc, Reason}};
-                        [] ->
-                            ResultAcc#{total => ResL ++ TotalAcc}
-                    end
-            end;
+    case counting_total_fun(Ms, FuzzyFun) of
         false ->
-            ResultAcc
+            ResultAcc;
+        _Fun ->
+            collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc)
+    end.
+
+collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc = #{total := TotalAcc}) ->
+    %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node
+    case
+        rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun], ?LONG_QUERY_TIMEOUT)
+    of
+        {_, [Node | _]} ->
+            {error, Node, {badrpc, badnode}};
+        {ResL0, []} ->
+            ResL = lists:zip(Nodes, ResL0),
+            case lists:filter(fun({_, I}) -> not is_integer(I) end, ResL) of
+                [{Node, {badrpc, Reason}} | _] ->
+                    {error, Node, {badrpc, Reason}};
+                [] ->
+                    ResultAcc#{total => ResL ++ TotalAcc}
+            end
     end.
 
 %%--------------------------------------------------------------------
@@ -286,7 +293,7 @@ do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) ->
             ?MODULE,
             do_query,
             [Node, Tab, QString, MsFun, QueryState],
-            50000
+            ?LONG_QUERY_TIMEOUT
         )
     of
         {badrpc, _} = R -> {error, R};
@@ -329,26 +336,31 @@ maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState = #{total := TotalAc
             QueryState
     end.
 
-%% XXX: Calculating the total number of data that match a certain condition under a large table
-%% is very expensive because the entire ETS table needs to be scanned.
 apply_total_query(Tab, Ms, FuzzyFun) ->
-    case is_countable_total(Ms, FuzzyFun) of
-        true ->
-            ets:info(Tab, size);
+    case counting_total_fun(Ms, FuzzyFun) of
         false ->
             %% return a fake total number if the query have any conditions
-            0
+            0;
+        Fun ->
+            Fun(Tab)
     end.
 
-is_countable_total(Ms, FuzzyFun) ->
-    FuzzyFun =:= undefined andalso is_non_conditions_match_spec(Ms).
-
-is_non_conditions_match_spec([{_MatchHead, _Conds = [], _Return} | More]) ->
-    is_non_conditions_match_spec(More);
-is_non_conditions_match_spec([{_MatchHead, Conds, _Return} | _More]) when length(Conds) =/= 0 ->
-    false;
-is_non_conditions_match_spec([]) ->
-    true.
+counting_total_fun(Ms, undefined) ->
+    %% XXX: Calculating the total number of data that match a certain
+    %% condition under a large table is very expensive because the
+    %% entire ETS table needs to be scanned.
+    %%
+    %% XXX: How to optimize it? i.e, using:
+    %% `fun(Tab) -> ets:info(Tab, size) end`
+    [{MatchHead, Conditions, _Return}] = Ms,
+    CountingMs = [{MatchHead, Conditions, [true]}],
+    fun(Tab) ->
+        ets:select_count(Tab, CountingMs)
+    end;
+counting_total_fun(_Ms, FuzzyFun) when is_function(FuzzyFun) ->
+    %% XXX: Calculating the total number for a fuzzy searching is very very expensive
+    %% so it is not supported now
+    false.
 
 %% ResultAcc :: #{count := integer(),
 %%                cursor := integer(),
@@ -387,10 +399,6 @@ accumulate_query_rows(
             }}
     end.
 
-%%--------------------------------------------------------------------
-%% Table Select
-%%--------------------------------------------------------------------
-
 %%--------------------------------------------------------------------
 %% Internal Functions
 %%--------------------------------------------------------------------
@@ -402,6 +410,7 @@ parse_qstring(QString, QSchema) ->
     {length(NQString) + length(FuzzyQString), {NQString, FuzzyQString}}.
 
 do_parse_qstring([], _, Acc1, Acc2) ->
+    %% remove fuzzy keys if present in accurate query
     NAcc2 = [E || E <- Acc2, not lists:keymember(element(1, E), 1, Acc1)],
     {lists:reverse(Acc1), lists:reverse(NAcc2)};
 do_parse_qstring([{Key, Value} | RestQString], QSchema, Acc1, Acc2) ->

+ 3 - 1
apps/emqx_management/test/emqx_mgmt_api_subscription_SUITE.erl

@@ -93,6 +93,7 @@ t_subscription_api(_) ->
         {"match_topic", "t/#"}
     ]),
     Headers = emqx_mgmt_api_test_util:auth_header_(),
+
     {ok, ResponseTopic2} = emqx_mgmt_api_test_util:request_api(get, Path, QS, Headers),
     DataTopic2 = emqx_json:decode(ResponseTopic2, [return_maps]),
     Meta2 = maps:get(<<"meta">>, DataTopic2),
@@ -114,7 +115,8 @@ t_subscription_api(_) ->
     MatchMeta = maps:get(<<"meta">>, MatchData),
     ?assertEqual(1, maps:get(<<"page">>, MatchMeta)),
     ?assertEqual(emqx_mgmt:max_row_limit(), maps:get(<<"limit">>, MatchMeta)),
-    ?assertEqual(1, maps:get(<<"count">>, MatchMeta)),
+    %% count equals 0 in fuzzy searching
+    ?assertEqual(0, maps:get(<<"count">>, MatchMeta)),
     MatchSubs = maps:get(<<"data">>, MatchData),
     ?assertEqual(1, length(MatchSubs)),
 

+ 37 - 23
apps/emqx_rule_engine/src/emqx_rule_engine_api.erl

@@ -554,41 +554,55 @@ filter_out_request_body(Conf) ->
     maps:without(ExtraConfs, Conf).
 
 qs2ms(_Tab, {Qs, Fuzzy}) ->
-    Ms = qs2ms(),
-    {Ms, fuzzy_match_fun(Qs, Ms, Fuzzy)}.
+    case lists:keytake(from, 1, Qs) of
+        false ->
+            {generate_match_spec(Qs), fuzzy_match_fun(Fuzzy)};
+        {value, {from, '=:=', From}, Ls} ->
+            {generate_match_spec(Ls), fuzzy_match_fun([{from, '=:=', From} | Fuzzy])}
+    end.
+
+generate_match_spec(Qs) ->
+    {MtchHead, Conds} = generate_match_spec(Qs, 2, {#{}, []}),
+    [{{'_', MtchHead}, Conds, ['$_']}].
+
+generate_match_spec([], _, {MtchHead, Conds}) ->
+    {MtchHead, lists:reverse(Conds)};
+generate_match_spec([Qs | Rest], N, {MtchHead, Conds}) ->
+    Holder = binary_to_atom(iolist_to_binary(["$", integer_to_list(N)]), utf8),
+    NMtchHead = emqx_mgmt_util:merge_maps(MtchHead, ms(element(1, Qs), Holder)),
+    NConds = put_conds(Qs, Holder, Conds),
+    generate_match_spec(Rest, N + 1, {NMtchHead, NConds}).
+
+put_conds({_, Op, V}, Holder, Conds) ->
+    [{Op, Holder, V} | Conds];
+put_conds({_, Op1, V1, Op2, V2}, Holder, Conds) ->
+    [
+        {Op2, Holder, V2},
+        {Op1, Holder, V1}
+        | Conds
+    ].
 
-%% rule is not a record, so everything is fuzzy filter.
-qs2ms() ->
-    [{'_', [], ['$_']}].
+ms(enable, X) ->
+    #{enable => X}.
 
-fuzzy_match_fun(Qs, Ms, Fuzzy) ->
-    MsC = ets:match_spec_compile(Ms),
-    fun(Rows) ->
-        Ls = ets:match_spec_run(Rows, MsC),
+fuzzy_match_fun([]) ->
+    undefined;
+fuzzy_match_fun(Fuzzy) ->
+    fun(MsRaws) when is_list(MsRaws) ->
         lists:filter(
-            fun(E) ->
-                run_qs_match(E, Qs) andalso
-                    run_fuzzy_match(E, Fuzzy)
-            end,
-            Ls
+            fun(E) -> run_fuzzy_match(E, Fuzzy) end,
+            MsRaws
         )
     end.
 
-run_qs_match(_, []) ->
-    true;
-run_qs_match(E = {_Id, #{enable := Enable}}, [{enable, '=:=', Pattern} | Qs]) ->
-    Enable =:= Pattern andalso run_qs_match(E, Qs);
-run_qs_match(E = {_Id, #{from := From}}, [{from, '=:=', Pattern} | Qs]) ->
-    lists:member(Pattern, From) andalso run_qs_match(E, Qs);
-run_qs_match(E, [_ | Qs]) ->
-    run_qs_match(E, Qs).
-
 run_fuzzy_match(_, []) ->
     true;
 run_fuzzy_match(E = {Id, _}, [{id, like, Pattern} | Fuzzy]) ->
     binary:match(Id, Pattern) /= nomatch andalso run_fuzzy_match(E, Fuzzy);
 run_fuzzy_match(E = {_Id, #{description := Desc}}, [{description, like, Pattern} | Fuzzy]) ->
     binary:match(Desc, Pattern) /= nomatch andalso run_fuzzy_match(E, Fuzzy);
+run_fuzzy_match(E = {_, #{from := Topics}}, [{from, '=:=', Pattern} | Fuzzy]) ->
+    lists:member(Pattern, Topics) /= false andalso run_fuzzy_match(E, Fuzzy);
 run_fuzzy_match(E = {_Id, #{from := Topics}}, [{from, match, Pattern} | Fuzzy]) ->
     lists:any(fun(For) -> emqx_topic:match(For, Pattern) end, Topics) andalso
         run_fuzzy_match(E, Fuzzy);

+ 6 - 6
apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl

@@ -133,23 +133,23 @@ t_list_rule_api(_Config) ->
 
     QueryStr2 = #{query_string => #{<<"like_description">> => <<"也能"/utf8>>}},
     {200, Result2} = emqx_rule_engine_api:'/rules'(get, QueryStr2),
-    ?assertEqual(Result1, Result2),
+    ?assertEqual(maps:get(data, Result1), maps:get(data, Result2)),
 
     QueryStr3 = #{query_string => #{<<"from">> => <<"t/1">>}},
-    {200, #{meta := #{count := Count3}}} = emqx_rule_engine_api:'/rules'(get, QueryStr3),
-    ?assertEqual(19, Count3),
+    {200, #{data := Data3}} = emqx_rule_engine_api:'/rules'(get, QueryStr3),
+    ?assertEqual(19, length(Data3)),
 
     QueryStr4 = #{query_string => #{<<"like_from">> => <<"t/1/+">>}},
     {200, Result4} = emqx_rule_engine_api:'/rules'(get, QueryStr4),
-    ?assertEqual(Result1, Result4),
+    ?assertEqual(maps:get(data, Result1), maps:get(data, Result4)),
 
     QueryStr5 = #{query_string => #{<<"match_from">> => <<"t/+/+">>}},
     {200, Result5} = emqx_rule_engine_api:'/rules'(get, QueryStr5),
-    ?assertEqual(Result1, Result5),
+    ?assertEqual(maps:get(data, Result1), maps:get(data, Result5)),
 
     QueryStr6 = #{query_string => #{<<"like_id">> => RuleID}},
     {200, Result6} = emqx_rule_engine_api:'/rules'(get, QueryStr6),
-    ?assertEqual(Result1, Result6),
+    ?assertEqual(maps:get(data, Result1), maps:get(data, Result6)),
 
     %% clean up
     lists:foreach(