From 49f5325c6768eae83d0e6e398a05e74328b234a8 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Wed, 26 Apr 2023 22:45:24 +0300 Subject: [PATCH] feat(tpl): unify validations / errors var representations --- .../src/emqx_authz/emqx_authz_rule.erl | 2 +- .../test/emqx_authz/emqx_authz_rule_SUITE.erl | 10 ++-- apps/emqx_auth_http/src/emqx_authz_http.erl | 18 ++++---- .../src/emqx_authz_mongodb.erl | 10 ++-- apps/emqx_auth_mysql/src/emqx_authz_mysql.erl | 10 ++-- .../src/emqx_authz_postgresql.erl | 10 ++-- apps/emqx_auth_redis/src/emqx_authz_redis.erl | 10 ++-- .../src/emqx_connector_template.erl | 46 +++++++++---------- .../src/emqx_connector_template_sql.erl | 6 +-- .../test/emqx_connector_template_SUITE.erl | 40 ++++++++-------- .../src/emqx_rule_actions.erl | 4 +- 11 files changed, 81 insertions(+), 85 deletions(-) diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl index 9cf79ba88..6f5369aec 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl @@ -184,7 +184,7 @@ compile_topic({eq, Topic}) -> {eq, emqx_topic:words(bin(Topic))}; compile_topic(Topic) -> Template = emqx_connector_template:parse(Topic), - ok = emqx_connector_template:validate([<>, <>], Template), + ok = emqx_connector_template:validate([?VAR_USERNAME, ?VAR_CLIENTID], Template), case emqx_connector_template:trivial(Template) of true -> emqx_topic:words(bin(Topic)); false -> {pattern, Template} diff --git a/apps/emqx_auth/test/emqx_authz/emqx_authz_rule_SUITE.erl b/apps/emqx_auth/test/emqx_authz/emqx_authz_rule_SUITE.erl index bca21cd8d..5031daff6 100644 --- a/apps/emqx_auth/test/emqx_authz/emqx_authz_rule_SUITE.erl +++ b/apps/emqx_auth/test/emqx_authz/emqx_authz_rule_SUITE.erl @@ -78,13 +78,13 @@ t_compile(_) -> emqx_authz_rule:compile({allow, {ipaddr, "127.0.0.1"}, all, [{eq, "#"}, {eq, "+"}]}) ), - ?assertEqual( + ?assertMatch( {allow, {ipaddrs, [ {{127, 0, 0, 1}, {127, 0, 0, 1}, 32}, {{192, 168, 1, 0}, {192, 168, 1, 255}, 24} ]}, - subscribe, [{pattern, [{var, [<<"clientid">>]}]}]}, + subscribe, [{pattern, [{var, "clientid", [_]}]}]}, emqx_authz_rule:compile( {allow, {ipaddrs, ["127.0.0.1", "192.168.1.0/24"]}, subscribe, [?PH_S_CLIENTID]} ) @@ -106,7 +106,7 @@ t_compile(_) -> {clientid, {re_pattern, _, _, _, _}} ]}, publish, [ - {pattern, [{var, [<<"username">>]}]}, {pattern, [{var, [<<"clientid">>]}]} + {pattern, [{var, "username", [_]}]}, {pattern, [{var, "clientid", [_]}]} ]}, emqx_authz_rule:compile( {allow, @@ -118,9 +118,9 @@ t_compile(_) -> ) ), - ?assertEqual( + ?assertMatch( {allow, {username, {eq, <<"test">>}}, publish, [ - {pattern, [<<"t/foo">>, {var, [<<"username">>]}, <<"boo">>]} + {pattern, [<<"t/foo">>, {var, "username", [_]}, <<"boo">>]} ]}, emqx_authz_rule:compile({allow, {username, "test"}, publish, ["t/foo${username}boo"]}) ), diff --git a/apps/emqx_auth_http/src/emqx_authz_http.erl b/apps/emqx_auth_http/src/emqx_authz_http.erl index 2ab76f305..bbb2bf9b5 100644 --- a/apps/emqx_auth_http/src/emqx_authz_http.erl +++ b/apps/emqx_auth_http/src/emqx_authz_http.erl @@ -39,15 +39,15 @@ -endif. -define(PLACEHOLDERS, [ - <>, - <>, - <>, - <>, - <>, - <>, - <>, - <>, - <> + ?VAR_USERNAME, + ?VAR_CLIENTID, + ?VAR_PEERHOST, + ?VAR_PROTONAME, + ?VAR_MOUNTPOINT, + ?VAR_TOPIC, + ?VAR_ACTION, + ?VAR_CERT_SUBJECT, + ?VAR_CERT_CN_NAME ]). -define(PLACEHOLDERS_FOR_RICH_ACTIONS, [ diff --git a/apps/emqx_auth_mongodb/src/emqx_authz_mongodb.erl b/apps/emqx_auth_mongodb/src/emqx_authz_mongodb.erl index 97a5fa3a6..35ac3a41b 100644 --- a/apps/emqx_auth_mongodb/src/emqx_authz_mongodb.erl +++ b/apps/emqx_auth_mongodb/src/emqx_authz_mongodb.erl @@ -36,11 +36,11 @@ -endif. -define(PLACEHOLDERS, [ - <>, - <>, - <>, - <>, - <> + ?VAR_USERNAME, + ?VAR_CLIENTID, + ?VAR_PEERHOST, + ?VAR_CERT_CN_NAME, + ?VAR_CERT_SUBJECT ]). description() -> diff --git a/apps/emqx_auth_mysql/src/emqx_authz_mysql.erl b/apps/emqx_auth_mysql/src/emqx_authz_mysql.erl index e87d2afa2..a6d71d1ca 100644 --- a/apps/emqx_auth_mysql/src/emqx_authz_mysql.erl +++ b/apps/emqx_auth_mysql/src/emqx_authz_mysql.erl @@ -38,11 +38,11 @@ -endif. -define(PLACEHOLDERS, [ - <>, - <>, - <>, - <>, - <> + ?VAR_USERNAME, + ?VAR_CLIENTID, + ?VAR_PEERHOST, + ?VAR_CERT_CN_NAME, + ?VAR_CERT_SUBJECT ]). description() -> diff --git a/apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl b/apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl index 645fff293..b538bd95e 100644 --- a/apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl +++ b/apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl @@ -38,11 +38,11 @@ -endif. -define(PLACEHOLDERS, [ - <>, - <>, - <>, - <>, - <> + ?VAR_USERNAME, + ?VAR_CLIENTID, + ?VAR_PEERHOST, + ?VAR_CERT_CN_NAME, + ?VAR_CERT_SUBJECT ]). description() -> diff --git a/apps/emqx_auth_redis/src/emqx_authz_redis.erl b/apps/emqx_auth_redis/src/emqx_authz_redis.erl index 7ac893da1..eb63804b9 100644 --- a/apps/emqx_auth_redis/src/emqx_authz_redis.erl +++ b/apps/emqx_auth_redis/src/emqx_authz_redis.erl @@ -36,11 +36,11 @@ -endif. -define(PLACEHOLDERS, [ - <>, - <>, - <>, - <>, - <> + ?VAR_CERT_CN_NAME, + ?VAR_CERT_SUBJECT, + ?VAR_PEERHOST, + ?VAR_CLIENTID, + ?VAR_USERNAME ]). description() -> diff --git a/apps/emqx_connector/src/emqx_connector_template.erl b/apps/emqx_connector/src/emqx_connector_template.erl index bb26edec1..221cc5e86 100644 --- a/apps/emqx_connector/src/emqx_connector_template.erl +++ b/apps/emqx_connector/src/emqx_connector_template.erl @@ -37,6 +37,7 @@ -export_type([str/0]). -export_type([deep/0]). -export_type([placeholder/0]). +-export_type([varname/0]). -export_type([bindings/0]). -type t() :: str() | {'$tpl', deeptpl()}. @@ -55,8 +56,9 @@ | port() | reference(). --type placeholder() :: {var, var()}. --type var() :: _Name :: [binary()]. +-type placeholder() :: {var, varname(), accessor()}. +-type accessor() :: [binary()]. +-type varname() :: string(). -type scalar() :: atom() | unicode:chardata() | number(). -type binding() :: scalar() | list(scalar()) | bindings(). @@ -64,7 +66,7 @@ -type var_trans() :: fun((Value :: term()) -> unicode:chardata()) - | fun((var(), Value :: term()) -> unicode:chardata()). + | fun((varname(), Value :: term()) -> unicode:chardata()). -type parse_opts() :: #{ strip_double_quote => boolean() @@ -103,7 +105,7 @@ parse(String, Opts) -> parse_split([Part, _PH, <<>>, Var]) -> % Regular placeholder - prepend(Part, [{var, parse_var(Var)}]); + prepend(Part, [{var, unicode:characters_to_list(Var), parse_accessor(Var)}]); parse_split([Part, _PH = <>, <<"$">>, _]) -> % Escaped literal, take all but the second byte, which is always `$`. % Important to make a whole token starting with `$` so the `unparse/11` @@ -117,7 +119,7 @@ prepend(<<>>, To) -> prepend(Head, To) -> [Head | To]. -parse_var(Var) -> +parse_accessor(Var) -> case string:split(Var, <<".">>, all) of [<<>>] -> ?PH_VAR_THIS; @@ -126,10 +128,9 @@ parse_var(Var) -> Name end. --spec validate([var() | binary()], t()) -> - ok | {error, [_Error :: {var(), disallowed}]}. -validate(AllowedIn, Template) -> - Allowed = [try_parse_var(V) || V <- AllowedIn], +-spec validate([varname()], t()) -> + ok | {error, [_Error :: {varname(), disallowed}]}. +validate(Allowed, Template) -> {_, Errors} = render(Template, #{}), {Used, _} = lists:unzip(Errors), case lists:usort(Used) -- Allowed of @@ -139,11 +140,6 @@ validate(AllowedIn, Template) -> {error, [{Var, disallowed} || Var <- Disallowed]} end. -try_parse_var(Var) when is_binary(Var) -> - parse_var(Var); -try_parse_var(Name) when is_list(Name) -> - Name. - -spec trivial(t()) -> boolean(). trivial(Template) -> @@ -156,7 +152,7 @@ unparse({'$tpl', Template}) -> unparse(Template) -> unicode:characters_to_list(lists:map(fun unparse_part/1, Template)). -unparse_part({var, Name}) -> +unparse_part({var, Name, _Accessor}) -> render_placeholder(Name); unparse_part(Part = <<"${", _/binary>>) -> <<"$", Part/binary>>; @@ -164,7 +160,7 @@ unparse_part(Part) -> Part. render_placeholder(Name) -> - "${" ++ lists:join($., Name) ++ "}". + "${" ++ Name ++ "}". %% @doc Render a template with given bindings. %% Returns a term with all placeholders replaced with values from bindings. @@ -172,17 +168,17 @@ render_placeholder(Name) -> %% By default, all binding values are converted to strings using `to_string/1` %% function. Option `var_trans` can be used to override this behaviour. -spec render(t(), bindings()) -> - {term(), [_Error :: {var(), undefined}]}. + {term(), [_Error :: {varname(), undefined}]}. render(Template, Bindings) -> render(Template, Bindings, #{}). -spec render(t(), bindings(), render_opts()) -> - {term(), [_Error :: {var(), undefined}]}. + {term(), [_Error :: {varname(), undefined}]}. render(Template, Bindings, Opts) when is_list(Template) -> lists:mapfoldl( fun - ({var, Name}, EAcc) -> - {String, Errors} = render_binding(Name, Bindings, Opts), + ({var, Name, Accessor}, EAcc) -> + {String, Errors} = render_binding(Name, Accessor, Bindings, Opts), {String, Errors ++ EAcc}; (String, EAcc) -> {String, EAcc} @@ -193,8 +189,8 @@ render(Template, Bindings, Opts) when is_list(Template) -> render({'$tpl', Template}, Bindings, Opts) -> render_deep(Template, Bindings, Opts). -render_binding(Name, Bindings, Opts) -> - case lookup_var(Name, Bindings) of +render_binding(Name, Accessor, Bindings, Opts) -> + case lookup_var(Accessor, Bindings) of {ok, Value} -> {render_value(Name, Value, Opts), []}; {error, Reason} -> @@ -231,12 +227,12 @@ render_strict(Template, Bindings, Opts) -> %% lists are not analyzed for "printability" and are treated as nested terms. %% The result is a usual template, and can be fed to other functions in this %% module. --spec parse_deep(unicode:chardata()) -> +-spec parse_deep(term()) -> t(). parse_deep(Term) -> parse_deep(Term, #{}). --spec parse_deep(unicode:chardata(), parse_opts()) -> +-spec parse_deep(term(), parse_opts()) -> t(). parse_deep(Term, Opts) -> {'$tpl', parse_deep_term(Term, Opts)}. @@ -305,7 +301,7 @@ unparse_deep(Term) -> %% --spec lookup_var(var(), bindings()) -> +-spec lookup_var(accessor(), bindings()) -> {ok, binding()} | {error, undefined}. lookup_var(Var, Value) when Var == ?PH_VAR_THIS orelse Var == [] -> {ok, Value}; diff --git a/apps/emqx_connector/src/emqx_connector_template_sql.erl b/apps/emqx_connector/src/emqx_connector_template_sql.erl index 0febfe575..e95ecde42 100644 --- a/apps/emqx_connector/src/emqx_connector_template_sql.erl +++ b/apps/emqx_connector/src/emqx_connector_template_sql.erl @@ -88,14 +88,14 @@ render_strict(Template, Bindings, Opts) -> %% #{parameters => '$n'} %% ), %% Statement = <<"INSERT INTO table (id, name, age) VALUES ($1, $2, 42)">>, -%% RowTemplate = [{var, [...]}, ...] +%% RowTemplate = [{var, "...", [...]}, ...] %% ``` -spec parse_prepstmt(unicode:chardata(), parse_opts()) -> {unicode:chardata(), row_template()}. parse_prepstmt(String, Opts) -> Template = emqx_connector_template:parse(String, maps:with(?TEMPLATE_PARSE_OPTS, Opts)), Statement = mk_prepared_statement(Template, Opts), - Placeholders = [Placeholder || Placeholder = {var, _} <- Template], + Placeholders = [Placeholder || Placeholder <- Template, element(1, Placeholder) == var], {Statement, Placeholders}. mk_prepared_statement(Template, Opts) -> @@ -103,7 +103,7 @@ mk_prepared_statement(Template, Opts) -> {Statement, _} = lists:mapfoldl( fun - ({var, _}, Acc) -> + (Var, Acc) when element(1, Var) == var -> mk_replace(ParameterFormat, Acc); (String, Acc) -> {String, Acc} diff --git a/apps/emqx_connector/test/emqx_connector_template_SUITE.erl b/apps/emqx_connector/test/emqx_connector_template_SUITE.erl index 666fbfa58..998baae37 100644 --- a/apps/emqx_connector/test/emqx_connector_template_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_template_SUITE.erl @@ -47,7 +47,7 @@ t_render_var_trans(_) -> {String, Errors} = emqx_connector_template:render( Template, Bindings, - #{var_trans => fun(Name, _) -> "<" ++ lists:join($., Name) ++ ">" end} + #{var_trans => fun(Name, _) -> "<" ++ Name ++ ">" end} ), ?assertEqual( {<<"a:,b:,c:">>, []}, @@ -59,7 +59,7 @@ t_render_path(_) -> Template = emqx_connector_template:parse(<<"d.d1:${d.d1}">>), ?assertEqual( ok, - emqx_connector_template:validate([<<"d.d1">>], Template) + emqx_connector_template:validate(["d.d1"], Template) ), ?assertEqual( {<<"d.d1:hi">>, []}, @@ -70,8 +70,8 @@ t_render_custom_ph(_) -> Bindings = #{a => <<"a">>, b => <<"b">>}, Template = emqx_connector_template:parse(<<"a:${a},b:${b}">>), ?assertEqual( - {error, [{[<<"b">>], disallowed}]}, - emqx_connector_template:validate([<<"a">>], Template) + {error, [{"b", disallowed}]}, + emqx_connector_template:validate(["a"], Template) ), ?assertEqual( <<"a:a,b:b">>, @@ -81,7 +81,7 @@ t_render_custom_ph(_) -> t_render_this(_) -> Bindings = #{a => <<"a">>, b => [1, 2, 3]}, Template = emqx_connector_template:parse(<<"this:${} / also:${.}">>), - ?assertEqual(ok, emqx_connector_template:validate([?PH_VAR_THIS], Template)), + ?assertEqual(ok, emqx_connector_template:validate(["."], Template)), ?assertEqual( % NOTE: order of the keys in the JSON object depends on the JSON encoder <<"this:{\"b\":[1,2,3],\"a\":\"a\"} / also:{\"b\":[1,2,3],\"a\":\"a\"}">>, @@ -95,21 +95,21 @@ t_render_missing_bindings(_) -> ), ?assertEqual( {<<"a:,b:,c:,d:,e:">>, [ - {[<<"no">>, <<"such_atom_i_swear">>], undefined}, - {[<<"d">>, <<"d1">>], undefined}, - {[<<"c">>], undefined}, - {[<<"b">>], undefined}, - {[<<"a">>], undefined} + {"no.such_atom_i_swear", undefined}, + {"d.d1", undefined}, + {"c", undefined}, + {"b", undefined}, + {"a", undefined} ]}, render_string(Template, Bindings) ), ?assertError( [ - {[<<"no">>, <<"such_atom_i_swear">>], undefined}, - {[<<"d">>, <<"d1">>], undefined}, - {[<<"c">>], undefined}, - {[<<"b">>], undefined}, - {[<<"a">>], undefined} + {"no.such_atom_i_swear", undefined}, + {"d.d1", undefined}, + {"c", undefined}, + {"b", undefined}, + {"a", undefined} ], render_strict_string(Template, Bindings) ). @@ -256,10 +256,10 @@ t_render_cql(_) -> t_render_sql_custom_ph(_) -> {PrepareStatement, RowTemplate} = - emqx_connector_template_sql:parse_prepstmt(<<"a:${a},b:${b}">>, #{parameters => '$n'}), + emqx_connector_template_sql:parse_prepstmt(<<"a:${a},b:${b.c}">>, #{parameters => '$n'}), ?assertEqual( - {error, [{[<<"b">>], disallowed}]}, - emqx_connector_template:validate([<<"a">>], RowTemplate) + {error, [{"b.c", disallowed}]}, + emqx_connector_template:validate(["a"], RowTemplate) ), ?assertEqual(<<"a:$1,b:$2">>, bin(PrepareStatement)). @@ -296,8 +296,8 @@ t_render_tmpl_deep(_) -> ), ?assertEqual( - {error, [{V, disallowed} || V <- [[<<"b">>], [<<"c">>]]]}, - emqx_connector_template:validate([<<"a">>], Template) + {error, [{V, disallowed} || V <- ["b", "c"]]}, + emqx_connector_template:validate(["a"], Template) ), ?assertEqual( diff --git a/apps/emqx_rule_engine/src/emqx_rule_actions.erl b/apps/emqx_rule_engine/src/emqx_rule_actions.erl index bb9966b4a..fa677ce78 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_actions.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_actions.erl @@ -224,8 +224,8 @@ parse_user_properties(_) -> %% invalid, discard undefined. -render_simple_var([{var, Name}], Data, Default) -> - case emqx_connector_template:lookup_var(Name, Data) of +render_simple_var([{var, _Name, Accessor}], Data, Default) -> + case emqx_connector_template:lookup_var(Accessor, Data) of {ok, Var} -> Var; %% cannot find the variable from Data {error, _} -> Default