From b33b3f6eeef736a75fee6ea30528bd3a0019bcb1 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Mon, 14 Mar 2022 10:21:51 +0800 Subject: [PATCH 1/3] fix: reduce status_and_metrics --- apps/emqx_authn/src/emqx_authn_api.erl | 24 ++++++++++-------- apps/emqx_authn/test/emqx_authn_api_SUITE.erl | 6 ++--- .../emqx_authz/src/emqx_authz_api_sources.erl | 25 +++++++++++-------- .../test/emqx_authz_api_sources_SUITE.erl | 7 +----- 4 files changed, 32 insertions(+), 30 deletions(-) diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index 1619d9239..24de25dc8 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -756,10 +756,13 @@ list_authenticator(ChainName, ConfKeyPath, AuthenticatorID) -> AuthenticatorsConfig = get_raw_config_with_defaults(ConfKeyPath), case find_config(AuthenticatorID, AuthenticatorsConfig) of {ok, AuthenticatorConfig} -> - StatusAndMetrics = lookup_from_all_nodes(ChainName, AuthenticatorID), - Fun = fun ({Key, Val}, Map) -> maps:put(Key, Val, Map) end, - AppendList = [{id, AuthenticatorID}, {status_and_metrics, StatusAndMetrics}], - {200, lists:foldl(Fun, convert_certs(AuthenticatorConfig), AppendList)}; + case lookup_from_all_nodes(ChainName, AuthenticatorID) of + {ok, StatusAndMetrics} -> + Fun = fun ({Key, Val}, Map) -> maps:put(Key, Val, Map) end, + AppendList = [{id, AuthenticatorID} | maps:to_list(StatusAndMetrics)], + {200, lists:foldl(Fun, convert_certs(AuthenticatorConfig), AppendList)}; + {error, ErrorMsg} -> {500, ErrorMsg} + end; {error, Reason} -> serialize_error(Reason) end. @@ -797,14 +800,15 @@ lookup_from_all_nodes(ChainName, AuthenticatorID) -> AggregateStatus = aggregate_status(maps:values(StatusMap)), AggregateMetrics = aggregate_metrics(maps:values(MetricsMap)), Fun = fun(_, V1) -> restructure_map(V1) end, - #{node_status => StatusMap, - node_metrics => maps:map(Fun, MetricsMap), - node_error => ErrorMap, - status => AggregateStatus, - metrics => restructure_map(AggregateMetrics) + {ok, #{node_status => StatusMap, + node_metrics => maps:map(Fun, MetricsMap), + node_error => ErrorMap, + status => AggregateStatus, + metrics => restructure_map(AggregateMetrics) + } }; {error, ErrL} -> - {error_msg('INTERNAL_ERROR', ErrL)} + {error, error_msg('INTERNAL_ERROR', ErrL)} end. aggregate_status([]) -> error_some_strange_happen; diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index cc6476f0b..b0aced062 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -183,15 +183,13 @@ test_authenticator(PathPrefix) -> {<<"rate_max">>, 0.0}, {<<"success">>, 0}], EqualFun = fun ({M, V}) -> - ?assertEqual(V, LookFun([<<"status_and_metrics">>, - <<"metrics">>, + ?assertEqual(V, LookFun([<<"metrics">>, M] ) ) end, lists:map(EqualFun, MetricsList), ?assertEqual(<<"connected">>, - LookFun([<<"status_and_metrics">>, - <<"status">> + LookFun([<<"status">> ])), {ok, 404, _} = request( get, diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 54d2f181c..aed536bd1 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -230,9 +230,13 @@ source(get, #{bindings := #{type := Type}}) -> [Source] -> case emqx_authz:lookup(Type) of #{annotations := #{id := ResourceId }} -> - StatusAndMetrics = lookup_from_all_nodes(ResourceId), - {200, maps:put(status_and_metrics, StatusAndMetrics, read_certs(Source))}; - _ -> {200, maps:put(status_and_metrics, resource_not_found, read_certs(Source))} + case lookup_from_all_nodes(ResourceId) of + {ok, StatusAndMetrics} -> + Fun = fun ({Key, Val}, Map) -> maps:put(Key, Val, Map) end, + {200, lists:foldl(Fun, read_certs(Source), maps:to_list(StatusAndMetrics))}; + {error, ErrorMsg} -> {500, ErrorMsg} + end; + _ -> {200, read_certs(Source)} end end; source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>, @@ -292,14 +296,15 @@ lookup_from_all_nodes(ResourceId) -> AggregateStatus = aggregate_status(maps:values(StatusMap)), AggregateMetrics = aggregate_metrics(maps:values(MetricsMap)), Fun = fun(_, V1) -> restructure_map(V1) end, - #{node_status => StatusMap, - node_metrics => maps:map(Fun, MetricsMap), - node_error => ErrorMap, - status => AggregateStatus, - metrics => restructure_map(AggregateMetrics) - }; + {ok, #{node_status => StatusMap, + node_metrics => maps:map(Fun, MetricsMap), + node_error => ErrorMap, + status => AggregateStatus, + metrics => restructure_map(AggregateMetrics) + } + }; {error, ErrL} -> - {error_msg('INTERNAL_ERROR', ErrL)} + {error, error_msg('INTERNAL_ERROR', ErrL)} end. aggregate_status([]) -> error_some_strange_happen; diff --git a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl index e3cb5254a..608090640 100644 --- a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl @@ -186,8 +186,7 @@ t_api(_) -> EqualFun = fun (RList) -> fun ({M, V}) -> ?assertEqual(V, - LookupVal([<<"status_and_metrics">>, - <<"metrics">>, M], + LookupVal([<<"metrics">>, M], RList) ) end @@ -218,10 +217,6 @@ t_api(_) -> {ok, 204, _} = request(put, uri(["authorization", "sources", "http"]), ?SOURCE1#{<<"enable">> := false}), {ok, 200, Result3} = request(get, uri(["authorization", "sources", "http"]), []), - {ok, RList3} = emqx_json:safe_decode(Result3), - ?assertEqual(<<"resource_not_found">>, - LookupVal([<<"status_and_metrics">> - ], RList3)), ?assertMatch(#{<<"type">> := <<"http">>, <<"enable">> := false}, jsx:decode(Result3)), Keyfile = emqx_common_test_helpers:app_path( From 740b3870bf035513dfec205297c4f478777df739 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Mon, 14 Mar 2022 10:38:03 +0800 Subject: [PATCH 2/3] fix(elvis): nesting_level shouldn't exceed 6 --- apps/emqx_authz/src/emqx_authz_api_sources.erl | 4 +++- elvis.config | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index aed536bd1..73f28176b 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -233,7 +233,9 @@ source(get, #{bindings := #{type := Type}}) -> case lookup_from_all_nodes(ResourceId) of {ok, StatusAndMetrics} -> Fun = fun ({Key, Val}, Map) -> maps:put(Key, Val, Map) end, - {200, lists:foldl(Fun, read_certs(Source), maps:to_list(StatusAndMetrics))}; + {200, lists:foldl(Fun, + read_certs(Source), + maps:to_list(StatusAndMetrics))}; {error, ErrorMsg} -> {500, ErrorMsg} end; _ -> {200, read_certs(Source)} diff --git a/elvis.config b/elvis.config index dff44c31d..f2e5b90b2 100644 --- a/elvis.config +++ b/elvis.config @@ -30,7 +30,8 @@ {elvis_text_style, line_length, #{ limit => 100 , skip_comments => false }}, - {elvis_style, dont_repeat_yourself, #{ min_complexity => 100 }} + {elvis_style, dont_repeat_yourself, #{ min_complexity => 100 }}, + {elvis_style, nesting_level, #{ level => 6 }} ] }, #{dirs => ["."], From 258d2e9e03531d9bf0c1cd7d5d039b21c372629d Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Mon, 14 Mar 2022 14:20:25 +0800 Subject: [PATCH 3/3] fix(emqx_authz_api_sources): use merge replace foldl --- apps/emqx_authz/src/emqx_authz_api_sources.erl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 73f28176b..731c5d40f 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -232,10 +232,7 @@ source(get, #{bindings := #{type := Type}}) -> #{annotations := #{id := ResourceId }} -> case lookup_from_all_nodes(ResourceId) of {ok, StatusAndMetrics} -> - Fun = fun ({Key, Val}, Map) -> maps:put(Key, Val, Map) end, - {200, lists:foldl(Fun, - read_certs(Source), - maps:to_list(StatusAndMetrics))}; + {200, maps:merge(read_certs(Source), StatusAndMetrics)}; {error, ErrorMsg} -> {500, ErrorMsg} end; _ -> {200, read_certs(Source)}