Skip to content

Commit

Permalink
Avoids overwriting Plug.Conn body_params and params with the cast out…
Browse files Browse the repository at this point in the history
…come (#425)
  • Loading branch information
albertored authored May 15, 2022
1 parent 50aad2a commit 6503474
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 26 deletions.
28 changes: 26 additions & 2 deletions lib/open_api_spex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,24 @@ defmodule OpenApiSpex do
OpenApiSpex.Cast.cast(schema, value, spec.components.schemas)
end

@type cast_opt :: {:replace_params, boolean()}

@spec cast_and_validate(
OpenApi.t(),
Operation.t(),
Plug.Conn.t(),
content_type :: nil | String.t(),
opts :: [cast_opt()]
) :: {:error, [Error.t()]} | {:ok, Plug.Conn.t()}
def cast_and_validate(
spec = %OpenApi{},
operation = %Operation{},
conn = %Plug.Conn{},
content_type \\ nil
content_type \\ nil,
opts \\ []
) do
content_type = content_type || content_type_from_header(conn)
Operation2.cast(operation, conn, content_type, spec.components)
Operation2.cast(operation, conn, content_type, spec.components, opts)
end

defp content_type_from_header(conn = %Plug.Conn{}) do
Expand Down Expand Up @@ -392,4 +402,18 @@ defmodule OpenApiSpex do
@spec resolve_schema(Schema.t() | Reference.t(), Components.schemas_map()) :: Schema.t()
def resolve_schema(%Schema{} = schema, _), do: schema
def resolve_schema(%Reference{} = ref, schemas), do: Reference.resolve_schema(ref, schemas)

@doc """
Get casted body params from a `Plug.Conn`.
If the conn has not been yet casted `nil` is returned.
"""
@spec body_params(Plug.Conn.t()) :: nil | map()
def body_params(%Plug.Conn{} = conn), do: get_in(conn.private, [:open_api_spex, :body_params])

@doc """
Get casted params from a `Plug.Conn`.
If the conn has not been yet casted `nil` is returned.
"""
@spec params(Plug.Conn.t()) :: nil | map()
def params(%Plug.Conn{} = conn), do: get_in(conn.private, [:open_api_spex, :params])
end
21 changes: 18 additions & 3 deletions lib/open_api_spex/cast_parameters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,29 @@ defmodule OpenApiSpex.CastParameters do
alias OpenApiSpex.Cast.Error
alias Plug.Conn

@spec cast(Plug.Conn.t(), Operation.t(), Components.t()) ::
@spec cast(Plug.Conn.t(), Operation.t(), Components.t(), opts :: [OpenApiSpex.cast_opt()]) ::
{:error, [Error.t()]} | {:ok, Conn.t()}
def cast(conn, operation, components) do
def cast(conn, operation, components, opts \\ []) do
replace_params = Keyword.get(opts, :replace_params, true)

with {:ok, params} <- cast_to_params(conn, operation, components) do
{:ok, %{conn | params: params}}
{:ok, conn |> cast_conn(params) |> maybe_replace_params(params, replace_params)}
end
end

defp cast_conn(conn, params) do
private_data =
conn
|> Map.get(:private)
|> Map.get(:open_api_spex, %{})
|> Map.put(:params, params)

Plug.Conn.put_private(conn, :open_api_spex, private_data)
end

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
operation
|> schemas_by_location(components)
Expand Down
39 changes: 33 additions & 6 deletions lib/open_api_spex/operation2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,47 @@ defmodule OpenApiSpex.Operation2 do
alias OpenApiSpex.Cast.Error
alias Plug.Conn

@spec cast(Operation.t(), Conn.t(), String.t() | nil, Components.t()) ::
@spec cast(
Operation.t(),
Conn.t(),
String.t() | nil,
Components.t(),
opts :: [OpenApiSpex.cast_opt()]
) ::
{:error, [Error.t()]} | {:ok, Conn.t()}
def cast(operation = %Operation{}, conn = %Conn{}, content_type, components = %Components{}) do
with {:ok, conn} <- cast_parameters(conn, operation, components),
def cast(
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),
{:ok, body} <-
cast_request_body(operation.requestBody, conn.body_params, content_type, components) do
{:ok, %{conn | body_params: body}}
{:ok, conn |> cast_conn(body) |> maybe_replace_body(body, replace_params)}
end
end

## Private functions

defp cast_parameters(conn, operation, components) do
CastParameters.cast(conn, operation, components)
defp cast_conn(conn, body) do
private_data =
conn
|> Map.get(:private)
|> Map.get(:open_api_spex, %{})
|> Map.put(:body_params, body)

Plug.Conn.put_private(conn, :open_api_spex, private_data)
end

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)
end

