Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests and fix parameter exploding issue #576

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion examples/plug_app/lib/plug_app/user_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,31 @@ defmodule PlugApp.UserHandler do
description: "Show a user by ID",
operationId: "UserHandler.Show",
parameters: [
parameter(:id, :path, %Schema{type: :integer, minimum: 1}, "User ID", example: 123)
parameter(:id, :path, %Schema{type: :integer, minimum: 1}, "User ID", example: 123),
parameter(:qux, :query, %Schema{type: :string}, "qux param", required: false),
parameter(
:some,
:query,
%Schema{
type: :object,
oneOf: [
%Schema{
type: :object,
properties: %{foo: %Schema{type: :string}, bar: %Schema{type: :string}},
required: [:foo]
},
%Schema{
type: :object,
properties: %{foo: %Schema{type: :string}, baz: %Schema{type: :string}},
required: [:baz]
}
]
},
"Some query parameter ",
explode: true,
style: :form,
required: true
)
],
responses: %{
200 => response("User", "application/json", Schemas.UserResponse)
Expand All @@ -80,6 +104,8 @@ defmodule PlugApp.UserHandler do
end

def show(conn = %Plug.Conn{assigns: %{user: user}}, _opts) do
user = Accounts.get_user!(conn.params.id)

conn
|> put_resp_header("content-type", "application/json")
|> send_resp(200, render(user))
Expand Down
21 changes: 20 additions & 1 deletion examples/plug_app/test/user_handler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ defmodule UserHandlerTest do
} do
%{resp_body: body} =
conn =
conn(:get, "/api/users/#{user_id}")
conn(:get, "/api/users/#{user_id}?foo=asd")
|> Router.call(@opts)

assert %{status: 200} = conn
Expand All @@ -57,6 +57,25 @@ defmodule UserHandlerTest do

assert_schema(json_response, "UserResponse", api_spec)
end

test "responds with 422 when there is no either foo nor bar in query params", %{
user: %{id: user_id},
api_spec: _api_spec
} do
%{resp_body: body} =
conn =
conn(:get, "/api/users/#{user_id}")
|> Router.call(@opts)

assert %{status: 422} = conn

json_response = Jason.decode!(body)

IO.inspect(json_response)
# assert %{} = json_response

# assert_schema(json_response, "UserResponse", api_spec)
end
end

describe "POST /api/users" do
Expand Down
56 changes: 56 additions & 0 deletions lib/open_api_spex/cast_parameters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ defmodule OpenApiSpex.CastParameters do
}
end


# Extract context information from parameters, useful later when casting
defp parameters_contexts(parameters) do
Map.new(parameters, fn parameter ->
Expand Down Expand Up @@ -127,13 +128,68 @@ defmodule OpenApiSpex.CastParameters do
location,
schema.properties |> Map.keys() |> Enum.map(&Atom.to_string/1)
)
|> maybe_combine_params(schema, parameters_contexts)
|> pre_parse_parameters(parameters_contexts, parsers)
|> case do
{:error, _} = err -> err
params -> Cast.cast(schema, params, components.schemas, opts)
end
end

# in caase some parameters have explode: true we want to search for those
# fields in parameters and combine the parameters in a single struct
# so that the casting can do further work
defp maybe_combine_params(%{} = parameters, %{} = schema, %{} = parameters_contexts) do
# first filter out from parameters fields that match non exploding properties.
# we do this because we want to keep the original parameters struct intact
# and not remove fields that are not part of the exploding property

non_exploding_matches = Enum.reduce(parameters, Map.new(), fn {key, value}, acc ->
case Map.get(parameters_contexts, key, %{}) do
%{explode: false} ->
Map.put(acc, key, value)

_ ->
acc
end
end)

possible_exploding_matches = Enum.reject(parameters, &Enum.member?(non_exploding_matches, &1)) |> Map.new()

combined_params = Enum.reduce(parameters_contexts, possible_exploding_matches, fn
{key, %{explode: true}}, parameters ->
# we have exploding property, we need to search for it's possible fields
# and add them under the key into the parameters struct.
# do we leave the fields in the params as well? not sure.
schema_of_exploding_property = Map.get(schema.properties, String.to_existing_atom(key), %{})

fields =
Schema.properties(schema_of_exploding_property) ++
Schema.possible_properties(schema_of_exploding_property)

{struct_params, found_keys} =
Enum.reduce(fields, {Map.new(), []}, fn {field_key, _default}, {struct_params, found_keys} ->
param_field_key = field_key |> Atom.to_string()
val = Map.get(parameters, param_field_key)

unless is_nil(val) do
{Map.put(struct_params, param_field_key, val), [param_field_key | found_keys]}
else
{struct_params, found_keys}
end
end)

parameters
|> Map.drop(found_keys)
|> Map.put(key, struct_params)

_, parameters ->
parameters
end)

Map.merge(non_exploding_matches, combined_params)
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
Expand Down
17 changes: 17 additions & 0 deletions lib/open_api_spex/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,23 @@ defmodule OpenApiSpex.Schema do

def properties(_), do: []

@doc """
Get the names of all properties possible for polymorphic schemas using `oneOf`.

This is different from properties/1 in that it returns properties that *might*
be a part of the schema sometimes based on the discriminator.
"""

def possible_properties(%Schema{oneOf: schemas}) when is_list(schemas) do
Enum.flat_map(schemas, &properties/1) |> Enum.uniq()
end

def possible_properties(%Schema{anyOf: schemas}) when is_list(schemas) do
Enum.flat_map(schemas, &properties/1) |> Enum.uniq()
end

def possible_properties(_), do: []

@doc """
Generate example value from a `%Schema{}` struct.

Expand Down
64 changes: 64 additions & 0 deletions test/cast_parameters_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,20 @@ defmodule OpenApiSpex.CastParametersTest do
[%OpenApiSpex.Cast.Error{format: "application/json", reason: :invalid_format}]} =
CastParameters.cast(conn, operation, spec)
end

test "cast param with oneof and valid args" do
{operation, spec} = oneof_query_spec_operation()

filter_params = URI.encode_query(%{size: "XL"})

conn =
:get
|> Plug.Test.conn("/api/users?#{filter_params}")
|> Plug.Conn.put_req_header("content-type", "application/json")
|> Plug.Conn.fetch_query_params()

assert {:ok, _} = CastParameters.cast(conn, operation, spec)
end
end

defp create_conn() do
Expand Down Expand Up @@ -294,4 +308,54 @@ defmodule OpenApiSpex.CastParametersTest do

{operation, spec}
end

defp oneof_query_spec_operation() do
schema = %Schema{
type: :object,
title: "Filters",
oneOf: [
%Schema{
type: :object,
properties: %{
size: %Schema{type: :string, pattern: "^XS|S|M|L|XL$"},
color: %Schema{type: :string}
},
required: [:size]
},
%Schema{
type: :object,
properties: %{
size: %Schema{type: :string, pattern: "^XS|S|M|L|XL$"},
color: %Schema{type: :string}
},
required: [:color]
}
],
example: %{size: "XL"}
}

parameter = %Parameter{
in: :query,
name: :filter,
required: false,
schema: %Reference{"$ref": "#/components/schemas/Filters"},
explode: true,
style: :form,
required: true
}

operation = %Operation{
parameters: [parameter],
responses: %{
200 => %Schema{type: :object}
}
}

spec =
spec_with_components(%Components{
schemas: %{"Filters" => schema}
})

{operation, spec}
end
end