From 6ee18b6104ec2ea8d21ed8545331383842fba5ac Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Mon, 27 Dec 2021 23:03:15 +0100 Subject: [PATCH 01/10] refactor(broker): Don't use a macro --- apps/emqx/rebar.config | 2 +- apps/emqx/src/emqx_rpc.erl | 14 ++++++-------- mix.exs | 2 +- rebar.config | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index f573ddc87..4ec68d585 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -16,7 +16,7 @@ , {cowboy, {git, "https://github.com/emqx/cowboy", {tag, "2.9.0"}}} , {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.9.0"}}} , {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.11.2"}}} - , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.5.1"}}} + , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.0"}}} , {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.22.2"}}} , {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}} , {recon, {git, "https://github.com/ferd/recon", {tag, "2.5.1"}}} diff --git a/apps/emqx/src/emqx_rpc.erl b/apps/emqx/src/emqx_rpc.erl index 527123745..0ea2814b0 100644 --- a/apps/emqx/src/emqx_rpc.erl +++ b/apps/emqx/src/emqx_rpc.erl @@ -30,27 +30,25 @@ , rpc_nodes/1 ]}). --define(RPC, gen_rpc). - -define(DefaultClientNum, 1). call(Node, Mod, Fun, Args) -> - filter_result(?RPC:call(rpc_node(Node), Mod, Fun, Args)). + filter_result(gen_rpc:call(rpc_node(Node), Mod, Fun, Args)). call(Key, Node, Mod, Fun, Args) -> - filter_result(?RPC:call(rpc_node({Key, Node}), Mod, Fun, Args)). + filter_result(gen_rpc:call(rpc_node({Key, Node}), Mod, Fun, Args)). multicall(Nodes, Mod, Fun, Args) -> - filter_result(?RPC:multicall(rpc_nodes(Nodes), Mod, Fun, Args)). + filter_result(gen_rpc:multicall(rpc_nodes(Nodes), Mod, Fun, Args)). multicall(Key, Nodes, Mod, Fun, Args) -> - filter_result(?RPC:multicall(rpc_nodes([{Key, Node} || Node <- Nodes]), Mod, Fun, Args)). + filter_result(gen_rpc:multicall(rpc_nodes([{Key, Node} || Node <- Nodes]), Mod, Fun, Args)). cast(Node, Mod, Fun, Args) -> - filter_result(?RPC:cast(rpc_node(Node), Mod, Fun, Args)). + filter_result(gen_rpc:cast(rpc_node(Node), Mod, Fun, Args)). cast(Key, Node, Mod, Fun, Args) -> - filter_result(?RPC:cast(rpc_node({Key, Node}), Mod, Fun, Args)). + filter_result(gen_rpc:cast(rpc_node({Key, Node}), Mod, Fun, Args)). rpc_node(Node) when is_atom(Node) -> {Node, rand:uniform(max_client_num())}; diff --git a/mix.exs b/mix.exs index b4b9ef701..ca66f0c6d 100644 --- a/mix.exs +++ b/mix.exs @@ -54,7 +54,7 @@ defmodule EMQXUmbrella.MixProject do {:esockd, github: "emqx/esockd", tag: "5.9.0", override: true}, {:mria, github: "emqx/mria", tag: "0.1.5", override: true}, {:ekka, github: "emqx/ekka", tag: "0.11.2", override: true}, - {:gen_rpc, github: "emqx/gen_rpc", tag: "2.5.1", override: true}, + {:gen_rpc, github: "emqx/gen_rpc", tag: "2.8.0", override: true}, {:minirest, github: "emqx/minirest", tag: "1.2.9", override: true}, {:ecpool, github: "emqx/ecpool", tag: "0.5.2"}, {:replayq, "0.3.3", override: true}, diff --git a/rebar.config b/rebar.config index 17d4a0ce3..49e37e4af 100644 --- a/rebar.config +++ b/rebar.config @@ -55,7 +55,7 @@ , {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.9.0"}}} , {mria, {git, "https://github.com/emqx/mria", {tag, "0.1.5"}}} , {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.11.2"}}} - , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.5.1"}}} + , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.0"}}} , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.2.9"}}} , {ecpool, {git, "https://github.com/emqx/ecpool", {tag, "0.5.2"}}} , {replayq, "0.3.3"} From 5c2a559991c38071ef8428434ca7bbd54c675b95 Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 10:55:16 +0100 Subject: [PATCH 02/10] feat(bpapi): Initial commit --- apps/emqx/src/emqx_broker.erl | 2 +- apps/emqx/src/emqx_rpc.erl | 4 +- apps/emqx/src/proto/emqx_broker_proto_v1.erl | 34 +++ apps/emqx_bpapi/README.md | 5 + apps/emqx_bpapi/include/bpapi.hrl | 6 + apps/emqx_bpapi/priv/.gitkeep | 0 apps/emqx_bpapi/src/emqx_bpapi.app.src | 15 ++ apps/emqx_bpapi/src/emqx_bpapi.erl | 54 +++++ .../src/emqx_bpapi_static_checks.erl | 147 +++++++++++++ apps/emqx_bpapi/src/emqx_bpapi_trans.erl | 200 ++++++++++++++++++ 10 files changed, 465 insertions(+), 2 deletions(-) create mode 100644 apps/emqx/src/proto/emqx_broker_proto_v1.erl create mode 100644 apps/emqx_bpapi/README.md create mode 100644 apps/emqx_bpapi/include/bpapi.hrl create mode 100644 apps/emqx_bpapi/priv/.gitkeep create mode 100644 apps/emqx_bpapi/src/emqx_bpapi.app.src create mode 100644 apps/emqx_bpapi/src/emqx_bpapi.erl create mode 100644 apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl create mode 100644 apps/emqx_bpapi/src/emqx_bpapi_trans.erl diff --git a/apps/emqx/src/emqx_broker.erl b/apps/emqx/src/emqx_broker.erl index 9dbfb0b43..bdb6acd4f 100644 --- a/apps/emqx/src/emqx_broker.erl +++ b/apps/emqx/src/emqx_broker.erl @@ -300,7 +300,7 @@ forward(Node, To, Delivery, sync) -> end. -spec(dispatch(emqx_types:topic(), emqx_types:delivery()) -> emqx_types:deliver_result()). -dispatch(Topic, Delivery) -> +dispatch(Topic, Delivery = #delivery{}) when is_binary(Topic) -> case emqx:is_running() of true -> do_dispatch(Topic, Delivery); diff --git a/apps/emqx/src/emqx_rpc.erl b/apps/emqx/src/emqx_rpc.erl index 0ea2814b0..ad7e9891f 100644 --- a/apps/emqx/src/emqx_rpc.erl +++ b/apps/emqx/src/emqx_rpc.erl @@ -14,9 +14,11 @@ %% limitations under the License. %%-------------------------------------------------------------------- -%% @doc wrap gen_rpc? -module(emqx_rpc). +%% Note: please don't forget to add new API functions to +%% `emqx_bpapi_trans:extract_mfa' + -export([ call/4 , call/5 , cast/4 diff --git a/apps/emqx/src/proto/emqx_broker_proto_v1.erl b/apps/emqx/src/proto/emqx_broker_proto_v1.erl new file mode 100644 index 000000000..a40884c38 --- /dev/null +++ b/apps/emqx/src/proto/emqx_broker_proto_v1.erl @@ -0,0 +1,34 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_broker_proto_v1). + +-introduced_in("5.0.0"). + +-export([ forward/3 + , forward_async/3 + ]). + +-include_lib("emqx_bpapi/include/bpapi.hrl"). +-include("emqx.hrl"). + +-spec forward(node(), emqx_types:topic(), emqx_types:delivery()) -> emqx_types:deliver_result(). +forward(Node, Topic, Delivery = #delivery{}) when is_binary(Topic) -> + emqx_rpc:call(Topic, Node, emqx_broker, dispatch, [Topic, Delivery]). + +-spec forward_async(node(), emqx_types:topic(), emqx_types:delivery()) -> ok. +forward_async(Node, Topic, Delivery = #delivery{}) when is_binary(Topic) -> + emqx_rpc:cast(Topic, Node, emqx_broker, dispatch, [Topic, Delivery]). diff --git a/apps/emqx_bpapi/README.md b/apps/emqx_bpapi/README.md new file mode 100644 index 000000000..78f529a4c --- /dev/null +++ b/apps/emqx_bpapi/README.md @@ -0,0 +1,5 @@ +emqx_bpapi +===== + +A library that helps maintaining EMQX's backplane API backward and +forward compatibility. diff --git a/apps/emqx_bpapi/include/bpapi.hrl b/apps/emqx_bpapi/include/bpapi.hrl new file mode 100644 index 000000000..b4e52d50f --- /dev/null +++ b/apps/emqx_bpapi/include/bpapi.hrl @@ -0,0 +1,6 @@ +-ifndef(EMQX_BPAPI_HRL). +-define(EMQX_BPAPI_HRL, true). + +-compile({parse_transform, emqx_bpapi_trans}). + +-endif. diff --git a/apps/emqx_bpapi/priv/.gitkeep b/apps/emqx_bpapi/priv/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/apps/emqx_bpapi/src/emqx_bpapi.app.src b/apps/emqx_bpapi/src/emqx_bpapi.app.src new file mode 100644 index 000000000..45a3efc10 --- /dev/null +++ b/apps/emqx_bpapi/src/emqx_bpapi.app.src @@ -0,0 +1,15 @@ +{application, emqx_bpapi, + [{description, "A library for verifying safety of RPC calls"}, + {vsn, "0.1.0"}, + {registered, []}, + {applications, + [kernel, + stdlib, + typerefl %% Just for some metaprogramming utils + ]}, + {env,[]}, + {modules, []}, + + {licenses, ["Apache 2.0"]}, + {links, []} + ]}. diff --git a/apps/emqx_bpapi/src/emqx_bpapi.erl b/apps/emqx_bpapi/src/emqx_bpapi.erl new file mode 100644 index 000000000..284de4dfc --- /dev/null +++ b/apps/emqx_bpapi/src/emqx_bpapi.erl @@ -0,0 +1,54 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-module(emqx_bpapi). + +-export([parse_semver/1, api_and_version/1]). + +-export_type([var_name/0, call/0, rpc/0, bpapi_meta/0]). + +-type semver() :: {non_neg_integer(), non_neg_integer(), non_neg_integer()}. + +-type api() :: atom(). +-type api_version() :: non_neg_integer(). +-type var_name() :: atom(). +-type call() :: {module(), atom(), [var_name()]}. +-type rpc() :: {_From :: call(), _To :: call()}. + +-type bpapi_meta() :: + #{ api := api() + , version := api_version() + , calls := [rpc()] + , casts := [rpc()] + }. + +-spec parse_semver(string()) -> {ok, semver()} + | false. +parse_semver(Str) -> + Opts = [{capture, all_but_first, list}], + case re:run(Str, "^([0-9]+)\\.([0-9]+)\\.([0-9]+)$", Opts) of + {match, [A, B, C]} -> {ok, {list_to_integer(A), list_to_integer(B), list_to_integer(C)}}; + nomatch -> error + end. + +-spec api_and_version(module()) -> {atom(), non_neg_integer()}. +api_and_version(Module) -> + Opts = [{capture, all_but_first, list}], + case re:run(atom_to_list(Module), "(.*)_proto_v([0-9]+)$", Opts) of + {match, [API, VsnStr]} -> + {ok, list_to_atom(API), list_to_integer(VsnStr)}; + nomatch -> + error(Module) + end. diff --git a/apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl b/apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl new file mode 100644 index 000000000..de252b2b2 --- /dev/null +++ b/apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl @@ -0,0 +1,147 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_bpapi_static_checks). + +-export([run/1, run/0]). + +-include_lib("emqx/include/logger.hrl"). + +%% `emqx_bpapi:call' enriched with dialyzer spec +-type typed_call() :: {emqx_bpapi:call(), _DialyzerSpec}. +-type typed_rpc() :: {typed_call(), typed_call()}. + +-type fulldump() :: #{emqx_bpapi:api() => + #{emqx_bpapi:api_version() => + #{ calls := [typed_rpc()] + , casts := [typed_rpc()] + } + }}. + +%% Applications we wish to ignore in the analysis: +-define(IGNORED_APPS, "gen_rpc, recon, observer_cli, snabbkaffe, ekka, mria"). +%% List of known RPC backend modules: +-define(RPC_MODULES, "gen_rpc, erpc, rpc, emqx_rpc"). +%% List of functions in the RPC backend modules that we can ignore: +-define(IGNORED_RPC_CALLS, "gen_rpc:nodes/0"). + +-define(XREF, myxref). + +run() -> + case {filelib:wildcard("*_plt"), filelib:wildcard("_build/emqx*/lib")} of + {[PLT|_], [RelDir|_]} -> + run(#{plt => PLT, reldir => RelDir}); + _ -> + error("failed to guess run options") + end. + +-spec run(map()) -> boolean(). +run(Opts) -> + put(bpapi_ok, true), + PLT = prepare(Opts), + %% First we run XREF to find all callers of any known RPC backend: + Callers = find_remote_calls(Opts), + {BPAPICalls, NonBPAPICalls} = lists:partition(fun is_bpapi_call/1, Callers), + warn_nonbpapi_rpcs(NonBPAPICalls), + CombinedAPI = collect_bpapis(BPAPICalls, PLT), + dump_api(CombinedAPI), + erase(bpapi_ok). + +prepare(#{reldir := RelDir, plt := PLT}) -> + ?INFO("Starting xref...", []), + xref:start(?XREF), + filelib:wildcard(RelDir ++ "/*/ebin/") =:= [] andalso + error("No applications found in the release directory. Wrong directory?"), + xref:set_default(?XREF, [{warnings, false}]), + xref:add_release(?XREF, RelDir), + %% Now to the dialyzer stuff: + ?INFO("Loading PLT...", []), + dialyzer_plt:from_file(PLT). + +find_remote_calls(_Opts) -> + Query = "XC | (A - [" ?IGNORED_APPS "]:App) + || ([" ?RPC_MODULES "] : Mod - " ?IGNORED_RPC_CALLS ")", + {ok, Calls} = xref:q(?XREF, Query), + ?INFO("Calls to RPC modules ~p", [Calls]), + {Callers, _Callees} = lists:unzip(Calls), + Callers. + +-spec warn_nonbpapi_rpcs([mfa()]) -> ok. +warn_nonbpapi_rpcs([]) -> + ok; +warn_nonbpapi_rpcs(L) -> + setnok(), + lists:foreach(fun({M, F, A}) -> + ?ERROR("~p:~p/~p does a remote call outside of a dedicated " + "backplane API module. " + "It may break during rolling cluster upgrade", + [M, F, A]) + end, + L). + +-spec is_bpapi_call(mfa()) -> boolean(). +is_bpapi_call({Module, _Function, _Arity}) -> + case catch Module:bpapi_meta() of + #{api := _} -> true; + _ -> false + end. + +-spec dump_api(fulldump()) -> ok. +dump_api(Term) -> + Filename = filename:join(code:priv_dir(emqx_bpapi), emqx_app:get_release() ++ ".bpapi"), + file:write_file(Filename, io_lib:format("~0p.", [Term])). + +-spec collect_bpapis([mfa()], _PLT) -> fulldump(). +collect_bpapis(L, PLT) -> + Modules = lists:usort([M || {M, _F, _A} <- L]), + lists:foldl(fun(Mod, Acc) -> + #{ api := API + , version := Vsn + , calls := Calls0 + , casts := Casts0 + } = Mod:bpapi_meta(), + Calls = enrich(PLT, Calls0), + Casts = enrich(PLT, Casts0), + Acc#{API => #{Vsn => #{ calls => Calls + , casts => Casts + }}} + end, + #{}, + Modules + ). + +%% Add information about types from the PLT +-spec enrich(_PLT, [emqx_bpapi:rpc()]) -> [typed_rpc()]. +enrich(PLT, Calls) -> + [case {lookup_type(PLT, From), lookup_type(PLT, To)} of + {{value, TFrom}, {value, TTo}} -> + {{From, TFrom}, {To, TTo}}; + {_, none} -> + setnok(), + ?CRITICAL("Backplane API function ~s calls a missing remote function ~s", + [format_call(From), format_call(To)]), + error(missing_target) + end + || {From, To} <- Calls]. + +lookup_type(PLT, {M, F, A}) -> + dialyzer_plt:lookup(PLT, {M, F, length(A)}). + +format_call({M, F, A}) -> + io_lib:format("~p:~p/~p", [M, F, length(A)]). + +setnok() -> + put(bpapi_ok, false). diff --git a/apps/emqx_bpapi/src/emqx_bpapi_trans.erl b/apps/emqx_bpapi/src/emqx_bpapi_trans.erl new file mode 100644 index 000000000..bdcab83af --- /dev/null +++ b/apps/emqx_bpapi/src/emqx_bpapi_trans.erl @@ -0,0 +1,200 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +%% @hidden This parse transform generates BPAPI metadata function for +%% a module, and helps dialyzer typechecking RPC calls +-module(emqx_bpapi_trans). + +-export([parse_transform/2, format_error/1]). + +%%-define(debug, true). + +-define(META_FUN, bpapi_meta). + +-type semantics() :: call | cast. + +-record(s, + { api :: atom() + , module :: atom() + , version :: non_neg_integer() | undefined + , introduced_in :: emqx_bpapi:semver() | undefined + , deprecated_since :: emqx_bpapi:semver() | undefined + , targets = [] :: [{semantics(), emqx_bpapi:call(), emqx_bpapi:call()}] + , errors = [] :: [string()] + , file + }). + +format_error(invalid_name) -> + "BPAPI module name should follow _proto_v pattern"; +format_error(invalid_introduced_in) -> + "-introduced_in attribute should be present and its value should be a semver string"; +format_error(invalid_deprecated_since) -> + "value of -deprecated_since attribute should be a semver string"; +format_error({invalid_fun, Name, Arity}) -> + io_lib:format("malformed function ~p/~p. " + "BPAPI functions should have exactly one clause " + "and call (emqx_|e)rpc at the top level", + [Name, Arity]). + +parse_transform(Forms, _Options) -> + log("Original:~n~p", [Forms]), + State = #s{file = File} = lists:foldl(fun go/2, #s{}, Forms), + log("parse_trans state: ~p", [State]), + case check(State) of + [] -> + finalize(Forms, State); + Errors -> + {error, [{File, [{Line, ?MODULE, Msg} || {Line, Msg} <- Errors]}], []} + end. + +%% Scan erl_forms: +go({attribute, _, file, {File, _}}, S) -> + S#s{file = File}; +go({attribute, Line, module, Mod}, S) -> + case emqx_bpapi:api_and_version(Mod) of + {ok, API, Vsn} -> S#s{api = API, version = Vsn, module = Mod}; + error -> push_err(Line, invalid_name, S) + end; +go({attribute, _Line, introduced_in, Str}, S) -> + case is_list(Str) andalso emqx_bpapi:parse_semver(Str) of + {ok, Vsn} -> S#s{introduced_in = Vsn}; + false -> S %% Don't report error here, it's done in check/1 + end; +go({attribute, Line, deprecated_since, Str}, S) -> + case is_list(Str) andalso emqx_bpapi:parse_semver(Str) of + {ok, Vsn} -> S#s{deprecated_since = Vsn}; + false -> push_err(Line, invalid_deprecated_since, S) + end; +go({function, Line, Name, Arity, Clauses}, S) -> + analyze_fun(Line, Name, Arity, Clauses, S); +go(_, S) -> + S. + +check(#s{errors = Err0, introduced_in = II}) -> + [{none, invalid_introduced_in} || II =:= undefined] ++ + Err0. + +finalize(Forms, S) -> + {Attrs, Funcs} = lists:splitwith(fun is_attribute/1, Forms), + AST = mk_meta_fun(S), + log("Meta fun:~n~p", [AST]), + Attrs ++ [mk_export()] ++ [AST|Funcs]. + +mk_meta_fun(#s{api = API, version = Vsn, targets = Targets}) -> + Line = 0, + Calls = [{From, To} || {call, From, To} <- Targets], + Casts = [{From, To} || {cast, From, To} <- Targets], + Ret = typerefl_quote:const(Line, #{ api => API + , version => Vsn + , calls => Calls + , casts => Casts + }), + {function, Line, ?META_FUN, _Arity = 0, + [{clause, Line, _Args = [], _Guards = [], + [Ret]}]}. + +mk_export() -> + {attribute, 0, export, [{?META_FUN, 0}]}. + +is_attribute({attribute, _Line, _Attr, _Val}) -> true; +is_attribute(_) -> false. + +%% Extract the target function of the RPC call +analyze_fun(Line, Name, Arity, [{clause, Line, Head, _Guards, Exprs}], S) -> + analyze_exprs(Line, Name, Arity, Head, Exprs, S); +analyze_fun(Line, Name, Arity, _Clauses, S) -> + invalid_fun(Line, Name, Arity, S). + +analyze_exprs(Line, Name, Arity, Head, Exprs, S) -> + log("~p/~p (~p):~n~p", [Name, Arity, Head, Exprs]), + try + [{call, _, CallToBackend, CallArgs}] = Exprs, + OuterArgs = extract_outer_args(Head), + Key = {S#s.module, Name, OuterArgs}, + {Semantics, Target} = extract_target_call(CallToBackend, CallArgs), + push_target({Semantics, Key, Target}, S) + catch + _:Err:Stack -> + log("Failed to process function call:~n~s~nStack: ~p", [Err, Stack]), + invalid_fun(Line, Name, Arity, S) + end. + +-spec extract_outer_args(erl_parse:abstract_form()) -> [atom()]. +extract_outer_args(Abs) -> + lists:map(fun({var, _, Var}) -> + Var; + ({match, _, {var, _, Var}, _}) -> + Var; + ({match, _, _, {var, _, Var}}) -> + Var + end, + Abs). + +-spec extract_target_call(Abs, [Abs]) -> {semantics(), emqx_bpapi:call()} + when Abs :: erl_parse:abstract_form(). +extract_target_call(RPCBackend, OuterArgs) -> + {Semantics, {atom, _, M}, {atom, _, F}, A} = extract_mfa(RPCBackend, OuterArgs), + {Semantics, {M, F, list_to_args(A)}}. + +-define(BACKEND(MOD, FUN), {remote, _, {atom, _, MOD}, {atom, _, FUN}}). +-define(IS_RPC(MOD), (MOD =:= erpc orelse MOD =:= rpc)). + +-spec extract_mfa(Abs, #s{}) -> {call | cast, Abs, Abs, Abs} + when Abs :: erl_parse:abstract_form(). +%% gen_rpc: +extract_mfa(?BACKEND(gen_rpc, _), _) -> + %% gen_rpc has an extremely messy API, thankfully it's fully wrapped + %% by emqx_rpc, so we simply forbid direct calls to it: + error("direct call to gen_rpc"); +%% emqx_rpc: +extract_mfa(?BACKEND(emqx_rpc, CallOrCast), [_Node, M, F, A]) -> + {call_or_cast(CallOrCast), M, F, A}; +extract_mfa(?BACKEND(emqx_rpc, CallOrCast), [_Tag, _Node, M, F, A]) -> + {call_or_cast(CallOrCast), M, F, A}; +%% (e)rpc: +extract_mfa(?BACKEND(RPC, CallOrCast), [_Node, M, F, A]) when ?IS_RPC(RPC) -> + {call_or_cast(CallOrCast), M, F, A}; +extract_mfa(?BACKEND(RPC, CallOrCast), [_Node, M, F, A, _Timeout]) when ?IS_RPC(RPC) -> + {call_or_cast(CallOrCast), M, F, A}; +extract_mfa(_, _) -> + error("unrecognized RPC call"). + +call_or_cast(cast) -> cast; +call_or_cast(multicast) -> cast; +call_or_cast(multicall) -> call; +call_or_cast(call) -> call. + +list_to_args({cons, _, {var, _, A}, T}) -> + [A|list_to_args(T)]; +list_to_args({nil, _}) -> + []. + +invalid_fun(Line, Name, Arity, S) -> + push_err(Line, {invalid_fun, Name, Arity}, S). + +push_err(Line, Err, S = #s{errors = Errs}) -> + S#s{errors = [{Line, Err}|Errs]}. + +push_target(Target, S = #s{targets = Targets}) -> + S#s{targets = [Target|Targets]}. + +-ifdef(debug). +log(Fmt, Args) -> + io:format(user, "!! " ++ Fmt ++ "~n", Args). +-else. +log(_, _) -> + ok. +-endif. From 96fdd0c31f3dbae17cb93e16b62c24833124e34c Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 12:07:40 +0100 Subject: [PATCH 03/10] fix(bpapi): Fix build order --- apps/emqx/src/emqx.app.src | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/emqx/src/emqx.app.src b/apps/emqx/src/emqx.app.src index 0989cd7f1..597e1596f 100644 --- a/apps/emqx/src/emqx.app.src +++ b/apps/emqx/src/emqx.app.src @@ -17,6 +17,7 @@ , jiffy , lc , hocon + , emqx_bpapi ]}, {mod, {emqx_app,[]}}, {env, []}, From 2c3af8d9fe9fe6eab885409502ea1ec10e82f59c Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 15:39:09 +0100 Subject: [PATCH 04/10] feat(bpapi): Move to emqx Fix standalone build --- apps/{emqx_bpapi => emqx}/include/bpapi.hrl | 0 apps/{emqx_bpapi => emqx}/priv/.gitkeep | 0 apps/emqx/rebar.config | 2 ++ .../src => emqx/src/bpapi}/emqx_bpapi.erl | 0 .../src/bpapi}/emqx_bpapi_static_checks.erl | 0 .../src => emqx/src/bpapi}/emqx_bpapi_trans.erl | 0 apps/emqx/src/emqx.app.src | 1 - apps/emqx/src/proto/emqx_broker_proto_v1.erl | 2 +- apps/emqx_bpapi/README.md | 5 ----- apps/emqx_bpapi/src/emqx_bpapi.app.src | 15 --------------- rebar.config | 2 ++ 11 files changed, 5 insertions(+), 22 deletions(-) rename apps/{emqx_bpapi => emqx}/include/bpapi.hrl (100%) rename apps/{emqx_bpapi => emqx}/priv/.gitkeep (100%) rename apps/{emqx_bpapi/src => emqx/src/bpapi}/emqx_bpapi.erl (100%) rename apps/{emqx_bpapi/src => emqx/src/bpapi}/emqx_bpapi_static_checks.erl (100%) rename apps/{emqx_bpapi/src => emqx/src/bpapi}/emqx_bpapi_trans.erl (100%) delete mode 100644 apps/emqx_bpapi/README.md delete mode 100644 apps/emqx_bpapi/src/emqx_bpapi.app.src diff --git a/apps/emqx_bpapi/include/bpapi.hrl b/apps/emqx/include/bpapi.hrl similarity index 100% rename from apps/emqx_bpapi/include/bpapi.hrl rename to apps/emqx/include/bpapi.hrl diff --git a/apps/emqx_bpapi/priv/.gitkeep b/apps/emqx/priv/.gitkeep similarity index 100% rename from apps/emqx_bpapi/priv/.gitkeep rename to apps/emqx/priv/.gitkeep diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index 4ec68d585..be8989dcd 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -4,6 +4,8 @@ {xref_checks,[undefined_function_calls,undefined_functions,locals_not_used, deprecated_function_calls,warnings_as_errors,deprecated_functions]}. +{erl_first_files, ["apps/emqx/src/bpapi/emqx_bpapi.erl"]}. + %% Deps here may duplicate with emqx.git root level rebar.config %% but there not be any descrpancy. %% This rebar.config is necessary because the app may be used as a diff --git a/apps/emqx_bpapi/src/emqx_bpapi.erl b/apps/emqx/src/bpapi/emqx_bpapi.erl similarity index 100% rename from apps/emqx_bpapi/src/emqx_bpapi.erl rename to apps/emqx/src/bpapi/emqx_bpapi.erl diff --git a/apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl similarity index 100% rename from apps/emqx_bpapi/src/emqx_bpapi_static_checks.erl rename to apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl diff --git a/apps/emqx_bpapi/src/emqx_bpapi_trans.erl b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl similarity index 100% rename from apps/emqx_bpapi/src/emqx_bpapi_trans.erl rename to apps/emqx/src/bpapi/emqx_bpapi_trans.erl diff --git a/apps/emqx/src/emqx.app.src b/apps/emqx/src/emqx.app.src index 597e1596f..0989cd7f1 100644 --- a/apps/emqx/src/emqx.app.src +++ b/apps/emqx/src/emqx.app.src @@ -17,7 +17,6 @@ , jiffy , lc , hocon - , emqx_bpapi ]}, {mod, {emqx_app,[]}}, {env, []}, diff --git a/apps/emqx/src/proto/emqx_broker_proto_v1.erl b/apps/emqx/src/proto/emqx_broker_proto_v1.erl index a40884c38..178fa2f08 100644 --- a/apps/emqx/src/proto/emqx_broker_proto_v1.erl +++ b/apps/emqx/src/proto/emqx_broker_proto_v1.erl @@ -22,7 +22,7 @@ , forward_async/3 ]). --include_lib("emqx_bpapi/include/bpapi.hrl"). +-include("bpapi.hrl"). -include("emqx.hrl"). -spec forward(node(), emqx_types:topic(), emqx_types:delivery()) -> emqx_types:deliver_result(). diff --git a/apps/emqx_bpapi/README.md b/apps/emqx_bpapi/README.md deleted file mode 100644 index 78f529a4c..000000000 --- a/apps/emqx_bpapi/README.md +++ /dev/null @@ -1,5 +0,0 @@ -emqx_bpapi -===== - -A library that helps maintaining EMQX's backplane API backward and -forward compatibility. diff --git a/apps/emqx_bpapi/src/emqx_bpapi.app.src b/apps/emqx_bpapi/src/emqx_bpapi.app.src deleted file mode 100644 index 45a3efc10..000000000 --- a/apps/emqx_bpapi/src/emqx_bpapi.app.src +++ /dev/null @@ -1,15 +0,0 @@ -{application, emqx_bpapi, - [{description, "A library for verifying safety of RPC calls"}, - {vsn, "0.1.0"}, - {registered, []}, - {applications, - [kernel, - stdlib, - typerefl %% Just for some metaprogramming utils - ]}, - {env,[]}, - {modules, []}, - - {licenses, ["Apache 2.0"]}, - {links, []} - ]}. diff --git a/rebar.config b/rebar.config index 49e37e4af..f333d3e51 100644 --- a/rebar.config +++ b/rebar.config @@ -13,6 +13,8 @@ {d, snk_kind, msg} ]}. +{erl_first_files, ["apps/emqx/src/bpapi/emqx_bpapi.erl"]}. + {xref_checks,[undefined_function_calls,undefined_functions,locals_not_used, deprecated_function_calls,warnings_as_errors,deprecated_functions]}. From 64378be9a09af5e37456d430083bda821583bcd9 Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 17:45:42 +0100 Subject: [PATCH 05/10] fix(bpapi): Optimize BPAPI dump size --- .../src/bpapi/emqx_bpapi_static_checks.erl | 101 ++++++++++-------- 1 file changed, 59 insertions(+), 42 deletions(-) diff --git a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl index de252b2b2..2065a70cf 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl @@ -16,21 +16,23 @@ -module(emqx_bpapi_static_checks). --export([run/1, run/0]). +-export([dump/1, dump/0]). -include_lib("emqx/include/logger.hrl"). -%% `emqx_bpapi:call' enriched with dialyzer spec --type typed_call() :: {emqx_bpapi:call(), _DialyzerSpec}. --type typed_rpc() :: {typed_call(), typed_call()}. - --type fulldump() :: #{emqx_bpapi:api() => - #{emqx_bpapi:api_version() => - #{ calls := [typed_rpc()] - , casts := [typed_rpc()] - } +-type api_dump() :: #{{emqx_bpapi:api(), emqx_bpapi:api_version()} => + #{ calls := [emqx_bpapi:rpc()] + , casts := [emqx_bpapi:rpc()] }}. +-type dialyzer_spec() :: {_Type, [_Type]}. + +-type dialyzer_dump() :: #{mfa() => dialyzer_spec()}. + +-type fulldump() :: #{ api => api_dump() + , signatures => dialyzer_dump() + }. + %% Applications we wish to ignore in the analysis: -define(IGNORED_APPS, "gen_rpc, recon, observer_cli, snabbkaffe, ekka, mria"). %% List of known RPC backend modules: @@ -40,24 +42,26 @@ -define(XREF, myxref). -run() -> +dump() -> case {filelib:wildcard("*_plt"), filelib:wildcard("_build/emqx*/lib")} of {[PLT|_], [RelDir|_]} -> - run(#{plt => PLT, reldir => RelDir}); + dump(#{plt => PLT, reldir => RelDir}); _ -> error("failed to guess run options") end. --spec run(map()) -> boolean(). -run(Opts) -> +%% Collect the local BPAPI modules to a dump file: +-spec dump(map()) -> boolean(). +dump(Opts) -> put(bpapi_ok, true), PLT = prepare(Opts), %% First we run XREF to find all callers of any known RPC backend: Callers = find_remote_calls(Opts), {BPAPICalls, NonBPAPICalls} = lists:partition(fun is_bpapi_call/1, Callers), warn_nonbpapi_rpcs(NonBPAPICalls), - CombinedAPI = collect_bpapis(BPAPICalls, PLT), - dump_api(CombinedAPI), + APIDump = collect_bpapis(BPAPICalls), + DialyzerDump = collect_signatures(PLT, APIDump), + dump_api(#{api => APIDump, signatures => DialyzerDump}), erase(bpapi_ok). prepare(#{reldir := RelDir, plt := PLT}) -> @@ -100,45 +104,58 @@ is_bpapi_call({Module, _Function, _Arity}) -> end. -spec dump_api(fulldump()) -> ok. -dump_api(Term) -> - Filename = filename:join(code:priv_dir(emqx_bpapi), emqx_app:get_release() ++ ".bpapi"), +dump_api(Term = #{api := _, signatures := _}) -> + Filename = filename:join(code:priv_dir(emqx), emqx_app:get_release() ++ ".bpapi"), file:write_file(Filename, io_lib:format("~0p.", [Term])). --spec collect_bpapis([mfa()], _PLT) -> fulldump(). -collect_bpapis(L, PLT) -> +-spec collect_bpapis([mfa()]) -> api_dump(). +collect_bpapis(L) -> Modules = lists:usort([M || {M, _F, _A} <- L]), lists:foldl(fun(Mod, Acc) -> #{ api := API , version := Vsn - , calls := Calls0 - , casts := Casts0 + , calls := Calls + , casts := Casts } = Mod:bpapi_meta(), - Calls = enrich(PLT, Calls0), - Casts = enrich(PLT, Casts0), - Acc#{API => #{Vsn => #{ calls => Calls - , casts => Casts - }}} + Acc#{{API, Vsn} => #{ calls => Calls + , casts => Casts + }} end, #{}, Modules ). -%% Add information about types from the PLT --spec enrich(_PLT, [emqx_bpapi:rpc()]) -> [typed_rpc()]. -enrich(PLT, Calls) -> - [case {lookup_type(PLT, From), lookup_type(PLT, To)} of - {{value, TFrom}, {value, TTo}} -> - {{From, TFrom}, {To, TTo}}; - {_, none} -> - setnok(), - ?CRITICAL("Backplane API function ~s calls a missing remote function ~s", - [format_call(From), format_call(To)]), - error(missing_target) - end - || {From, To} <- Calls]. +-spec collect_signatures(_PLT, api_dump()) -> dialyzer_dump(). +collect_signatures(PLT, APIs) -> + maps:fold(fun(_APIAndVersion, #{calls := Calls, casts := Casts}, Acc0) -> + Acc1 = lists:foldl(fun enrich/2, {Acc0, PLT}, Calls), + {Acc, PLT} = lists:foldl(fun enrich/2, Acc1, Casts), + Acc + end, + #{}, + APIs). -lookup_type(PLT, {M, F, A}) -> - dialyzer_plt:lookup(PLT, {M, F, length(A)}). +%% Add information about the call types from the PLT +-spec enrich(emqx_bpapi:rpc(), {dialyzer_dump(), _PLT}) -> {dialyzer_dump(), _PLT}. +enrich({From0, To0}, {Acc0, PLT}) -> + From = call_to_mfa(From0), + To = call_to_mfa(To0), + case {dialyzer_plt:lookup(PLT, From), dialyzer_plt:lookup(PLT, To)} of + {{value, TFrom}, {value, TTo}} -> + Acc = Acc0#{ From => TFrom + , To => TTo + }, + {Acc, PLT}; + {_, none} -> + setnok(), + ?CRITICAL("Backplane API function ~s calls a missing remote function ~s", + [format_call(From0), format_call(To0)]), + error(missing_target) + end. + +-spec call_to_mfa(emqx_bpapi:call()) -> mfa(). +call_to_mfa({M, F, A}) -> + {M, F, length(A)}. format_call({M, F, A}) -> io_lib:format("~p:~p/~p", [M, F, length(A)]). From eaa71438b276d7acb429171123dfb34e06b876cd Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 20:01:23 +0100 Subject: [PATCH 06/10] feat(bpapi): Typecheck function parameters --- .../src/bpapi/emqx_bpapi_static_checks.erl | 114 ++++++++++++++++-- 1 file changed, 106 insertions(+), 8 deletions(-) diff --git a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl index 2065a70cf..c77ede7a3 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl @@ -16,7 +16,7 @@ -module(emqx_bpapi_static_checks). --export([dump/1, dump/0]). +-export([dump/1, dump/0, check_compat/1]). -include_lib("emqx/include/logger.hrl"). @@ -31,8 +31,11 @@ -type fulldump() :: #{ api => api_dump() , signatures => dialyzer_dump() + , release => string() }. +-type param_types() :: #{emqx_bpapi:var_name() => _Type}. + %% Applications we wish to ignore in the analysis: -define(IGNORED_APPS, "gen_rpc, recon, observer_cli, snabbkaffe, ekka, mria"). %% List of known RPC backend modules: @@ -42,6 +45,101 @@ -define(XREF, myxref). +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +%% Functions related to BPAPI compatibility checking +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + +-spec check_compat([file:filename()]) -> boolean(). +check_compat(DumpFilenames) -> + put(bpapi_ok, true), + Dumps = lists:map(fun(FN) -> + {ok, [Dump]} = file:consult(FN), + Dump + end, + DumpFilenames), + [check_compat(I, J) || I <- Dumps, J <- Dumps], + erase(bpapi_ok). + +%% Note: sets nok flag +-spec check_compat(fulldump(), fulldump()) -> ok. +check_compat(Dump1, Dump2) -> + check_api_immutability(Dump1, Dump2), + typecheck_apis(Dump1, Dump2). + +%% It's not allowed to change BPAPI modules. Check that no changes +%% have been made. (sets nok flag) +-spec check_api_immutability(fulldump(), fulldump()) -> ok. +check_api_immutability(#{release := Rel1, api := APIs1}, #{release := Rel2, api := APIs2}) + when Rel2 >= Rel1 -> + %% TODO: Handle API deprecation + maps:map(fun(Key = {API, Version}, Val) -> + case maps:get(Key, APIs2, undefined) of + Val -> + ok; + undefined -> + setnok(), + ?ERROR("API ~p v~p was removed in release ~p without being deprecated.", + [API, Version, Rel2]); + _Val -> + setnok(), + ?ERROR("API ~p v~p was changed between ~p and ~p. Backplane API should be immutable.", + [API, Version, Rel1, Rel2]) + end + end, + APIs1), + ok; +check_api_immutability(_, _) -> + ok. + +%% Note: sets nok flag +-spec typecheck_apis(fulldump(), fulldump()) -> ok. +typecheck_apis( #{release := CallerRelease, api := CallerAPIs, signatures := CallerSigs} + , #{release := CalleeRelease, signatures := CalleeSigs} + ) -> + AllCalls = lists:flatten([[Calls, Casts] + || #{calls := Calls, casts := Casts} <- maps:values(CallerAPIs)]), + lists:foreach(fun({From, To}) -> + Caller = get_param_types(CallerSigs, From), + Callee = get_param_types(CalleeSigs, To), + %% TODO: check return types + case typecheck_rpc(Caller, Callee) of + [] -> + ok; + TypeErrors -> + setnok(), + [?ERROR("Incompatible RPC call: " + "type of the parameter ~p of RPC call ~s on release ~p " + "is not a subtype of the target function ~s on release ~p", + [Var, format_call(From), CallerRelease, + format_call(To), CalleeRelease]) + || Var <- TypeErrors] + end + end, + AllCalls). + +-spec typecheck_rpc(param_types(), param_types()) -> [emqx_bpapi:var_name()]. +typecheck_rpc(Caller, Callee) -> + maps:fold(fun(Var, CalleeType, Acc) -> + #{Var := CallerType} = Caller, + case erl_types:t_is_subtype(CallerType, CalleeType) of + true -> Acc; + false -> [Var|Acc] + end + end, + [], + Callee). + +-spec get_param_types(dialyzer_dump(), emqx_bpapi:call()) -> param_types(). +get_param_types(Signatures, {M, F, A}) -> + Arity = length(A), + #{{M, F, Arity} := {_RetType, AttrTypes}} = Signatures, + Arity = length(AttrTypes), % assert + maps:from_list(lists:zip(A, AttrTypes)). + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +%% Functions related to BPAPI dumping +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + dump() -> case {filelib:wildcard("*_plt"), filelib:wildcard("_build/emqx*/lib")} of {[PLT|_], [RelDir|_]} -> @@ -50,7 +148,7 @@ dump() -> error("failed to guess run options") end. -%% Collect the local BPAPI modules to a dump file: +%% Collect the local BPAPI modules to a dump file -spec dump(map()) -> boolean(). dump(Opts) -> put(bpapi_ok, true), @@ -61,7 +159,8 @@ dump(Opts) -> warn_nonbpapi_rpcs(NonBPAPICalls), APIDump = collect_bpapis(BPAPICalls), DialyzerDump = collect_signatures(PLT, APIDump), - dump_api(#{api => APIDump, signatures => DialyzerDump}), + Release = emqx_app:get_release(), + dump_api(#{api => APIDump, signatures => DialyzerDump, release => Release}), erase(bpapi_ok). prepare(#{reldir := RelDir, plt := PLT}) -> @@ -104,8 +203,8 @@ is_bpapi_call({Module, _Function, _Arity}) -> end. -spec dump_api(fulldump()) -> ok. -dump_api(Term = #{api := _, signatures := _}) -> - Filename = filename:join(code:priv_dir(emqx), emqx_app:get_release() ++ ".bpapi"), +dump_api(Term = #{api := _, signatures := _, release := Release}) -> + Filename = filename:join(code:priv_dir(emqx), Release ++ ".bpapi"), file:write_file(Filename, io_lib:format("~0p.", [Term])). -spec collect_bpapis([mfa()]) -> api_dump(). @@ -122,8 +221,7 @@ collect_bpapis(L) -> }} end, #{}, - Modules - ). + Modules). -spec collect_signatures(_PLT, api_dump()) -> dialyzer_dump(). collect_signatures(PLT, APIs) -> @@ -146,7 +244,7 @@ enrich({From0, To0}, {Acc0, PLT}) -> , To => TTo }, {Acc, PLT}; - {_, none} -> + {{value, _}, none} -> setnok(), ?CRITICAL("Backplane API function ~s calls a missing remote function ~s", [format_call(From0), format_call(To0)]), From 0f6ec9d646998ffdccae2639cdd074c911214a7f Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Tue, 4 Jan 2022 20:28:31 +0100 Subject: [PATCH 07/10] fix(bpapi): Fix build --- apps/emqx/rebar.config | 2 - apps/emqx/src/bpapi/emqx_bpapi.erl | 23 +-- apps/emqx/src/bpapi/emqx_bpapi_trans.erl | 200 +++++++++++++---------- rebar.config | 2 - 4 files changed, 111 insertions(+), 116 deletions(-) diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index be8989dcd..4ec68d585 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -4,8 +4,6 @@ {xref_checks,[undefined_function_calls,undefined_functions,locals_not_used, deprecated_function_calls,warnings_as_errors,deprecated_functions]}. -{erl_first_files, ["apps/emqx/src/bpapi/emqx_bpapi.erl"]}. - %% Deps here may duplicate with emqx.git root level rebar.config %% but there not be any descrpancy. %% This rebar.config is necessary because the app may be used as a diff --git a/apps/emqx/src/bpapi/emqx_bpapi.erl b/apps/emqx/src/bpapi/emqx_bpapi.erl index 284de4dfc..c3da7c067 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi.erl @@ -15,9 +15,7 @@ %%-------------------------------------------------------------------- -module(emqx_bpapi). --export([parse_semver/1, api_and_version/1]). - --export_type([var_name/0, call/0, rpc/0, bpapi_meta/0]). +-export_type([var_name/0, call/0, rpc/0, bpapi_meta/0, semver/0]). -type semver() :: {non_neg_integer(), non_neg_integer(), non_neg_integer()}. @@ -33,22 +31,3 @@ , calls := [rpc()] , casts := [rpc()] }. - --spec parse_semver(string()) -> {ok, semver()} - | false. -parse_semver(Str) -> - Opts = [{capture, all_but_first, list}], - case re:run(Str, "^([0-9]+)\\.([0-9]+)\\.([0-9]+)$", Opts) of - {match, [A, B, C]} -> {ok, {list_to_integer(A), list_to_integer(B), list_to_integer(C)}}; - nomatch -> error - end. - --spec api_and_version(module()) -> {atom(), non_neg_integer()}. -api_and_version(Module) -> - Opts = [{capture, all_but_first, list}], - case re:run(atom_to_list(Module), "(.*)_proto_v([0-9]+)$", Opts) of - {match, [API, VsnStr]} -> - {ok, list_to_atom(API), list_to_integer(VsnStr)}; - nomatch -> - error(Module) - end. diff --git a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl index bdcab83af..04366deba 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl @@ -38,139 +38,139 @@ }). format_error(invalid_name) -> - "BPAPI module name should follow _proto_v pattern"; + "BPAPI module name should follow _proto_v pattern"; format_error(invalid_introduced_in) -> - "-introduced_in attribute should be present and its value should be a semver string"; + "-introduced_in attribute should be present and its value should be a semver string"; format_error(invalid_deprecated_since) -> - "value of -deprecated_since attribute should be a semver string"; + "value of -deprecated_since attribute should be a semver string"; format_error({invalid_fun, Name, Arity}) -> - io_lib:format("malformed function ~p/~p. " - "BPAPI functions should have exactly one clause " - "and call (emqx_|e)rpc at the top level", - [Name, Arity]). + io_lib:format("malformed function ~p/~p. " + "BPAPI functions should have exactly one clause " + "and call (emqx_|e)rpc at the top level", + [Name, Arity]). parse_transform(Forms, _Options) -> - log("Original:~n~p", [Forms]), - State = #s{file = File} = lists:foldl(fun go/2, #s{}, Forms), - log("parse_trans state: ~p", [State]), - case check(State) of - [] -> - finalize(Forms, State); - Errors -> - {error, [{File, [{Line, ?MODULE, Msg} || {Line, Msg} <- Errors]}], []} - end. + log("Original:~n~p", [Forms]), + State = #s{file = File} = lists:foldl(fun go/2, #s{}, Forms), + log("parse_trans state: ~p", [State]), + case check(State) of + [] -> + finalize(Forms, State); + Errors -> + {error, [{File, [{Line, ?MODULE, Msg} || {Line, Msg} <- Errors]}], []} + end. %% Scan erl_forms: go({attribute, _, file, {File, _}}, S) -> - S#s{file = File}; + S#s{file = File}; go({attribute, Line, module, Mod}, S) -> - case emqx_bpapi:api_and_version(Mod) of - {ok, API, Vsn} -> S#s{api = API, version = Vsn, module = Mod}; - error -> push_err(Line, invalid_name, S) - end; + case api_and_version(Mod) of + {ok, API, Vsn} -> S#s{api = API, version = Vsn, module = Mod}; + error -> push_err(Line, invalid_name, S) + end; go({attribute, _Line, introduced_in, Str}, S) -> - case is_list(Str) andalso emqx_bpapi:parse_semver(Str) of - {ok, Vsn} -> S#s{introduced_in = Vsn}; - false -> S %% Don't report error here, it's done in check/1 - end; + case is_list(Str) andalso parse_semver(Str) of + {ok, Vsn} -> S#s{introduced_in = Vsn}; + error -> S %% Don't report error here, it's done in check/1 + end; go({attribute, Line, deprecated_since, Str}, S) -> - case is_list(Str) andalso emqx_bpapi:parse_semver(Str) of - {ok, Vsn} -> S#s{deprecated_since = Vsn}; - false -> push_err(Line, invalid_deprecated_since, S) - end; + case is_list(Str) andalso parse_semver(Str) of + {ok, Vsn} -> S#s{deprecated_since = Vsn}; + error -> push_err(Line, invalid_deprecated_since, S) + end; go({function, Line, Name, Arity, Clauses}, S) -> - analyze_fun(Line, Name, Arity, Clauses, S); + analyze_fun(Line, Name, Arity, Clauses, S); go(_, S) -> - S. + S. check(#s{errors = Err0, introduced_in = II}) -> - [{none, invalid_introduced_in} || II =:= undefined] ++ - Err0. + [{none, invalid_introduced_in} || II =:= undefined] ++ + Err0. finalize(Forms, S) -> - {Attrs, Funcs} = lists:splitwith(fun is_attribute/1, Forms), - AST = mk_meta_fun(S), - log("Meta fun:~n~p", [AST]), - Attrs ++ [mk_export()] ++ [AST|Funcs]. + {Attrs, Funcs} = lists:splitwith(fun is_attribute/1, Forms), + AST = mk_meta_fun(S), + log("Meta fun:~n~p", [AST]), + Attrs ++ [mk_export()] ++ [AST|Funcs]. mk_meta_fun(#s{api = API, version = Vsn, targets = Targets}) -> - Line = 0, - Calls = [{From, To} || {call, From, To} <- Targets], - Casts = [{From, To} || {cast, From, To} <- Targets], - Ret = typerefl_quote:const(Line, #{ api => API - , version => Vsn - , calls => Calls - , casts => Casts - }), - {function, Line, ?META_FUN, _Arity = 0, - [{clause, Line, _Args = [], _Guards = [], - [Ret]}]}. + Line = 0, + Calls = [{From, To} || {call, From, To} <- Targets], + Casts = [{From, To} || {cast, From, To} <- Targets], + Ret = typerefl_quote:const(Line, #{ api => API + , version => Vsn + , calls => Calls + , casts => Casts + }), + {function, Line, ?META_FUN, _Arity = 0, + [{clause, Line, _Args = [], _Guards = [], + [Ret]}]}. mk_export() -> - {attribute, 0, export, [{?META_FUN, 0}]}. + {attribute, 0, export, [{?META_FUN, 0}]}. is_attribute({attribute, _Line, _Attr, _Val}) -> true; is_attribute(_) -> false. %% Extract the target function of the RPC call analyze_fun(Line, Name, Arity, [{clause, Line, Head, _Guards, Exprs}], S) -> - analyze_exprs(Line, Name, Arity, Head, Exprs, S); + analyze_exprs(Line, Name, Arity, Head, Exprs, S); analyze_fun(Line, Name, Arity, _Clauses, S) -> - invalid_fun(Line, Name, Arity, S). + invalid_fun(Line, Name, Arity, S). analyze_exprs(Line, Name, Arity, Head, Exprs, S) -> - log("~p/~p (~p):~n~p", [Name, Arity, Head, Exprs]), - try - [{call, _, CallToBackend, CallArgs}] = Exprs, - OuterArgs = extract_outer_args(Head), - Key = {S#s.module, Name, OuterArgs}, - {Semantics, Target} = extract_target_call(CallToBackend, CallArgs), - push_target({Semantics, Key, Target}, S) - catch - _:Err:Stack -> - log("Failed to process function call:~n~s~nStack: ~p", [Err, Stack]), - invalid_fun(Line, Name, Arity, S) - end. + log("~p/~p (~p):~n~p", [Name, Arity, Head, Exprs]), + try + [{call, _, CallToBackend, CallArgs}] = Exprs, + OuterArgs = extract_outer_args(Head), + Key = {S#s.module, Name, OuterArgs}, + {Semantics, Target} = extract_target_call(CallToBackend, CallArgs), + push_target({Semantics, Key, Target}, S) + catch + _:Err:Stack -> + log("Failed to process function call:~n~s~nStack: ~p", [Err, Stack]), + invalid_fun(Line, Name, Arity, S) + end. -spec extract_outer_args(erl_parse:abstract_form()) -> [atom()]. extract_outer_args(Abs) -> - lists:map(fun({var, _, Var}) -> - Var; - ({match, _, {var, _, Var}, _}) -> - Var; - ({match, _, _, {var, _, Var}}) -> - Var - end, - Abs). + lists:map(fun({var, _, Var}) -> + Var; + ({match, _, {var, _, Var}, _}) -> + Var; + ({match, _, _, {var, _, Var}}) -> + Var + end, + Abs). -spec extract_target_call(Abs, [Abs]) -> {semantics(), emqx_bpapi:call()} - when Abs :: erl_parse:abstract_form(). + when Abs :: erl_parse:abstract_form(). extract_target_call(RPCBackend, OuterArgs) -> - {Semantics, {atom, _, M}, {atom, _, F}, A} = extract_mfa(RPCBackend, OuterArgs), - {Semantics, {M, F, list_to_args(A)}}. + {Semantics, {atom, _, M}, {atom, _, F}, A} = extract_mfa(RPCBackend, OuterArgs), + {Semantics, {M, F, list_to_args(A)}}. -define(BACKEND(MOD, FUN), {remote, _, {atom, _, MOD}, {atom, _, FUN}}). -define(IS_RPC(MOD), (MOD =:= erpc orelse MOD =:= rpc)). -spec extract_mfa(Abs, #s{}) -> {call | cast, Abs, Abs, Abs} - when Abs :: erl_parse:abstract_form(). + when Abs :: erl_parse:abstract_form(). %% gen_rpc: extract_mfa(?BACKEND(gen_rpc, _), _) -> - %% gen_rpc has an extremely messy API, thankfully it's fully wrapped - %% by emqx_rpc, so we simply forbid direct calls to it: - error("direct call to gen_rpc"); + %% gen_rpc has an extremely messy API, thankfully it's fully wrapped + %% by emqx_rpc, so we simply forbid direct calls to it: + error("direct call to gen_rpc"); %% emqx_rpc: extract_mfa(?BACKEND(emqx_rpc, CallOrCast), [_Node, M, F, A]) -> - {call_or_cast(CallOrCast), M, F, A}; + {call_or_cast(CallOrCast), M, F, A}; extract_mfa(?BACKEND(emqx_rpc, CallOrCast), [_Tag, _Node, M, F, A]) -> - {call_or_cast(CallOrCast), M, F, A}; + {call_or_cast(CallOrCast), M, F, A}; %% (e)rpc: extract_mfa(?BACKEND(RPC, CallOrCast), [_Node, M, F, A]) when ?IS_RPC(RPC) -> - {call_or_cast(CallOrCast), M, F, A}; + {call_or_cast(CallOrCast), M, F, A}; extract_mfa(?BACKEND(RPC, CallOrCast), [_Node, M, F, A, _Timeout]) when ?IS_RPC(RPC) -> - {call_or_cast(CallOrCast), M, F, A}; + {call_or_cast(CallOrCast), M, F, A}; extract_mfa(_, _) -> - error("unrecognized RPC call"). + error("unrecognized RPC call"). call_or_cast(cast) -> cast; call_or_cast(multicast) -> cast; @@ -178,23 +178,43 @@ call_or_cast(multicall) -> call; call_or_cast(call) -> call. list_to_args({cons, _, {var, _, A}, T}) -> - [A|list_to_args(T)]; + [A|list_to_args(T)]; list_to_args({nil, _}) -> - []. + []. invalid_fun(Line, Name, Arity, S) -> - push_err(Line, {invalid_fun, Name, Arity}, S). + push_err(Line, {invalid_fun, Name, Arity}, S). push_err(Line, Err, S = #s{errors = Errs}) -> - S#s{errors = [{Line, Err}|Errs]}. + S#s{errors = [{Line, Err}|Errs]}. push_target(Target, S = #s{targets = Targets}) -> - S#s{targets = [Target|Targets]}. + S#s{targets = [Target|Targets]}. + + +-spec parse_semver(string()) -> {ok, emqx_bpapi:semver()} + | error. +parse_semver(Str) -> + Opts = [{capture, all_but_first, list}], + case re:run(Str, "^([0-9]+)\\.([0-9]+)\\.([0-9]+)$", Opts) of + {match, [A, B, C]} -> {ok, {list_to_integer(A), list_to_integer(B), list_to_integer(C)}}; + nomatch -> error + end. + +-spec api_and_version(module()) -> {ok, emqx_bpapi:api(), emqx_bpapi:version()} | error. +api_and_version(Module) -> + Opts = [{capture, all_but_first, list}], + case re:run(atom_to_list(Module), "(.*)_proto_v([0-9]+)$", Opts) of + {match, [API, VsnStr]} -> + {ok, list_to_atom(API), list_to_integer(VsnStr)}; + nomatch -> + error + end. -ifdef(debug). log(Fmt, Args) -> - io:format(user, "!! " ++ Fmt ++ "~n", Args). + io:format(user, "!! " ++ Fmt ++ "~n", Args). -else. log(_, _) -> - ok. + ok. -endif. diff --git a/rebar.config b/rebar.config index f333d3e51..49e37e4af 100644 --- a/rebar.config +++ b/rebar.config @@ -13,8 +13,6 @@ {d, snk_kind, msg} ]}. -{erl_first_files, ["apps/emqx/src/bpapi/emqx_bpapi.erl"]}. - {xref_checks,[undefined_function_calls,undefined_functions,locals_not_used, deprecated_function_calls,warnings_as_errors,deprecated_functions]}. From 4f3f938d716e65e3ea4561f4382924f16b30ebe0 Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Wed, 5 Jan 2022 01:42:46 +0100 Subject: [PATCH 08/10] feat(bpapi): Introduce bpapi behavior --- apps/emqx/src/bpapi/emqx_bpapi.erl | 12 +++++-- apps/emqx/src/bpapi/emqx_bpapi_trans.erl | 36 ++++---------------- apps/emqx/src/proto/emqx_broker_proto_v1.erl | 8 +++-- 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/apps/emqx/src/bpapi/emqx_bpapi.erl b/apps/emqx/src/bpapi/emqx_bpapi.erl index c3da7c067..d76cb0085 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi.erl @@ -15,9 +15,7 @@ %%-------------------------------------------------------------------- -module(emqx_bpapi). --export_type([var_name/0, call/0, rpc/0, bpapi_meta/0, semver/0]). - --type semver() :: {non_neg_integer(), non_neg_integer(), non_neg_integer()}. +-export_type([var_name/0, call/0, rpc/0, bpapi_meta/0]). -type api() :: atom(). -type api_version() :: non_neg_integer(). @@ -31,3 +29,11 @@ , calls := [rpc()] , casts := [rpc()] }. + +-callback introduced_in() -> string(). + +-callback deprecated_since() -> string(). + +-callback bpapi_meta() -> bpapi_meta(). + +-optional_callbacks([deprecated_since/0]). diff --git a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl index 04366deba..311edf399 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl @@ -30,8 +30,6 @@ { api :: atom() , module :: atom() , version :: non_neg_integer() | undefined - , introduced_in :: emqx_bpapi:semver() | undefined - , deprecated_since :: emqx_bpapi:semver() | undefined , targets = [] :: [{semantics(), emqx_bpapi:call(), emqx_bpapi:call()}] , errors = [] :: [string()] , file @@ -39,10 +37,6 @@ format_error(invalid_name) -> "BPAPI module name should follow _proto_v pattern"; -format_error(invalid_introduced_in) -> - "-introduced_in attribute should be present and its value should be a semver string"; -format_error(invalid_deprecated_since) -> - "value of -deprecated_since attribute should be a semver string"; format_error({invalid_fun, Name, Arity}) -> io_lib:format("malformed function ~p/~p. " "BPAPI functions should have exactly one clause " @@ -68,24 +62,18 @@ go({attribute, Line, module, Mod}, S) -> {ok, API, Vsn} -> S#s{api = API, version = Vsn, module = Mod}; error -> push_err(Line, invalid_name, S) end; -go({attribute, _Line, introduced_in, Str}, S) -> - case is_list(Str) andalso parse_semver(Str) of - {ok, Vsn} -> S#s{introduced_in = Vsn}; - error -> S %% Don't report error here, it's done in check/1 - end; -go({attribute, Line, deprecated_since, Str}, S) -> - case is_list(Str) andalso parse_semver(Str) of - {ok, Vsn} -> S#s{deprecated_since = Vsn}; - error -> push_err(Line, invalid_deprecated_since, S) - end; +go({function, _Line, introduced_in, 0, _}, S) -> + S; +go({function, _Line, deprecated_since, 0, _}, S) -> + S; go({function, Line, Name, Arity, Clauses}, S) -> analyze_fun(Line, Name, Arity, Clauses, S); go(_, S) -> S. -check(#s{errors = Err0, introduced_in = II}) -> - [{none, invalid_introduced_in} || II =:= undefined] ++ - Err0. +check(#s{errors = Err}) -> + %% Post-processing checks can be placed here + Err. finalize(Forms, S) -> {Attrs, Funcs} = lists:splitwith(fun is_attribute/1, Forms), @@ -191,16 +179,6 @@ push_err(Line, Err, S = #s{errors = Errs}) -> push_target(Target, S = #s{targets = Targets}) -> S#s{targets = [Target|Targets]}. - --spec parse_semver(string()) -> {ok, emqx_bpapi:semver()} - | error. -parse_semver(Str) -> - Opts = [{capture, all_but_first, list}], - case re:run(Str, "^([0-9]+)\\.([0-9]+)\\.([0-9]+)$", Opts) of - {match, [A, B, C]} -> {ok, {list_to_integer(A), list_to_integer(B), list_to_integer(C)}}; - nomatch -> error - end. - -spec api_and_version(module()) -> {ok, emqx_bpapi:api(), emqx_bpapi:version()} | error. api_and_version(Module) -> Opts = [{capture, all_but_first, list}], diff --git a/apps/emqx/src/proto/emqx_broker_proto_v1.erl b/apps/emqx/src/proto/emqx_broker_proto_v1.erl index 178fa2f08..3efbcd3b0 100644 --- a/apps/emqx/src/proto/emqx_broker_proto_v1.erl +++ b/apps/emqx/src/proto/emqx_broker_proto_v1.erl @@ -16,15 +16,19 @@ -module(emqx_broker_proto_v1). --introduced_in("5.0.0"). +-behaviour(emqx_bpapi). --export([ forward/3 +-export([ introduced_in/0 + , forward/3 , forward_async/3 ]). -include("bpapi.hrl"). -include("emqx.hrl"). +introduced_in() -> + "5.0.0". + -spec forward(node(), emqx_types:topic(), emqx_types:delivery()) -> emqx_types:deliver_result(). forward(Node, Topic, Delivery = #delivery{}) when is_binary(Topic) -> emqx_rpc:call(Topic, Node, emqx_broker, dispatch, [Topic, Delivery]). From 9c675194f502d0d92f0fa99d3d0f36a9eb7dc479 Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Wed, 5 Jan 2022 11:27:56 +0100 Subject: [PATCH 09/10] fix(bpapi): Fix dialyzer warnings --- .../src/bpapi/emqx_bpapi_static_checks.erl | 31 ++++++++++--------- apps/emqx/src/bpapi/emqx_bpapi_trans.erl | 9 ++---- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl index c77ede7a3..f26143219 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl @@ -72,21 +72,22 @@ check_compat(Dump1, Dump2) -> check_api_immutability(#{release := Rel1, api := APIs1}, #{release := Rel2, api := APIs2}) when Rel2 >= Rel1 -> %% TODO: Handle API deprecation - maps:map(fun(Key = {API, Version}, Val) -> - case maps:get(Key, APIs2, undefined) of - Val -> - ok; - undefined -> - setnok(), - ?ERROR("API ~p v~p was removed in release ~p without being deprecated.", - [API, Version, Rel2]); - _Val -> - setnok(), - ?ERROR("API ~p v~p was changed between ~p and ~p. Backplane API should be immutable.", - [API, Version, Rel1, Rel2]) - end - end, - APIs1), + _ = maps:map( + fun(Key = {API, Version}, Val) -> + case maps:get(Key, APIs2, undefined) of + Val -> + ok; + undefined -> + setnok(), + ?ERROR("API ~p v~p was removed in release ~p without being deprecated.", + [API, Version, Rel2]); + _Val -> + setnok(), + ?ERROR("API ~p v~p was changed between ~p and ~p. Backplane API should be immutable.", + [API, Version, Rel1, Rel2]) + end + end, + APIs1), ok; check_api_immutability(_, _) -> ok. diff --git a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl index 311edf399..a2ef547c0 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl @@ -31,7 +31,7 @@ , module :: atom() , version :: non_neg_integer() | undefined , targets = [] :: [{semantics(), emqx_bpapi:call(), emqx_bpapi:call()}] - , errors = [] :: [string()] + , errors = [] :: list() , file }). @@ -120,7 +120,7 @@ analyze_exprs(Line, Name, Arity, Head, Exprs, S) -> invalid_fun(Line, Name, Arity, S) end. --spec extract_outer_args(erl_parse:abstract_form()) -> [atom()]. +-spec extract_outer_args([erl_parse:abstract_form()]) -> [atom()]. extract_outer_args(Abs) -> lists:map(fun({var, _, Var}) -> Var; @@ -131,8 +131,7 @@ extract_outer_args(Abs) -> end, Abs). --spec extract_target_call(Abs, [Abs]) -> {semantics(), emqx_bpapi:call()} - when Abs :: erl_parse:abstract_form(). +-spec extract_target_call(_AST, [_AST]) -> {semantics(), emqx_bpapi:call()}. extract_target_call(RPCBackend, OuterArgs) -> {Semantics, {atom, _, M}, {atom, _, F}, A} = extract_mfa(RPCBackend, OuterArgs), {Semantics, {M, F, list_to_args(A)}}. @@ -140,8 +139,6 @@ extract_target_call(RPCBackend, OuterArgs) -> -define(BACKEND(MOD, FUN), {remote, _, {atom, _, MOD}, {atom, _, FUN}}). -define(IS_RPC(MOD), (MOD =:= erpc orelse MOD =:= rpc)). --spec extract_mfa(Abs, #s{}) -> {call | cast, Abs, Abs, Abs} - when Abs :: erl_parse:abstract_form(). %% gen_rpc: extract_mfa(?BACKEND(gen_rpc, _), _) -> %% gen_rpc has an extremely messy API, thankfully it's fully wrapped From 22bdcfa4b5772948d3f14b5bc208627d6b0ef86e Mon Sep 17 00:00:00 2001 From: k32 <10274441+k32@users.noreply.github.com> Date: Wed, 5 Jan 2022 14:37:26 +0100 Subject: [PATCH 10/10] fix(bpapi): Apply remarks --- apps/emqx/src/bpapi/emqx_bpapi.erl | 4 ++-- apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl | 2 +- apps/emqx/src/bpapi/emqx_bpapi_trans.erl | 8 ++++---- apps/emqx/src/proto/emqx_broker_proto_v1.erl | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/emqx/src/bpapi/emqx_bpapi.erl b/apps/emqx/src/bpapi/emqx_bpapi.erl index d76cb0085..d99f0e53b 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi.erl @@ -1,5 +1,5 @@ %%-------------------------------------------------------------------- -%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -15,7 +15,7 @@ %%-------------------------------------------------------------------- -module(emqx_bpapi). --export_type([var_name/0, call/0, rpc/0, bpapi_meta/0]). +-export_type([api/0, api_version/0, var_name/0, call/0, rpc/0, bpapi_meta/0]). -type api() :: atom(). -type api_version() :: non_neg_integer(). diff --git a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl index f26143219..5023e9b9f 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_static_checks.erl @@ -1,5 +1,5 @@ %%-------------------------------------------------------------------- -%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. diff --git a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl index a2ef547c0..f39215069 100644 --- a/apps/emqx/src/bpapi/emqx_bpapi_trans.erl +++ b/apps/emqx/src/bpapi/emqx_bpapi_trans.erl @@ -1,5 +1,5 @@ %%-------------------------------------------------------------------- -%% Copyright (c) 2020-2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -27,9 +27,9 @@ -type semantics() :: call | cast. -record(s, - { api :: atom() - , module :: atom() - , version :: non_neg_integer() | undefined + { api :: emqx_bpapi:api() + , module :: module() + , version :: emqx_bpapi:api_version() | undefined , targets = [] :: [{semantics(), emqx_bpapi:call(), emqx_bpapi:call()}] , errors = [] :: list() , file diff --git a/apps/emqx/src/proto/emqx_broker_proto_v1.erl b/apps/emqx/src/proto/emqx_broker_proto_v1.erl index 3efbcd3b0..d5725b8ac 100644 --- a/apps/emqx/src/proto/emqx_broker_proto_v1.erl +++ b/apps/emqx/src/proto/emqx_broker_proto_v1.erl @@ -1,5 +1,5 @@ %%-------------------------------------------------------------------- -%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License.