Adapter options aren't merged correctly

[23shortstop]

I need to use a proxy server for requests for some services. I created a reusable module for this:

defmodule Tesla.Proxy do
  defmacro __using__(_opts) do
    quote do
      @proxy_url Application.fetch_env!(:app, :proxy_url)
      @proxy_user Application.fetch_env!(:app, :proxy_user)
      @proxy_password Application.fetch_env!(:app, :proxy_password)

      plug Tesla.Middleware.Opts,
        adapter: [proxy: @proxy_url, proxy_auth: {@proxy_user, @proxy_password}]
    end
  end
end

Then I used this module in an API client module where I also needed to add another adapter options:

defmodule App.ApiClient do
  use Tesla
  use Tesla.Proxy

  plug Tesla.Middleware.Opts, adapter: [ssl_options: [{:versions, [:tlsv1]}]]

  ...
end

I believe that Tesla should merge all adapter options, but now it's not.
At first all options are appended to one list:

opts = [
  adapter: [ssl_options: [{:versions, [:tlsv1]}]],
  adapter: [proxy: ..., proxy_auth: {..., ...}]
]

Then adapter options retrieved using square brackets here:
https://github.com/teamon/tesla/blob/d5ce3a75d7ff285563fdfd47905e66ba4a0433a3/lib/tesla.ex#L215-L220
And we get only the first value in case there are few pairs with the same key in a Keyword.

To solve the issue I suggest to rewrite this function this way:

  @spec opts(Keyword.t(), Tesla.Env.t(), Keyword.t()) :: Keyword.t()
  def opts(defaults \\ [], env, opts) do
    defaults
    |> Keyword.merge(opts || [])
    |> Keyword.merge(env_opts(env))
  end

  defp env_opts(env) do
    env.opts
    |> Keyword.get_values(:adapter)
    |> List.flatten
  end

[teamon]

I would discourage usage of macros for DRY. It's better to use the Runtime middleware.

I think Tesla.Middleware.Opts should use Keyword.merge, instead of simple ++, but unfortunately it would be a breaking change.

On the other hand, currently it doesn't work, and it seems like there is a little chance someone is relying on this behaviour.

[sambhavsaxena]

I would like to work on this issue

[teamon]

Hey @sambhavsaxena, are you still up for this one?

[polvalente]

@teamon if there's no one tackling this, I'd be happy to give it a go. To be clear, the goal would be to merge the adapter options and pass them through properly, right?

[brenomaia]

Didn't mean to go over your head @polvalente , I stumbled across this a few weeks ago and decided to try to fix it but hadn't seen your comment.

[polvalente]

Didn't mean to go over your head @polvalente , I stumbled across this a few weeks a go and decided to try to fix it but hadn't seen your comment.

No harm done!