Merge pull request #13127 from keynslug/fix/EMQX-12439/update-leaner-error

fix(bridge-v2): report descriptive error on invalid update request

apps/emqx_bridge/src/emqx_bridge.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_bridge, [
     {description, "EMQX bridges"},
-    {vsn, "0.2.0"},
+    {vsn, "0.2.1"},
     {registered, [emqx_bridge_sup]},
     {mod, {emqx_bridge_app, []}},
     {applications, [

apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -96,7 +96,7 @@
 namespace() -> "actions_and_sources".
 
 api_spec() ->
-    emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}).
+    emqx_dashboard_swagger:spec(?MODULE, #{check_schema => fun check_api_schema/2}).
 
 paths() ->
     [
@@ -656,6 +656,40 @@ schema("/source_types") ->
         }
     }.
 
+%%------------------------------------------------------------------------------
+
+check_api_schema(Request, ReqMeta = #{path := "/actions/:id", method := put}) ->
+    BridgeId = emqx_utils_maps:deep_get([bindings, id], Request),
+    try emqx_bridge_resource:parse_bridge_id(BridgeId, #{atom_name => false}) of
+        %% NOTE
+        %% Bridge type is known, refine the API schema to get more specific error messages.
+        {BridgeType, _Name} ->
+            Schema = emqx_bridge_v2_schema:action_api_schema("put", BridgeType),
+            emqx_dashboard_swagger:filter_check_request(Request, refine_api_schema(Schema, ReqMeta))
+    catch
+        throw:#{reason := Reason} ->
+            ?NOT_FOUND(<<"Invalid bridge ID, ", Reason/binary>>)
+    end;
+check_api_schema(Request, ReqMeta = #{path := "/sources/:id", method := put}) ->
+    SourceId = emqx_utils_maps:deep_get([bindings, id], Request),
+    try emqx_bridge_resource:parse_bridge_id(SourceId, #{atom_name => false}) of
+        %% NOTE
+        %% Source type is known, refine the API schema to get more specific error messages.
+        {BridgeType, _Name} ->
+            Schema = emqx_bridge_v2_schema:source_api_schema("put", BridgeType),
+            emqx_dashboard_swagger:filter_check_request(Request, refine_api_schema(Schema, ReqMeta))
+    catch
+        throw:#{reason := Reason} ->
+            ?NOT_FOUND(<<"Invalid source ID, ", Reason/binary>>)
+    end;
+check_api_schema(Request, ReqMeta) ->
+    emqx_dashboard_swagger:filter_check_request(Request, ReqMeta).
+
+refine_api_schema(Schema, ReqMeta = #{path := Path, method := Method}) ->
+    Spec = maps:get(Method, schema(Path)),
+    SpecRefined = Spec#{'requestBody' => Schema},
+    ReqMeta#{apispec => SpecRefined}.
+
 %%------------------------------------------------------------------------------
 %% Thin Handlers
 %%------------------------------------------------------------------------------

apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl

@@ -31,6 +31,7 @@
     actions_get_response/0,
     actions_put_request/0,
     actions_post_request/0,
+    action_api_schema/2,
     actions_examples/1,
     action_values/4
 ]).
@@ -39,6 +40,7 @@
     sources_get_response/0,
     sources_put_request/0,
     sources_post_request/0,
+    source_api_schema/2,
     sources_examples/1,
     source_values/4
 ]).
@@ -100,6 +102,15 @@ actions_api_schema(Method) ->
     APISchemas = ?MODULE:registered_actions_api_schemas(Method),
     hoconsc:union(bridge_api_union(APISchemas)).
 
+action_api_schema(Method, BridgeV2Type) ->
+    APISchemas = ?MODULE:registered_actions_api_schemas(Method),
+    case lists:keyfind(atom_to_binary(BridgeV2Type), 1, APISchemas) of
+        {_, SchemaRef} ->
+            hoconsc:mk(SchemaRef);
+        false ->
+            unknown_bridge_schema(BridgeV2Type)
+    end.
+
 registered_actions_api_schemas(Method) ->
     RegisteredSchemas = emqx_action_info:registered_schema_modules_actions(),
     [
@@ -159,6 +170,15 @@ sources_api_schema(Method) ->
     APISchemas = ?MODULE:registered_sources_api_schemas(Method),
     hoconsc:union(bridge_api_union(APISchemas)).
 
+source_api_schema(Method, SourceType) ->
+    APISchemas = ?MODULE:registered_sources_api_schemas(Method),
+    case lists:keyfind(atom_to_binary(SourceType), 1, APISchemas) of
+        {_, SchemaRef} ->
+            hoconsc:mk(SchemaRef);
+        false ->
+            unknown_source_schema(SourceType)
+    end.
+
 registered_sources_api_schemas(Method) ->
     RegisteredSchemas = emqx_action_info:registered_schema_modules_sources(),
     [
@@ -231,6 +251,25 @@ bridge_api_union(Refs) ->
             end
     end.
 
+unknown_bridge_schema(BridgeV2Type) ->
+    erroneous_value_schema(BridgeV2Type, <<"unknown bridge type">>).
+
+unknown_source_schema(SourceType) ->
+    erroneous_value_schema(SourceType, <<"unknown source type">>).
+
+%% @doc Construct a schema that always emits validation error.
+%% We need to silence dialyzer because inner anonymous function always throws.
+-dialyzer({nowarn_function, [erroneous_value_schema/2]}).
+erroneous_value_schema(Value, Reason) ->
+    hoconsc:mk(typerefl:any(), #{
+        validator => fun(_) ->
+            throw(#{
+                value => Value,
+                reason => Reason
+            })
+        end
+    }).
+
 -spec method_values(action | source, http_method(), atom()) -> schema_example_map().
 method_values(Kind, post, Type) ->
     KindBin = atom_to_binary(Kind),

apps/emqx_bridge_s3/test/emqx_bridge_s3_aggreg_upload_SUITE.erl

@@ -156,12 +156,15 @@ t_create_via_http(Config) ->
 t_on_get_status(Config) ->
     emqx_bridge_v2_testlib:t_on_get_status(Config, #{}).
 
-t_invalid_config(Config) ->
+t_create_invalid_config(Config) ->
     ?assertMatch(
         {error,
             {_Status, _, #{
                 <<"code">> := <<"BAD_REQUEST">>,
-                <<"message">> := #{<<"kind">> := <<"validation_error">>}
+                <<"message">> := #{
+                    <<"kind">> := <<"validation_error">>,
+                    <<"reason">> := <<"Inconsistent 'min_part_size'", _/bytes>>
+                }
             }}},
         emqx_bridge_v2_testlib:create_bridge_api(
             Config,
@@ -174,6 +177,28 @@ t_invalid_config(Config) ->
         )
     ).
 
+t_update_invalid_config(Config) ->
+    ?assertMatch({ok, _Bridge}, emqx_bridge_v2_testlib:create_bridge(Config)),
+    ?assertMatch(
+        {error,
+            {_Status, _, #{
+                <<"code">> := <<"BAD_REQUEST">>,
+                <<"message">> := #{
+                    <<"kind">> := <<"validation_error">>,
+                    <<"reason">> := <<"Inconsistent 'min_part_size'", _/bytes>>
+                }
+            }}},
+        emqx_bridge_v2_testlib:update_bridge_api(
+            Config,
+            _Overrides = #{
+                <<"parameters">> => #{
+                    <<"min_part_size">> => <<"5GB">>,
+                    <<"max_part_size">> => <<"100MB">>
+                }
+            }
+        )
+    ).
+
 t_aggreg_upload(Config) ->
     Bucket = ?config(s3_bucket, Config),
     BridgeName = ?config(bridge_name, Config),

