tesla icon indicating copy to clipboard operation
tesla copied to clipboard

Query parameters do not support nested maps

Open mfeckie opened this issue 2 years ago • 4 comments

When using Plug, we are able to serialize nested maps to query params.

e.g. Plug.Conn.Query.encode(%{filters: %{ name: "dave", direction: "asc", pagination: %{ page: 1} } }) which would give us something like filters[direction]=asc&filters[name]=dave&filters[pagination][page]=1"

Would you be open to a PR which either used Plug's implementation directly or re-implemented the algorithm?

mfeckie avatar Jul 13 '22 07:07 mfeckie

Ideally, we shouldn't depend on Plug since Telsa is about something else, so in the best scenario, Plug and Tesla would use the same package.

If I am not mistaken, these types of serialization became popular with OpenAPI and they have some names for it https://swagger.io/docs/specification/serialization/ maybe it is worth having some packages shared, but I am not sure if Plug peeps would take the suggestion.

Worth mentioning, that Tesla doesn't know about the serialization strategy of such query params and most likely are pluggable based on the specification of their APIs, so whatever we do, it needs to be taken into consideration.

Thoughts?

yordis avatar Jul 13 '22 07:07 yordis

Plug doesn't have a dependency, it has it's own implementation https://github.com/elixir-plug/plug/blob/v1.13.6/lib/plug/conn/query.ex#L203-L279

I'm not sure what the deal is with OpenAPI. Looking at that reference list, the only strategy I've very seen is deepObject and (infrequently) form.

Plug seemed like the logical implementation to me, because Tesla is very Plug like.

Seems like the issue hasn't come up before when I looked through the issues list, so maybe it's just something a small number of users would care about.

We were able to work around it by directly encoding using Plug.Conn.Query.encode and appending to the URL used by Tesla.

Personally, I'd only add the deepObject variant because it would be compatible with what already exists, just extending support for some extra data structures.

mfeckie avatar Jul 13 '22 07:07 mfeckie

To collect more implementations, I recently had to do the following one

defp encode_query(query) do
    query
    |> Enum.map(fn
      {k, v} when is_binary(v) -> "#{k}=#{v}"
      {k, v} when is_list(v) -> Enum.map(v, &"#{k}=#{&1}")
    end)
    |> List.flatten()
    |> Enum.join("&")
  end

Haven't done objects because we don't have a need for it

yordis avatar Sep 09 '22 18:09 yordis

Twilio's Ruby implementation (based on OpenAPI) has a flatten function which turns {a: {b: {c: 3}}} to {"a.b.c" => 3} to that using your encode_query approach, we would see a.b.c=3 or a.b.c=3&a.b.c=4 for a list.

halostatue avatar Oct 08 '23 16:10 halostatue