From 691407f9fd84312529b21b729fb7b45f24c4698a Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Wed, 20 Jul 2022 11:16:44 +0800
Subject: [PATCH] fix: add type binary_file to emqx_rule_validator

---
 .../src/emqx_rule_engine.appup.src            |  2 +
 .../src/emqx_rule_validator.erl               | 34 ++++++++---
 .../test/emqx_rule_validator_SUITE.erl        | 60 ++++++++++++++++++-
 3 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
index fd764bae2..caed4d0a8 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
@@ -3,6 +3,7 @@
 {VSN,
   [{"4.3.11",
     [{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
+     {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
    {"4.3.10",
     [{load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
@@ -165,6 +166,7 @@
    {<<".*">>,[]}],
   [{"4.3.11",
     [{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
+     {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
    {"4.3.10",
     [{load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
diff --git a/apps/emqx_rule_engine/src/emqx_rule_validator.erl b/apps/emqx_rule_engine/src/emqx_rule_validator.erl
index 237e5dfba..27156cb01 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_validator.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_validator.erl
@@ -33,7 +33,7 @@
 }.
 
 -type data_type() :: string | password | number | boolean
-    | object | array | file | cfgselect.
+    | object | array | file | binary_file | cfgselect.
 
 -type params_spec() :: #{name() => spec()} | any.
 -type params() :: #{binary() => term()}.
@@ -46,6 +46,7 @@
         , object
         , array
         , file
+        , binary_file
         , cfgselect %% TODO: [5.0] refactor this
         ]).
 
@@ -84,8 +85,8 @@ validate_spec(ParamsSepc) ->
 %% Internal Functions
 %%------------------------------------------------------------------------------
 
-validate_value(Val, #{type := Types} = Spec) when is_list(Types) ->
-    validate_types(Val, Types, Spec);
+validate_value(Val, #{type := Union} = Spec) when is_list(Union) ->
+    validate_union(Val, Union, Spec);
 validate_value(Val, #{enum := Enum}) ->
     validate_enum(Val, Enum);
 validate_value(Val, #{type := object} = Spec) ->
@@ -93,17 +94,22 @@ validate_value(Val, #{type := object} = Spec) ->
 validate_value(Val, #{type := Type} = Spec) ->
     validate_type(Val, Type, Spec).
 
-validate_types(Val, [], _Spec) ->
-    throw({invalid_data_type, Val});
-validate_types(Val, [Type | Types], Spec) ->
+validate_union(Val, Union, _Spec) ->
+    do_validate_union(Val, Union, Union, _Spec).
+
+do_validate_union(Val, Union, [], _Spec) ->
+    throw({invalid_data_type, {union, {Val, Union}}});
+do_validate_union(Val, Union, [Type | Types], Spec) ->
     try
         validate_type(Val, Type, Spec)
     catch _:_ ->
-        validate_types(Val, Types, Spec)
+        do_validate_union(Val, Union, Types, Spec)
     end.
 
 validate_type(Val, file, _Spec) ->
     validate_file(Val);
+validate_type(Val, binary_file, _Spec) ->
+    validate_binary_file(Val);
 validate_type(Val, String, Spec) when String =:= string;
                                       String =:= password ->
     validate_string(Val, reg_exp(maps:get(format, Spec, any)));
@@ -118,6 +124,8 @@ validate_type(Val, cfgselect, _Spec) ->
     %% TODO: [5.0] refactor this.
     Val.
 
+validate_enum(Val, BoolEnum) when BoolEnum == [true, false]; BoolEnum == [false, true] ->
+    validate_boolean(Val);
 validate_enum(Val, Enum) ->
     case lists:member(Val, Enum) of
         true -> Val;
@@ -147,6 +155,10 @@ validate_boolean(false) -> false;
 validate_boolean(<<"false">>) -> false;
 validate_boolean(Val) -> throw({invalid_data_type, {boolean, Val}}).
 
+validate_binary_file(#{<<"file">> := _, <<"filename">> := _} = Val) ->
+    Val;
+validate_binary_file(Val) ->
+    throw({invalid_data_type, {binary_file, Val}}).
 validate_file(Val) when is_map(Val) -> Val;
 validate_file(Val) when is_list(Val) -> Val;
 validate_file(Val) when is_binary(Val) -> Val;
@@ -163,6 +175,14 @@ do_validate_spec(Name, #{type := object} = Spec) ->
         fun (not_found) -> throw({required_field_missing, {schema, {in, Name}}});
             (Schema) -> validate_spec(Schema)
         end);
+do_validate_spec(Name, #{type := cfgselect} = Spec) ->
+    find_field(items, Spec,
+        fun (not_found) -> throw({required_field_missing, {items, {in, Name}}});
+            (Items) ->
+                maps:map(fun(_K, Schema) ->
+                        validate_spec(Schema)
+                    end, Items)
+        end);
 do_validate_spec(Name, #{type := array} = Spec) ->
     find_field(items, Spec,
         fun (not_found) -> throw({required_field_missing, {items, {in, Name}}});
diff --git a/apps/emqx_rule_engine/test/emqx_rule_validator_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_validator_SUITE.erl
index a5870ae79..8c7ff9b86 100644
--- a/apps/emqx_rule_engine/test/emqx_rule_validator_SUITE.erl
+++ b/apps/emqx_rule_engine/test/emqx_rule_validator_SUITE.erl
@@ -70,6 +70,32 @@
                 }
             }
         },
+        type_cfgselect => #{
+            type => cfgselect,
+            enum => [<<"upload">>, <<"path">>],
+            default => <<"upload">>,
+            items =>
+                #{
+                  upload =>
+                      #{
+                        kerberos_keytab =>
+                            #{
+                              order => 6,
+                              type => binary_file,
+                              default => #{file => <<"">>, filename => <<"no_keytab.key">>}
+                             }
+                       },
+                  path =>
+                      #{
+                        kerberos_keytab_path =>
+                            #{
+                              order => 7,
+                              type => string,
+                              default => <<"">>
+                             }
+                       }
+                 }
+        },
         type_array => #{
             type => array,
             required => true,
@@ -88,7 +114,7 @@ t_validate_spec_the_complex(_) ->
 t_validate_spec_invalid_1(_) ->
     ?assertThrow({required_field_missing, {type, _}},
         emqx_rule_validator:validate_spec(#{
-            type_enum_number => #{
+            a => #{
                 required => true
             }
         })).
@@ -96,15 +122,23 @@ t_validate_spec_invalid_1(_) ->
 t_validate_spec_invalid_2(_) ->
     ?assertThrow({required_field_missing, {schema, _}},
         emqx_rule_validator:validate_spec(#{
-            type_enum_number => #{
+            a => #{
                 type => object
             }
         })).
 
+t_validate_spec_invalid_2_1(_) ->
+    ?assertThrow({required_field_missing, {items, _}},
+        emqx_rule_validator:validate_spec(#{
+            a => #{
+                type => cfgselect
+            }
+        })).
+
 t_validate_spec_invalid_3(_) ->
     ?assertThrow({required_field_missing, {items, _}},
         emqx_rule_validator:validate_spec(#{
-            type_enum_number => #{
+            a => #{
                 type => array
             }
         })).
@@ -162,6 +196,22 @@ t_validate_params_fill_default(_) ->
     ?assertMatch(#{<<"abc">> := 1, <<"eee">> := <<"hello">>},
         emqx_rule_validator:validate_params(Params, Specs)).
 
+t_validate_params_binary_file(_) ->
+    Params = #{<<"kfile">> => #{<<"file">> => <<"foo">>, <<"filename">> => <<"foo.key">>}},
+    Specs = #{<<"kfile">> => #{
+                type => binary_file,
+                required => true
+             }},
+    ?assertMatch(#{<<"kfile">> := #{<<"file">> := <<"foo">>, <<"filename">> := <<"foo.key">>}},
+        emqx_rule_validator:validate_params(Params, Specs)),
+    Params1 = #{<<"kfile">> => #{<<"file">> => <<"foo">>}},
+    Specs1 = #{<<"kfile">> => #{
+                type => binary_file,
+                required => true
+             }},
+    ?assertThrow({invalid_data_type, {binary_file, #{<<"file">> := <<"foo">>}}},
+        emqx_rule_validator:validate_params(Params1, Specs1)).
+
 t_validate_params_the_complex(_) ->
     Params = #{
         <<"string_required">> => <<"hello">>,
@@ -173,6 +223,8 @@ t_validate_params_the_complex(_) ->
             <<"string_required">> => <<"hello2">>,
             <<"type_number">> => 1.3
         },
+        <<"type_cfgselect">> => <<"upload">>,
+        <<"kerberos_keytab">> => #{<<"file">> => <<"foo">>, <<"filename">> => <<"foo.key">>},
         <<"type_array">> => [<<"ok">>, <<"no">>]
     },
     ?assertMatch(
@@ -186,6 +238,8 @@ t_validate_params_the_complex(_) ->
                 <<"string_required">> := <<"hello2">>,
                 <<"type_number">> := 1.3
             },
+            <<"kerberos_keytab">> := #{<<"file">> := <<"foo">>, <<"filename">> := <<"foo.key">>},
+            <<"type_cfgselect">> := <<"upload">>,
             <<"type_array">> := [<<"ok">>, <<"no">>]
         },
         emqx_rule_validator:validate_params(Params, ?VALID_SPEC)).