Merge pull request #11204 from HJianBo/avoid-case-clause

Allow the `*/*` or multiple `Accept` values in header for the `/configs`  API

apps/emqx_management/src/emqx_management.app.src

@@ -2,7 +2,7 @@
 {application, emqx_management, [
     {description, "EMQX Management API and CLI"},
     % strict semver, bump manually!
-    {vsn, "5.0.25"},
+    {vsn, "5.0.26"},
     {modules, []},
     {registered, [emqx_management_sup]},
     {applications, [kernel, stdlib, emqx_plugins, minirest, emqx, emqx_ctl]},

apps/emqx_management/src/emqx_mgmt_api_configs.erl

@@ -122,6 +122,7 @@ schema("/configs") ->
                             }}
                         ]
                 },
+                400 => emqx_dashboard_swagger:error_codes(['INVALID_ACCEPT']),
                 404 => emqx_dashboard_swagger:error_codes(['NOT_FOUND']),
                 500 => emqx_dashboard_swagger:error_codes(['BAD_NODE'])
             }
@@ -337,9 +338,10 @@ config_reset(post, _Params, Req) ->
 
 configs(get, #{query_string := QueryStr, headers := Headers}, _Req) ->
     %% Should deprecated json v1 since 5.2.0
-    case maps:get(<<"accept">>, Headers, <<"text/plain">>) of
-        <<"application/json">> -> get_configs_v1(QueryStr);
-        <<"text/plain">> -> get_configs_v2(QueryStr)
+    case find_suitable_accept(Headers, [<<"text/plain">>, <<"application/json">>]) of
+        {ok, <<"application/json">>} -> get_configs_v1(QueryStr);
+        {ok, <<"text/plain">>} -> get_configs_v2(QueryStr);
+        {error, _} = Error -> {400, #{code => 'INVALID_ACCEPT', message => ?ERR_MSG(Error)}}
     end;
 configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) ->
     case emqx_conf_cli:load_config(Conf, Mode) of
@@ -348,6 +350,28 @@ configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) ->
         {error, Errors} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Errors)}}
     end.
 
+find_suitable_accept(Headers, Perferences) when is_list(Perferences), length(Perferences) > 0 ->
+    AcceptVal = maps:get(<<"accept">>, Headers, <<"*/*">>),
+    %% Multiple types, weighted with the quality value syntax:
+    %% Accept: text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8
+    Accepts = lists:map(
+        fun(S) ->
+            [T | _] = binary:split(string:trim(S), <<";">>),
+            T
+        end,
+        re:split(AcceptVal, ",")
+    ),
+    case lists:member(<<"*/*">>, Accepts) of
+        true ->
+            {ok, lists:nth(1, Perferences)};
+        false ->
+            Found = lists:filter(fun(Accept) -> lists:member(Accept, Accepts) end, Perferences),
+            case Found of
+                [] -> {error, no_suitalbe_accept};
+                _ -> {ok, lists:nth(1, Found)}
+            end
+    end.
+
 get_configs_v1(QueryStr) ->
     Node = maps:get(<<"node">>, QueryStr, node()),
     case

apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl

@@ -323,6 +323,36 @@ t_configs_key(_Config) ->
     ?assertEqual(<<"error">>, read_conf([<<"log">>, <<"console">>, <<"level">>])),
     ok.
 
+t_get_configs_in_different_accept(_Config) ->
+    [Key | _] = lists:sort(emqx_conf_cli:keys()),
+    URI = emqx_mgmt_api_test_util:api_path(["configs?key=" ++ Key]),
+    Auth = emqx_mgmt_api_test_util:auth_header_(),
+    Request = fun(Accept) ->
+        Headers = [{"accept", Accept}, Auth],
+        case
+            emqx_mgmt_api_test_util:request_api(get, URI, [], Headers, [], #{return_all => true})
+        of
+            {ok, {{_, Code, _}, RespHeaders, Body}} ->
+                Type = proplists:get_value("content-type", RespHeaders),
+                {Code, Type, Body};
+            {error, {{_, Code, _}, RespHeaders, Body}} ->
+                Type = proplists:get_value("content-type", RespHeaders),
+                {Code, Type, Body}
+        end
+    end,
+
+    %% returns text/palin if text/plain is acceptable
+    ?assertMatch({200, "text/plain", _}, Request(<<"text/plain">>)),
+    ?assertMatch({200, "text/plain", _}, Request(<<"*/*">>)),
+    ?assertMatch(
+        {200, "text/plain", _},
+        Request(<<"application/json, application/xml;q=0.9, image/webp, */*;q=0.8">>)
+    ),
+    %% returns application/json if it only support it
+    ?assertMatch({200, "application/json", _}, Request(<<"application/json">>)),
+    %% returns error if it set to other type
+    ?assertMatch({400, "application/json", _}, Request(<<"application/xml">>)).
+
 %% Helpers
 
 get_config(Name) ->