From a630e992d4225cc7d234842a9c26d5773da442a9 Mon Sep 17 00:00:00 2001 From: Alberto Sartori Date: Tue, 12 Apr 2022 10:50:15 +0200 Subject: [PATCH] add: cast parameters with json content-type --- lib/open_api_spex.ex | 24 +++- lib/open_api_spex/cast_parameters.ex | 94 +++++++++++---- lib/open_api_spex/open_api.ex | 1 + lib/open_api_spex/operation2.ex | 11 +- lib/open_api_spex/parameter.ex | 11 ++ test/cast_parameters_test.exs | 164 ++++++++++++++++++++------- test/operation2_test.exs | 53 +++------ 7 files changed, 250 insertions(+), 108 deletions(-) diff --git a/lib/open_api_spex.ex b/lib/open_api_spex.ex index a88c56bd..851de311 100644 --- a/lib/open_api_spex.ex +++ b/lib/open_api_spex.ex @@ -94,7 +94,7 @@ defmodule OpenApiSpex do opts \\ [] ) do content_type = content_type || content_type_from_header(conn) - Operation2.cast(operation, conn, content_type, spec.components, opts) + Operation2.cast(spec, operation, conn, content_type, opts) end defp content_type_from_header(conn = %Plug.Conn{}) do @@ -416,4 +416,26 @@ defmodule OpenApiSpex do """ @spec params(Plug.Conn.t()) :: nil | map() def params(%Plug.Conn{} = conn), do: get_in(conn.private, [:open_api_spex, :params]) + + @doc """ + + """ + @spec add_parameter_content_parser( + OpenApi.t(), + content_type | [content_type], + parser :: module() + ) :: OpenApi.t() + when content_type: String.t() | Regex.t() + def add_parameter_content_parser(%OpenApi{extensions: ext} = spec, content_type, parser) do + extensions = ext || %{} + + param_parsers = Map.get(extensions, "x-parameter-content-parsers", %{}) + + param_parsers = + content_type + |> List.wrap() + |> Enum.reduce(param_parsers, fn ct, acc -> Map.put(acc, ct, parser) end) + + %OpenApi{spec | extensions: Map.put(extensions, "x-parameter-content-parsers", param_parsers)} + end end diff --git a/lib/open_api_spex/cast_parameters.ex b/lib/open_api_spex/cast_parameters.ex index e6cc96aa..7c846a30 100644 --- a/lib/open_api_spex/cast_parameters.ex +++ b/lib/open_api_spex/cast_parameters.ex @@ -1,15 +1,17 @@ defmodule OpenApiSpex.CastParameters do @moduledoc false - alias OpenApiSpex.{Cast, Components, Operation, Parameter, Reference, Schema} + alias OpenApiSpex.{Cast, OpenApi, Operation, Parameter, Reference, Schema} alias OpenApiSpex.Cast.Error alias Plug.Conn - @spec cast(Plug.Conn.t(), Operation.t(), Components.t(), opts :: [OpenApiSpex.cast_opt()]) :: + @default_parsers %{~r/^application\/.*json.*$/ => OpenApi.json_encoder()} + + @spec cast(Plug.Conn.t(), Operation.t(), OpenApi.t(), opts :: [OpenApiSpex.cast_opt()]) :: {:error, [Error.t()]} | {:ok, Conn.t()} - def cast(conn, operation, components, opts \\ []) do + def cast(conn, operation, spec, opts \\ []) do replace_params = Keyword.get(opts, :replace_params, true) - with {:ok, params} <- cast_to_params(conn, operation, components) do + with {:ok, params} <- cast_to_params(conn, operation, spec) do {:ok, conn |> cast_conn(params) |> maybe_replace_params(params, replace_params)} end end @@ -27,11 +29,11 @@ defmodule OpenApiSpex.CastParameters do defp maybe_replace_params(conn, _params, false), do: conn defp maybe_replace_params(conn, params, true), do: %{conn | params: params} - defp cast_to_params(conn, operation, components) do + defp cast_to_params(conn, operation, %OpenApi{components: components} = spec) do operation |> schemas_by_location(components) |> Enum.map(fn {location, {schema, parameters_contexts}} -> - cast_location(location, schema, parameters_contexts, components, conn) + cast_location(location, schema, parameters_contexts, spec, conn) end) |> reduce_cast_results() end @@ -75,7 +77,12 @@ defmodule OpenApiSpex.CastParameters do # Extract context information from parameters, useful later when casting defp parameters_contexts(parameters) do Map.new(parameters, fn parameter -> - {Atom.to_string(parameter.name), Map.take(parameter, [:explode, :style])} + context = + parameter + |> Map.take([:explode, :style]) + |> Map.put(:content_type, Parameter.media_type(parameter)) + + {Atom.to_string(parameter.name), context} end) end @@ -104,32 +111,73 @@ defmodule OpenApiSpex.CastParameters do end) end - defp cast_location(location, schema, parameters_contexts, components, conn) do - params = - get_params_by_location( - conn, - location, - schema.properties |> Map.keys() |> Enum.map(&Atom.to_string/1) - ) - |> pre_parse_parameters(parameters_contexts) + defp cast_location( + location, + schema, + parameters_contexts, + %OpenApi{components: components, extensions: ext}, + conn + ) do + parsers = Map.get(ext || %{}, "x-parameter-content-parsers", %{}) + parsers = Map.merge(@default_parsers, parsers) + + conn + |> get_params_by_location( + location, + schema.properties |> Map.keys() |> Enum.map(&Atom.to_string/1) + ) + |> pre_parse_parameters(parameters_contexts, parsers) + |> case do + {:error, _} = err -> err + params -> Cast.cast(schema, params, components.schemas) + end + end + + defp pre_parse_parameters(%{} = parameters, %{} = parameters_context, parsers) do + Enum.reduce_while(parameters, Map.new(), fn {key, value}, acc -> + case pre_parse_parameter(value, Map.get(parameters_context, key, %{}), parsers) do + {:ok, param} -> {:cont, Map.put(acc, key, param)} + err -> {:halt, err} + end + end) + end - Cast.cast(schema, params, components.schemas) + defp pre_parse_parameter(parameter, %{content_type: content_type}, parsers) + when is_bitstring(content_type) and is_map_key(parsers, content_type) do + parser = Map.fetch!(parsers, content_type) + decode_parameter(parameter, content_type, parser) end - defp pre_parse_parameters(%{} = parameters, %{} = parameters_context) do - Map.new(parameters, fn {key, value} = _parameter -> - {key, pre_parse_parameter(value, Map.get(parameters_context, key, %{}))} + defp pre_parse_parameter(parameter, %{content_type: content_type}, parsers) + when is_bitstring(content_type) do + Enum.reduce_while(parsers, {:ok, parameter}, fn {match, parser}, acc -> + if Regex.regex?(match) and Regex.match?(match, content_type) do + {:halt, decode_parameter(parameter, content_type, parser)} + else + {:cont, acc} + end end) end - defp pre_parse_parameter(parameter, %{explode: false, style: :form} = _context) do + defp pre_parse_parameter(parameter, %{explode: false, style: :form} = _context, _parsers) do # e.g. sizes=S,L,M # This does not take care of cases where the value may contain a comma itself - String.split(parameter, ",") + {:ok, String.split(parameter, ",")} + end + + defp pre_parse_parameter(parameter, _context, _parsers) do + {:ok, parameter} end - defp pre_parse_parameter(parameter, _) do - parameter + defp decode_parameter(parameter, content_type, parser) when is_atom(parser) do + decode_parameter(parameter, content_type, &parser.decode/1) + end + + defp decode_parameter(parameter, content_type, parser) when is_function(parser, 1) do + case parser.(parameter) do + {:ok, result} -> {:ok, result} + {:error, _error} -> Cast.error(%Cast{}, {:invalid_format, content_type}) + end end defp reduce_cast_results(results) do diff --git a/lib/open_api_spex/open_api.ex b/lib/open_api_spex/open_api.ex index 21069627..5afc2019 100644 --- a/lib/open_api_spex/open_api.ex +++ b/lib/open_api_spex/open_api.ex @@ -74,6 +74,7 @@ defmodule OpenApiSpex.OpenApi do @vendor_extensions ~w( x-struct x-validate + x-parameter-content-parsers ) def json_encoder, do: @json_encoder diff --git a/lib/open_api_spex/operation2.ex b/lib/open_api_spex/operation2.ex index 755cd0bb..9bccec06 100644 --- a/lib/open_api_spex/operation2.ex +++ b/lib/open_api_spex/operation2.ex @@ -6,6 +6,7 @@ defmodule OpenApiSpex.Operation2 do Cast, CastParameters, Components, + OpenApi, Operation, Reference, RequestBody @@ -15,23 +16,23 @@ defmodule OpenApiSpex.Operation2 do alias Plug.Conn @spec cast( + OpenApi.t(), Operation.t(), Conn.t(), String.t() | nil, - Components.t(), opts :: [OpenApiSpex.cast_opt()] ) :: {:error, [Error.t()]} | {:ok, Conn.t()} def cast( + spec = %OpenApi{components: components}, operation = %Operation{}, conn = %Conn{}, content_type, - components = %Components{}, opts \\ [] ) do replace_params = Keyword.get(opts, :replace_params, true) - with {:ok, conn} <- cast_parameters(conn, operation, components, opts), + with {:ok, conn} <- cast_parameters(conn, operation, spec, opts), {:ok, body} <- cast_request_body(operation.requestBody, conn.body_params, content_type, components) do {:ok, conn |> cast_conn(body) |> maybe_replace_body(body, replace_params)} @@ -53,8 +54,8 @@ defmodule OpenApiSpex.Operation2 do defp maybe_replace_body(conn, _body, false), do: conn defp maybe_replace_body(conn, body, true), do: %{conn | body_params: body} - defp cast_parameters(conn, operation, components, opts) do - CastParameters.cast(conn, operation, components, opts) + defp cast_parameters(conn, operation, spec, opts) do + CastParameters.cast(conn, operation, spec, opts) end defp cast_request_body(ref = %Reference{}, body_params, content_type, components) do diff --git a/lib/open_api_spex/parameter.ex b/lib/open_api_spex/parameter.ex index 336068a2..c493c060 100644 --- a/lib/open_api_spex/parameter.ex +++ b/lib/open_api_spex/parameter.ex @@ -107,4 +107,15 @@ defmodule OpenApiSpex.Parameter do {_type, %MediaType{schema: schema}} = Enum.at(content, 0) schema end + + @doc """ + Gets the media type for a parameter, if not present `nil` is returned. + """ + @spec media_type(Parameter.t()) :: String.t() | nil + def media_type(%Parameter{content: content}) when is_map(content) and map_size(content) == 1 do + {type, _} = Enum.at(content, 0) + type + end + + def media_type(_), do: nil end diff --git a/test/cast_parameters_test.exs b/test/cast_parameters_test.exs index 265e1169..b881a914 100644 --- a/test/cast_parameters_test.exs +++ b/test/cast_parameters_test.exs @@ -15,9 +15,9 @@ defmodule OpenApiSpex.CastParametersTest do test "fails for unsupported additional properties " do conn = create_conn_with_unexpected_path_param() operation = create_operation() - components = create_components() + spec = spec_with_components() - cast_result = CastParameters.cast(conn, operation, components) + cast_result = CastParameters.cast(conn, operation, spec) expected_response = {:error, @@ -40,17 +40,17 @@ defmodule OpenApiSpex.CastParametersTest do test "default parameter values are supplied" do conn = create_conn() operation = create_operation() - components = create_components() - {:ok, conn} = CastParameters.cast(conn, operation, components) + spec = spec_with_components() + {:ok, conn} = CastParameters.cast(conn, operation, spec) assert %{params: %{includeInactive: false}} = conn end test "casting of headers is case-insensitive" do conn = create_conn() operation = create_operation() - components = create_components() + spec = spec_with_components() - {:ok, conn} = CastParameters.cast(conn, operation, components) + {:ok, conn} = CastParameters.cast(conn, operation, spec) assert %{params: %{"Content-Type": "application/json"}} = conn end @@ -91,9 +91,10 @@ defmodule OpenApiSpex.CastParametersTest do } } - components = %Components{ - schemas: %{"SizeParams" => schema} - } + spec = + spec_with_components(%Components{ + schemas: %{"SizeParams" => schema} + }) sizes_param = "S,M,L" @@ -103,57 +104,92 @@ defmodule OpenApiSpex.CastParametersTest do |> Plug.Conn.put_req_header("content-type", "application/json") |> Plug.Conn.fetch_query_params() - assert {:ok, conn} = CastParameters.cast(conn, operation, components) + assert {:ok, conn} = CastParameters.cast(conn, operation, spec) assert %{params: %{sizes: ["S", "M", "L"]}} = conn end - test "cast json query params" do - schema = %Schema{ - type: :object, - title: "FilterParams", - properties: %{ - size: %Schema{type: :string, pattern: "^XS|S|M|L|XL$"}, - color: %Schema{type: :string} - } - } + test "cast json query params with default parser" do + {operation, spec} = json_query_spec_operation() - parameter = %Parameter{ - in: :query, - name: :filter, - required: false, - content: %{ - "application/json" => %MediaType{ - schema: %Reference{"$ref": "#/components/schemas/FilterParams"} - } - } - } + filter_json = + %{size: "S", color: "blue"} + |> Jason.encode!() + |> URI.encode() - operation = %Operation{ - parameters: [parameter], - responses: %{ - 200 => %Schema{type: :object} - } - } + conn = + :get + |> Plug.Test.conn("/api/users?filter=#{filter_json}") + |> Plug.Conn.put_req_header("content-type", "application/json") + |> Plug.Conn.fetch_query_params() - components = %Components{ - schemas: %{"FilterParams" => schema} - } + assert {:ok, conn} = CastParameters.cast(conn, operation, spec) + assert %{params: %{filter: %{size: "S", color: "blue"}}} = conn + end + + test "cast json query params with custom parser" do + {operation, spec} = json_query_spec_operation("custom/json") + + spec = + OpenApiSpex.add_parameter_content_parser( + spec, + "custom/json", + OpenApiSpex.OpenApi.json_encoder() + ) + + filter_json = + %{size: "S", color: "blue"} + |> Jason.encode!() + |> URI.encode() + + conn = + :get + |> Plug.Test.conn("/api/users?filter=#{filter_json}") + |> Plug.Conn.put_req_header("content-type", "custom/json") + |> Plug.Conn.fetch_query_params() + + assert {:ok, conn} = CastParameters.cast(conn, operation, spec) + assert %{params: %{filter: %{size: "S", color: "blue"}}} = conn + end + + test "cast json query params with custom parser with regex" do + {operation, spec} = json_query_spec_operation("custom/json") + + spec = + OpenApiSpex.add_parameter_content_parser( + spec, + ~r/custom\/.+/, + fn string -> OpenApiSpex.OpenApi.json_encoder().decode(string) end + ) filter_json = %{size: "S", color: "blue"} |> Jason.encode!() |> URI.encode() + conn = + :get + |> Plug.Test.conn("/api/users?filter=#{filter_json}") + |> Plug.Conn.put_req_header("content-type", "custom/json") + |> Plug.Conn.fetch_query_params() + + assert {:ok, conn} = CastParameters.cast(conn, operation, spec) + assert %{params: %{filter: %{size: "S", color: "blue"}}} = conn + end + + test "cast json query params with invalid json" do + {operation, spec} = json_query_spec_operation() + + filter_json = URI.encode("{\"size\": \"S\", \"color: \"blue\"}") + conn = :get |> Plug.Test.conn("/api/users?filter=#{filter_json}") |> Plug.Conn.put_req_header("content-type", "application/json") |> Plug.Conn.fetch_query_params() - # Referencing issue: https://github.com/open-api-spex/open_api_spex/issues/349 - # Ideally, this would be a successful cast, the JSON parameter would be decoded as - # part of the casting process. However, that behavior is not yet supported. - CastParameters.cast(conn, operation, components) + assert {:error, + [%OpenApiSpex.Cast.Error{format: "application/json", reason: :invalid_format}]} = + CastParameters.cast(conn, operation, spec) end end @@ -214,4 +250,48 @@ defmodule OpenApiSpex.CastParametersTest do } } end + + defp spec_with_components(components \\ nil) do + %OpenApiSpex.OpenApi{ + info: %OpenApiSpex.Info{title: "Spec", version: "1.0.0"}, + paths: [], + components: components || create_components() + } + end + + defp json_query_spec_operation(content_type \\ "application/json") do + schema = %Schema{ + type: :object, + title: "FilterParams", + properties: %{ + size: %Schema{type: :string, pattern: "^XS|S|M|L|XL$"}, + color: %Schema{type: :string} + } + } + + parameter = %Parameter{ + in: :query, + name: :filter, + required: false, + content: %{ + content_type => %MediaType{ + schema: %Reference{"$ref": "#/components/schemas/FilterParams"} + } + } + } + + operation = %Operation{ + parameters: [parameter], + responses: %{ + 200 => %Schema{type: :object} + } + } + + spec = + spec_with_components(%Components{ + schemas: %{"FilterParams" => schema} + }) + + {operation, spec} + end end diff --git a/test/operation2_test.exs b/test/operation2_test.exs index 73f266f0..685997ee 100644 --- a/test/operation2_test.exs +++ b/test/operation2_test.exs @@ -153,10 +153,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:ok, conn} = Operation2.cast( + SpecModule.spec(), OperationFixtures.create_user(), conn, - "application/json", - SpecModule.spec().components + "application/json" ) assert %Plug.Conn{} = conn @@ -168,10 +168,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:ok, conn} = Operation2.cast( + SpecModule.spec(), OperationFixtures.create_user(), conn, "application/json", - SpecModule.spec().components, replace_params: false ) @@ -185,10 +185,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:ok, conn} = Operation2.cast( + SpecModule.spec(), OperationFixtures.update_user(), conn, - "application/json", - SpecModule.spec().components + "application/json" ) assert %Plug.Conn{} = conn @@ -199,10 +199,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:ok, conn} = Operation2.cast( + SpecModule.spec(), OperationFixtures.create_users(), conn, - "application/json", - SpecModule.spec().components + "application/json" ) assert %Plug.Conn{} = conn @@ -213,10 +213,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:error, errors} = Operation2.cast( + SpecModule.spec(), OperationFixtures.create_user(), conn, - "application/json", - SpecModule.spec().components + "application/json" ) assert [error] = errors @@ -229,10 +229,10 @@ defmodule OpenApiSpex.Operation2Test do assert {:error, errors} = Operation2.cast( + SpecModule.spec(), OperationFixtures.update_user(), conn, - "application/json", - SpecModule.spec().components + "application/json" ) assert [error] = errors @@ -296,12 +296,7 @@ defmodule OpenApiSpex.Operation2Test do operation = OperationFixtures.create_user() assert {:error, [%Error{reason: :missing_header, name: "content-type"}]} = - Operation2.cast( - operation, - conn, - nil, - SpecModule.spec().components - ) + Operation2.cast(SpecModule.spec(), operation, conn, nil) end test "validate invalid content-type header for required requestBody" do @@ -312,12 +307,7 @@ defmodule OpenApiSpex.Operation2Test do operation = OperationFixtures.create_user() assert {:error, [%Error{reason: :invalid_header, name: "content-type"}]} = - Operation2.cast( - operation, - conn, - "text/html", - SpecModule.spec().components - ) + Operation2.cast(SpecModule.spec(), operation, conn, "text/html") end test "validate invalid content-type header for required requestBody reference" do @@ -328,12 +318,7 @@ defmodule OpenApiSpex.Operation2Test do operation = OperationFixtures.update_user() assert {:error, [%Error{reason: :invalid_header, name: "content-type"}]} = - Operation2.cast( - operation, - conn, - "text/html", - SpecModule.spec().components - ) + Operation2.cast(SpecModule.spec(), operation, conn, "text/html") end test "validate invalid value for integer range" do @@ -376,13 +361,7 @@ defmodule OpenApiSpex.Operation2Test do } ]} - assert expected_response == - Operation2.cast( - operation, - conn, - "text/html", - SpecModule.spec().components - ) + assert expected_response == Operation2.cast(SpecModule.spec(), operation, conn, "text/html") end defp do_index_cast(query_params, opts \\ []) do @@ -397,10 +376,10 @@ defmodule OpenApiSpex.Operation2Test do operation = opts[:operation] || OperationFixtures.user_index() Operation2.cast( + SpecModule.spec(), operation, conn, "application/json", - SpecModule.spec().components, Keyword.take(opts, [:replace_params]) ) end