defp cast_request_body(ref = %Reference{}, body_params, content_type, components) do
Expand Down
24 changes: 19 additions & 5 deletions lib/open_api_spex/plug/cast_and_validate.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ defmodule OpenApiSpex.Plug.CastAndValidate do
...
end
Casted params and body params are always stored in `conn.private`.
The option `:replace_params` can be set to false to avoid overwriting conn `:body_params` and `:params`
with their casted version.
plug OpenApiSpex.Plug.CastAndValidate,
json_render_error_v2: true,
operation_id: "MyApp.ShowUser",
replace_params: false
If you want customize the error response, you can provide the `:render_error` option to register a plug which creates
a custom response in the case of a validation error.
Expand Down Expand Up @@ -57,14 +66,19 @@ defmodule OpenApiSpex.Plug.CastAndValidate do
end

@impl Plug
def call(conn = %{private: %{open_api_spex: _}}, %{
operation_id: operation_id,
render_error: render_error
}) do
def call(
conn = %{private: %{open_api_spex: _}},
%{
operation_id: operation_id,
render_error: render_error
} = opts
) do
{spec, operation_lookup} = PutApiSpec.get_spec_and_operation_lookup(conn)
operation = operation_lookup[operation_id]

with {:ok, conn} <- OpenApiSpex.cast_and_validate(spec, operation, conn) do
cast_opts = opts |> Map.take([:replace_params]) |> Map.to_list()

with {:ok, conn} <- OpenApiSpex.cast_and_validate(spec, operation, conn, nil, cast_opts) do
conn
else
{:error, errors} ->
Expand Down
35 changes: 34 additions & 1 deletion test/operation2_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,24 @@ defmodule OpenApiSpex.Operation2Test do
assert %Plug.Conn{} = conn
end

test "cast request body with replace_params: false" do
body = %{"user" => %{"email" => "foo@bar.com"}}
conn = create_conn(body)

assert {:ok, conn} =
Operation2.cast(
OperationFixtures.create_user(),
conn,
"application/json",
SpecModule.spec().components,
replace_params: false
)

assert Map.has_key?(conn.private.open_api_spex, :body_params)
assert Map.has_key?(conn.private.open_api_spex.body_params, :user)
assert conn.body_params == body
end

test "cast request body reference" do
conn = put_conn(%{"user" => %{"email" => "foo@bar.com"}})

Expand Down Expand Up @@ -240,6 +258,19 @@ defmodule OpenApiSpex.Operation2Test do
assert conn.params == %{age: 31, member: true, name: "Rubi", include_archived: true}
end

test "cast valid query params with replace_params: false" do
valid_query_params = %{"name" => "Rubi", "age" => "31", "member" => "true"}
assert {:ok, conn} = do_index_cast(valid_query_params, replace_params: false)
assert Plug.Conn.fetch_query_params(conn).params == valid_query_params

assert conn.private.open_api_spex.params == %{
age: 31,
member: true,
name: "Rubi",
include_archived: false
}
end

test "validate invalid data type for query param" do
query_params = %{"age" => "asdf"}
assert {:error, [error]} = do_index_cast(query_params)
Expand Down Expand Up @@ -360,6 +391,7 @@ defmodule OpenApiSpex.Operation2Test do
|> Plug.Test.conn("/api/users?" <> URI.encode_query(query_params))
|> Plug.Conn.put_req_header("content-type", "application/json")
|> Plug.Conn.fetch_query_params()
|> Map.put(:body_params, %{})
|> build_params()

operation = opts[:operation] || OperationFixtures.user_index()
Expand All @@ -368,7 +400,8 @@ defmodule OpenApiSpex.Operation2Test do
operation,
conn,
"application/json",
SpecModule.spec().components
SpecModule.spec().components,
Keyword.take(opts, [:replace_params])
)
end

Expand Down
88 changes: 79 additions & 9 deletions test/plug/cast_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ defmodule OpenApiSpex.Plug.CastTest do
|> OpenApiSpexTest.Router.call([])

assert conn.status == 200
assert conn.private.open_api_spex.params == conn.params

assert OpenApiSpex.params(conn) == %{validParam: true}
end

test "Valid Param with replace_params false" do
conn =
:get
|> Plug.Test.conn("/api/users_no_replace?validParam=true")
|> OpenApiSpexTest.Router.call([])

