Revert "Merge branch 'kn-51937-extra-object-key-pass' into 'master'"

This reverts merge request !59

Author: Max Lapshin

.gitlab-ci.yml

@@ -22,19 +22,3 @@ run-test:
     expire_in: 1 week
     paths:
       - test-logs
-
-run-test-legacy:
-  stage: test
-  needs:
-    - prepare-src
-  script:
-    - mkdir -p test-logs-legacy
-    - docker run --rm -v ${PWD}/test-logs-legacy:/openapi_handler/test-logs openapi_handler make ci-test-legacy
-  artifacts:
-    reports:
-      junit: test-logs-legacy/*/junit_report.xml
-    when: always
-    name: "${CI_JOB_STAGE}_${BRANCH_NAME}-test-legacy"
-    expire_in: 1 week
-    paths:
-      - test-logs-legacy

Makefile

@@ -6,8 +6,5 @@ all:
 ci-test:
 	REBAR_CONFIG=rebar.config_ ./rebar3 as dev ct --logdir test-logs --readable true
 
-ci-test-legacy:
-	REBAR_CONFIG=rebar.config_ ./rebar3 as dev_legacy ct --logdir test-logs --readable true
-
 clean:
 	rm -rf _build rebar.lock

README.md

@@ -87,7 +87,6 @@ Also see validation quirks below.
 You can customize the way objects are processed. 
 Available options:
  * `#{extra_obj_key => drop}` -- the original object may contain keys not described by schema, just ignore them instead of raising an error
- * `#{extra_obj_key => pass}` -- the original object may contain keys not described by schema, just pass them as is instead of raising an error
  * `#{required_obj_keys => error}` -- raise an error when original object misses some required keys
  * `#{validators => #{Format :: atom() => Validator}}`  
    `Validator :: fun(Value) -> {ok, ConvertedValue} | {error, #{}}`  

rebar.config_

@@ -12,16 +12,5 @@
       redbug
     ]},
     {ct_opts, [{ct_hooks, [cth_surefire]}]}
-  ]},
-  {dev_legacy, [
-    {deps, [
-      jsx,
-      yamerl,
-      {cowboy, "1.0.1"},
-      {lhttpc, {git, "https://github.com/erlyvideo/lhttpc.git", {branch, "master"}}},
-      redbug
-    ]},
-    {erl_opts, [{d,legacy}]},
-    {ct_opts, [{ct_hooks, [cth_surefire]}]}
   ]}
 ]}.

src/openapi_handler.erl

@@ -3,7 +3,7 @@
 
 
 -export([init/2, handle/2, terminate/3]).
--export([routes/1, load_schema/2, choose_module/0]).
+-export([routes/1, load_schema/2]).
 -export([read_schema/1]).
 -export([routes_sort/1]). % for tests
 
@@ -14,8 +14,6 @@
 routes(#{schema := SchemaPath, module := Module, name := Name, prefix := Prefix} = Config) ->
   #{} = Schema = load_schema(SchemaPath, Name),
   SchemaOpts = get_schema_opts(Config),
-  HandlerModule = choose_module(),
-
   #{paths := Paths} = Schema,
   Routes = lists:map(fun({Path,PathSpec}) ->
     <<"/", _/binary>> = CowboyPath = re:replace(atom_to_list(Path), "{([^\\}]+)}", ":\\1",[global,{return,binary}]),
@@ -27,23 +25,13 @@ routes(#{schema := SchemaPath, module := Module, name := Name, prefix := Prefix}
     % It is too bad to pass all this stuff through cowboy options because it starts suffering
     % from GC on big state. Either ETS, either persistent_term, either compilation of custom code
     persistent_term:put({openapi_handler_route,Name,CowboyPath}, PathSpec1#{name => Name, module => Module, schema_opts => SchemaOpts}),
-    {<<Prefix/binary, CowboyPath/binary>>, HandlerModule, {Name,CowboyPath}}
+    {<<Prefix/binary, CowboyPath/binary>>, ?MODULE, {Name,CowboyPath}}
   end, maps:to_list(Paths)),
   % After sorting, bindings (starting with ":") must be after constant path segments, so generic routes only work after specific ones.
   % E.g. "/api/users/admin" must be before "/api/users/:id"
   % Thus special sorting function
   routes_sort(Routes).
 
-choose_module() ->
-  try
-    % Check if cowboy uses modern Req (map)
-    1234 = cowboy_req:port(#{port => 1234}),
-    ?MODULE
-  catch
-    error:{badrecord,_}:_ ->
-      % cowboy uses record for Req, use legacy wrapper
-      openapi_handler_legacy
-  end.
 
 prepare_operation_fm(_M, #{operationId := OperationId_} = Operation, Parameters) ->
   Op1 = maps:remove(description,Operation),
@@ -540,7 +528,7 @@ terminate(_,_,_) ->
 
 get_schema_opts(#{schema_opts := #{} = SchemaOpts}) ->
   maps:map(fun
-    (extra_obj_key,Flag) when Flag == drop; Flag == error; Flag == pass -> ok;
+    (extra_obj_key,Flag) when Flag == drop; Flag == error -> ok;
     (K,V) -> error({bad_schema_opt,K,V})
   end, SchemaOpts),
   SchemaOpts;

src/openapi_handler_legacy.erl

@@ -45,7 +45,7 @@ process(Input, #{} = Opts) ->
     (query,Flag) when Flag == true; Flag == false -> ok;
     (apply_defaults,Flag) when Flag == true; Flag == false -> ok;
     (patch,Flag) when Flag == true; Flag == false -> ok;
-    (extra_obj_key,Flag) when Flag == drop; Flag == error; Flag == pass -> ok;
+    (extra_obj_key,Flag) when Flag == drop; Flag == error -> ok;
     (required_obj_keys,Flag) when Flag == drop; Flag == error -> ok;
     (access_type,Flag) when Flag == read; Flag == write -> ok;
     (explain,FlagList) -> check_explain_keys(FlagList);
@@ -593,11 +593,6 @@ check_extra_keys(Input, Encoded, #{extra_obj_key := error} = Opts) when is_map(I
     _ -> Encoded
   end;
 
-check_extra_keys(Input, Encoded, #{extra_obj_key := pass}) when is_map(Input) andalso is_map(Encoded) andalso map_size(Encoded) > 0 ->
-  EncodedKeys = [K || K <- maps:keys(Encoded), is_atom(K)],
-  ExtraKeys = maps:keys(Input) -- [atom_to_binary(K) || K <- EncodedKeys],
-  maps:merge(maps:with(EncodedKeys, Encoded), maps:with(ExtraKeys, Input));
-
 check_extra_keys(_Input, Encoded, _Opts) ->
   Encoded.
 

src/openapi_schema.erl

@@ -36,15 +36,10 @@ groups() ->
    ]}
   ].
 
--ifdef(legacy).
-start_http(Routes, ApiName) ->
-  cowboy:start_http(ApiName, 1, [{port, 0}],
-    [{env, [{dispatch, cowboy_router:compile([{'_', Routes}])}]}]).
--else.
 start_http(Routes, ApiName) ->
   cowboy:start_clear(ApiName, [{port, 0}],
     #{env => #{dispatch => cowboy_router:compile([{'_', Routes}])}}).
--endif.
+
 
 init_per_suite(Config) ->
   {ok, _} = application:ensure_all_started(cowboy),
@@ -285,7 +280,7 @@ json_array_ok(_) ->
   ok.
 
 putFile(#{req := Req, '$cowboy_req' := CowboyReq}) ->
-  <<"PUT">> = ?MODULE:method(CowboyReq),
+  <<"PUT">> = cowboy_req:method(CowboyReq),
   Body = <<"{\"size\":100}">>,
   Req1 = reply(200, #{<<"content-length">> => byte_size(Body), <<"content-type">> => <<"application/json">>}, Body, Req),
   {done, Req1}.

test/openapi_handler_SUITE.erl

@@ -13,7 +13,6 @@ groups() ->
       read_default,
       extra_keys_error,
       extra_keys_drop,
-      extra_keys_pass,
       null_in_array,
       nullable_by_oneof,
       discriminator,
@@ -85,14 +84,6 @@ extra_keys_drop(_) ->
   #{inputs := [],name := <<"read_default">>,static := true} = openapi_schema:process(Json, #{type => stream_config, whole_schema => Schema, apply_defaults => true}),
   ok.
 
-extra_keys_pass(_) ->
-  Json = #{<<"name">> => <<"read_default">>, extra_key1 => <<"abc">>, <<"extrakey2">> => def},
-  Schema = persistent_term:get({openapi_handler_schema,test_openapi}),
-  #{inputs := [],name := <<"read_default">>,static := true,
-      extra_key1 := <<"abc">>,<<"extrakey2">> := def
-   } = openapi_schema:process(Json, #{type => stream_config, whole_schema => Schema, apply_defaults => true, extra_obj_key => pass}),
-  ok.
-
 
 null_in_array(_) ->
   % When items are not nullable, passing null|undefined as a list element should return an error