From 8b6eeef7fce73674295302227eb9c1bd59988b5c Mon Sep 17 00:00:00 2001 From: Zaiming Shi Date: Fri, 24 Sep 2021 23:06:54 +0200 Subject: [PATCH] refactor(authz): use macro for cmd names --- apps/emqx_authz/include/emqx_authz.hrl | 6 +++ apps/emqx_authz/src/emqx_authz.erl | 40 +++++++++---------- .../emqx_authz/src/emqx_authz_api_sources.erl | 8 ++-- apps/emqx_authz/test/emqx_authz_SUITE.erl | 32 +++++++-------- 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/apps/emqx_authz/include/emqx_authz.hrl b/apps/emqx_authz/include/emqx_authz.hrl index 8ef8899b0..bf8371add 100644 --- a/apps/emqx_authz/include/emqx_authz.hrl +++ b/apps/emqx_authz/include/emqx_authz.hrl @@ -49,6 +49,12 @@ ignore = 'client.authorize.ignore' }). +-define(CMD_REPLCAE, replace). +-define(CMD_DELETE, delete). +-define(CMD_PREPEND, prepend). +-define(CMD_APPEND, append). +-define(CMD_MOVE, move). + -define(METRICS(Type), tl(tuple_to_list(#Type{}))). -define(METRICS(Type, K), #Type{}#Type.K). diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 3ca5dddb3..7a417925e 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -64,50 +64,50 @@ move(Type, Cmd) -> move(Type, Cmd, #{}). move(Type, #{<<"before">> := Before}, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {move, type(Type), #{<<"before">> => type(Before)}}, Opts); + emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), #{<<"before">> => type(Before)}}, Opts); move(Type, #{<<"after">> := After}, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {move, type(Type), #{<<"after">> => type(After)}}, Opts); + emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), #{<<"after">> => type(After)}}, Opts); move(Type, Position, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {move, type(Type), Position}, Opts). + emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), Position}, Opts). update(Cmd, Sources) -> update(Cmd, Sources, #{}). -update({replace_once, Type}, Sources, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {{replace_once, type(Type)}, Sources}, Opts); -update({delete_once, Type}, Sources, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {{delete_once, type(Type)}, Sources}, Opts); +update({replace, Type}, Sources, Opts) -> + emqx:update_config(?CONF_KEY_PATH, {{replace, type(Type)}, Sources}, Opts); +update({delete, Type}, Sources, Opts) -> + emqx:update_config(?CONF_KEY_PATH, {{delete, type(Type)}, Sources}, Opts); update(Cmd, Sources, Opts) -> emqx:update_config(?CONF_KEY_PATH, {Cmd, Sources}, Opts). -do_update({move, Type, <<"top">>}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, <<"top">>}, Conf) when is_list(Conf) -> {Source, Front, Rear} = take(Type, Conf), [Source | Front] ++ Rear; -do_update({move, Type, <<"bottom">>}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, <<"bottom">>}, Conf) when is_list(Conf) -> {Source, Front, Rear} = take(Type, Conf), Front ++ Rear ++ [Source]; -do_update({move, Type, #{<<"before">> := Before}}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, #{<<"before">> := Before}}, Conf) when is_list(Conf) -> {S1, Front1, Rear1} = take(Type, Conf), {S2, Front2, Rear2} = take(Before, Front1 ++ Rear1), Front2 ++ [S1, S2] ++ Rear2; -do_update({move, Type, #{<<"after">> := After}}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, #{<<"after">> := After}}, Conf) when is_list(Conf) -> {S1, Front1, Rear1} = take(Type, Conf), {S2, Front2, Rear2} = take(After, Front1 ++ Rear1), Front2 ++ [S2, S1] ++ Rear2; -do_update({head, Sources}, Conf) when is_list(Sources), is_list(Conf) -> +do_update({?CMD_PREPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) -> NConf = Sources ++ Conf, ok = check_dup_types(NConf), NConf; -do_update({tail, Sources}, Conf) when is_list(Sources), is_list(Conf) -> +do_update({?CMD_APPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) -> NConf = Conf ++ Sources, ok = check_dup_types(NConf), NConf; -do_update({{replace_once, Type}, Source}, Conf) when is_map(Source), is_list(Conf) -> +do_update({{replace, Type}, Source}, Conf) when is_map(Source), is_list(Conf) -> {_Old, Front, Rear} = take(Type, Conf), NConf = Front ++ [Source | Rear], ok = check_dup_types(NConf), NConf; -do_update({{delete_once, Type}, _Source}, Conf) when is_list(Conf) -> +do_update({{delete, Type}, _Source}, Conf) when is_list(Conf) -> {_Old, Front, Rear} = take(Type, Conf), NConf = Front ++ Rear, NConf; @@ -125,27 +125,27 @@ post_config_update(Cmd, NewSources, _OldSource, _AppEnvs) -> ok = do_post_update(Cmd, NewSources), ok = emqx_authz_cache:drain_cache(). -do_post_update({move, _Type, _Where} = Cmd, _NewSources) -> +do_post_update({?CMD_MOVE, _Type, _Where} = Cmd, _NewSources) -> InitedSources = lookup(), MovedSources = do_update(Cmd, InitedSources), ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [MovedSources]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update({head, Sources}, _NewSources) -> +do_post_update({?CMD_PREPEND, Sources}, _NewSources) -> InitedSources = init_sources(check_sources(Sources)), ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [InitedSources ++ lookup()]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update({tail, Sources}, _NewSources) -> +do_post_update({?CMD_APPEND, Sources}, _NewSources) -> InitedSources = init_sources(check_sources(Sources)), emqx_hooks:put('client.authorize', {?MODULE, authorize, [lookup() ++ InitedSources]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update({{replace_once, Type}, #{type := Type} = Source}, _NewSources) when is_map(Source) -> +do_post_update({{replace, Type}, #{type := Type} = Source}, _NewSources) when is_map(Source) -> OldInitedSources = lookup(), {OldSource, Front, Rear} = take(Type, OldInitedSources), ok = ensure_resource_deleted(OldSource), InitedSources = init_sources(check_sources([Source])), ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Front ++ InitedSources ++ Rear]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update({{delete_once, Type}, _Source}, _NewSources) -> +do_post_update({{delete, Type}, _Source}, _NewSources) -> OldInitedSources = lookup(), {OldSource, Front, Rear} = take(Type, OldInitedSources), ok = ensure_resource_deleted(OldSource), diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 1d053d442..e3dc36003 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -353,7 +353,7 @@ sources(put, #{body := Body}) when is_list(Body) -> _ -> write_cert(Source) end end || Source <- Body], - update_config(replace, NBody). + update_config(?CMD_REPLCAE, NBody). source(get, #{bindings := #{type := Type}}) -> case get_raw_source(Type) of @@ -375,16 +375,16 @@ source(get, #{bindings := #{type := Type}}) -> end; source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable}}) -> {ok, Filename} = write_file(maps:get(path, emqx_authz:lookup(file), ""), Rules), - case emqx_authz:update({replace_once, file}, #{type => file, enable => Enable, path => Filename}) of + case emqx_authz:update({?CMD_REPLCAE, file}, #{type => file, enable => Enable, path => Filename}) of {ok, _} -> {204}; {error, Reason} -> {400, #{code => <<"BAD_REQUEST">>, message => bin(Reason)}} end; source(put, #{bindings := #{type := Type}, body := Body}) when is_map(Body) -> - update_config({replace_once, Type}, write_cert(Body)); + update_config({?CMD_REPLCAE, Type}, write_cert(Body)); source(delete, #{bindings := #{type := Type}}) -> - update_config({delete_once, Type}, #{}). + update_config({?CMD_DELETE, Type}, #{}). move_source(post, #{bindings := #{type := Type}, body := #{<<"position">> := Position}}) -> case emqx_authz:move(Type, Position) of diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index bfdc131a0..16bf39d49 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_SUITE.erl @@ -50,14 +50,14 @@ init_per_suite(Config) -> Config. end_per_suite(_Config) -> - {ok, _} = emqx_authz:update(replace, []), + {ok, _} = emqx_authz:update(?CMD_REPLCAE, []), emqx_ct_helpers:stop_apps([emqx_authz, emqx_resource]), meck:unload(emqx_resource), meck:unload(emqx_schema), ok. init_per_testcase(_, Config) -> - {ok, _} = emqx_authz:update(replace, []), + {ok, _} = emqx_authz:update(?CMD_REPLCAE, []), Config. -define(SOURCE1, #{<<"type">> => <<"http">>, @@ -120,12 +120,12 @@ init_per_testcase(_, Config) -> %%------------------------------------------------------------------------------ t_update_source(_) -> - {ok, _} = emqx_authz:update(replace, [?SOURCE3]), - {ok, _} = emqx_authz:update(head, [?SOURCE2]), - {ok, _} = emqx_authz:update(head, [?SOURCE1]), - {ok, _} = emqx_authz:update(tail, [?SOURCE4]), - {ok, _} = emqx_authz:update(tail, [?SOURCE5]), - {ok, _} = emqx_authz:update(tail, [?SOURCE6]), + {ok, _} = emqx_authz:update(?CMD_REPLCAE, [?SOURCE3]), + {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE2]), + {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE1]), + {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE4]), + {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE5]), + {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE6]), ?assertMatch([ #{type := http, enable := true} , #{type := mongodb, enable := true} @@ -135,12 +135,12 @@ t_update_source(_) -> , #{type := file, enable := true} ], emqx:get_config([authorization, sources], [])), - {ok, _} = emqx_authz:update({replace_once, http}, ?SOURCE1#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({replace_once, mongodb}, ?SOURCE2#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({replace_once, mysql}, ?SOURCE3#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({replace_once, postgresql}, ?SOURCE4#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({replace_once, redis}, ?SOURCE5#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({replace_once, file}, ?SOURCE6#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, http}, ?SOURCE1#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, mongodb}, ?SOURCE2#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, mysql}, ?SOURCE3#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, postgresql}, ?SOURCE4#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, redis}, ?SOURCE5#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLCAE, file}, ?SOURCE6#{<<"enable">> := false}), ?assertMatch([ #{type := http, enable := false} , #{type := mongodb, enable := false} @@ -150,10 +150,10 @@ t_update_source(_) -> , #{type := file, enable := false} ], emqx:get_config([authorization, sources], [])), - {ok, _} = emqx_authz:update(replace, []). + {ok, _} = emqx_authz:update(?CMD_REPLCAE, []). t_move_source(_) -> - {ok, _} = emqx_authz:update(replace, [?SOURCE1, ?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), + {ok, _} = emqx_authz:update(?CMD_REPLCAE, [?SOURCE1, ?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), ?assertMatch([ #{type := http} , #{type := mongodb} , #{type := mysql}