assert conn.status == 200
assert conn.private.open_api_spex.params == %{validParam: true}
assert conn.params == %{"validParam" => "true"}

assert OpenApiSpex.params(conn) == %{validParam: true}
end

@tag :capture_log
Expand Down Expand Up @@ -79,6 +95,7 @@ defmodule OpenApiSpex.Plug.CastTest do
|> OpenApiSpexTest.Router.call([])

assert conn.status == 200
assert conn.private.open_api_spex.params == conn.params
end
end

Expand All @@ -90,6 +107,7 @@ defmodule OpenApiSpex.Plug.CastTest do
|> OpenApiSpexTest.Router.call([])

assert conn.status == 200
assert conn.private.open_api_spex.params == conn.params
end

@tag :capture_log
Expand Down Expand Up @@ -126,21 +144,25 @@ defmodule OpenApiSpex.Plug.CastTest do
}
}

casted_body = %OpenApiSpexTest.Schemas.UserRequest{
user: %OpenApiSpexTest.Schemas.User{
id: 123,
name: "asdf",
email: "foo@bar.com",
password: "0123456789",
updated_at: ~N[2017-09-12T14:44:55Z] |> DateTime.from_naive!("Etc/UTC")
}
}

conn =
:post
|> Plug.Test.conn("/api/users", Jason.encode!(request_body))
|> Plug.Conn.put_req_header("content-type", "application/json; charset=UTF-8")
|> OpenApiSpexTest.Router.call([])

assert conn.body_params == %OpenApiSpexTest.Schemas.UserRequest{
user: %OpenApiSpexTest.Schemas.User{
id: 123,
name: "asdf",
email: "foo@bar.com",
password: "0123456789",
updated_at: ~N[2017-09-12T14:44:55Z] |> DateTime.from_naive!("Etc/UTC")
}
}
assert conn.body_params == casted_body

assert conn.private.open_api_spex.body_params == conn.body_params

assert Jason.decode!(conn.resp_body) == %{
"data" => %{
Expand All @@ -151,6 +173,8 @@ defmodule OpenApiSpex.Plug.CastTest do
"updated_at" => "2017-09-12T14:44:55Z"
}
}

assert OpenApiSpex.body_params(conn) == casted_body
end

@tag :capture_log
Expand Down Expand Up @@ -182,6 +206,50 @@ defmodule OpenApiSpex.Plug.CastTest do
"title" => "Invalid value"
}
end

test "Valid Request with replace_params false" do
request_body = %{
"user" => %{
"id" => 123,
"name" => "asdf",
"email" => "foo@bar.com",
"password" => "0123456789",
"updated_at" => "2017-09-12T14:44:55Z"
}
}

casted_body = %OpenApiSpexTest.Schemas.UserRequest{
user: %OpenApiSpexTest.Schemas.User{
id: 123,
name: "asdf",
email: "foo@bar.com",
password: "0123456789",
updated_at: ~N[2017-09-12T14:44:55Z] |> DateTime.from_naive!("Etc/UTC")
}
}

conn =
:post
|> Plug.Test.conn("/api/users_no_replace", Jason.encode!(request_body))
|> Plug.Conn.put_req_header("content-type", "application/json; charset=UTF-8")
|> OpenApiSpexTest.Router.call([])

assert conn.private.open_api_spex.body_params == casted_body

assert conn.body_params == request_body

assert Jason.decode!(conn.resp_body) == %{
"data" => %{
"email" => "foo@bar.com",
"id" => 1234,
"inserted_at" => nil,
"name" => "asdf",
"updated_at" => "2017-09-12T14:44:55Z"
}
}

assert OpenApiSpex.body_params(conn) == casted_body
end
end

describe "oneOf body params" do
Expand All @@ -206,6 +274,8 @@ defmodule OpenApiSpex.Plug.CastTest do
}
}

assert conn.private.open_api_spex.body_params == conn.body_params

assert Jason.decode!(conn.resp_body) == %{
"data" => %{
"pet_type" => "Dog",
Expand Down
2 changes: 2 additions & 0 deletions test/support/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ defmodule OpenApiSpexTest.Router do
pipe_through :api
resources "/users", OpenApiSpexTest.UserController, only: [:create, :index, :show]

resources "/users_no_replace", OpenApiSpexTest.UserNoRepalceController, only: [:create, :index]

# Used by ParamsTest
resources "/custom_error_users", OpenApiSpexTest.CustomErrorUserController, only: [:index]

Expand Down
Loading

0 comments on commit 6503474

Please sign in to comment.