Params by plug convention should contain merged path, body and query parameters

[hamir-suspect]

According to Plug documentation conn.params is

the request params, the result of merging the :path_params on top of :body_params on top of :query_params

When using OpenApiSpex.Plug.CastAndValidate plug the params will overwrite only with params from :path | :query | :header | :cookie meaning that there is never parameters from request_body there.

This should be clearly stated in CastAndValidate plug like it is for cast plug as it can cause some confusion.

[mbuhot]

The library was initially implemented with all params merged into conn.params as per the docs. Unfortunately it is problematic with struct based body schemas.

If your body schema is a struct, the adding additional query params creates a struct with undeclared fields set. It technically works since structs are just maps, but it is confusing and may cause type errors in future elixir versions.

[PJUllrich]

If I may, I'd like to add a +1 here that this is confusing and I'd argue to revert it. I don't care whether the request_body is a struct or a map, but I do care that OpenApiSpex doesn't break the Phoenix convention of merging all parameters into params. Now, I need to add a custom plug to merge them all back together again because I don't want to rely on conn.body_params but continue the Phoenix custom of having all params in the second argument of a def action(conn, params) call.

[PJUllrich]

FYI for whomever is interested, this plug merges the params again. Just add it behind the CastAndValidate plug:

defmodule JadeWeb.Plugs.MergeParamsFromOpenApiSpex do
  @moduledoc """
  Merges the `body_params` into the `params` field if the request was validated by OpenApiSpex.

  For context, see: https://github.com/open-api-spex/open_api_spex/issues/588
  """
  @behaviour Plug

  @impl Plug
  def init(opts), do: opts

  @impl Plug
  def call(
        %Plug.Conn{
          params: params,
          body_params: body_params,
          private: private
        } = conn,
        _opts
      )
      when is_map_key(private, :open_api_spex) do
    body_params = if is_struct(body_params), do: Map.from_struct(body_params), else: body_params
    params = Map.merge(params, body_params)
    Map.put(conn, :params, params)
  end

  def call(conn, _opts), do: conn
end

[mbuhot]

You can also use replace_params: false to preserve the original behaviour:

open_api_spex/lib/open_api_spex/plug/cast_and_validate.ex

Lines 21 to 23 in abe90e3

	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.

[PJUllrich]

Ah interesting! I'd still like to use the casted parameters though, just not split between params and body_params. So, I'm okay with my plug mentioned above.

[zorbash]

We've also faced this problem and in our controllers replace_params defaults to false. I think it would be convenient this behaviour configurable through a OpenApiSpex.Plug.CastAndValidate option.