apps/emqx_dashboard/src/emqx_dashboard.app.src

@@ -2,7 +2,7 @@
 {application, emqx_dashboard, [
     {description, "EMQX Web Dashboard"},
     % strict semver, bump manually!
-    {vsn, "5.1.0"},
+    {vsn, "5.1.1"},
     {modules, []},
     {registered, [emqx_dashboard_sup]},
     {applications, [

apps/emqx_dashboard/src/emqx_dashboard_swagger.erl

@@ -91,7 +91,14 @@
 -define(DEFAULT_ROW, 100).
 
 -type request() :: #{bindings => map(), query_string => map(), body => map()}.
--type request_meta() :: #{module => module(), path => string(), method => atom()}.
+-type request_meta() :: #{
+    module := module(),
+    path := string(),
+    method := atom(),
+    %% API Operation specification override.
+    %% Takes precedence over the API specification defined in the module.
+    apispec => map()
+}.
 
 %% More exact types are defined in minirest.hrl, but we don't want to include it
 %% because it defines a lot of types and they may clash with the types declared locally.
@@ -335,8 +342,8 @@ filter_check_request_and_translate_body(Request, RequestMeta) ->
 filter_check_request(Request, RequestMeta) ->
     translate_req(Request, RequestMeta, fun check_only/3).
 
-translate_req(Request, #{module := Module, path := Path, method := Method}, CheckFun) ->
-    #{Method := Spec} = apply(Module, schema, [Path]),
+translate_req(Request, ReqMeta = #{module := Module}, CheckFun) ->
+    Spec = find_req_apispec(ReqMeta),
     try
         Params = maps:get(parameters, Spec, []),
         Body = maps:get('requestBody', Spec, []),
@@ -349,6 +356,12 @@ translate_req(Request, #{module := Module, path := Path, method := Method}, Chec
             {400, 'BAD_REQUEST', Msg}
     end.
 
+find_req_apispec(#{apispec := Spec}) ->
+    Spec;
+find_req_apispec(#{module := Module, path := Path, method := Method}) ->
+    #{Method := Spec} = apply(Module, schema, [Path]),
+    Spec.
+
 check_and_translate(Schema, Map, Opts) ->
     hocon_tconf:check_plain(Schema, Map, Opts).