-
Notifications
You must be signed in to change notification settings - Fork 350
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
Query parameters do not support nested maps #538
Comments
Ideally, we shouldn't depend on 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 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? |
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 Plug seemed like the logical implementation to me, because Tesla is very 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 Personally, I'd only add the |
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 |
Twilio's Ruby implementation (based on OpenAPI) has a |
After #558 is merged; I will allow a function to be passed as the query encoding; I will try to provide the default Tesla.get(
"http://example.com",
query: [username: "John Smith"],
opts: [query_encoding: &Plug.Conn.Query.encode\1]
) Something around those lines |
I wonder if shouldn't allow passing binary as query instead, like: Tesla.get("/", query: Plug.Conn.Query.encode([username: "John Smith"])) 🤔 |
@teamon I do not see why not, at that point we just concat strings; noted! |
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 likefilters[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?
The text was updated successfully, but these errors were encountered: