fix: avoid overriding user config if possible (#236)

* fix: avoid overriding user config if possible

* feat: make transport option replacement work

* fix: || %{}

* fix: override correctly

* docs: improve credential docs

lib/grpc/client/adapters/gun.ex

@@ -1,62 +1,56 @@
 defmodule GRPC.Client.Adapters.Gun do
   @moduledoc """
-  A client (`b:GRPC.Client.Adapter`) adapter using `:gun`.
+  A client adapter using Gun
 
   `conn_pid` and `stream_ref` are stored in `GRPC.Server.Stream`.
   """
 
   @behaviour GRPC.Client.Adapter
 
   @default_transport_opts [nodelay: true]
-  @default_http2_opts %{settings_timeout: :infinity}
   @max_retries 100
 
   @impl true
-  def connect(channel, nil), do: connect(channel, %{})
+  def connect(channel, opts \\ %{})
   def connect(%{scheme: "https"} = channel, opts), do: connect_securely(channel, opts)
   def connect(channel, opts), do: connect_insecurely(channel, opts)
 
   defp connect_securely(%{cred: %{ssl: ssl}} = channel, opts) do
-    transport_opts = Map.get(opts, :transport_opts, @default_transport_opts ++ ssl)
-    open_opts = %{transport: :ssl, protocols: [:http2]}
+    transport_opts = Map.get(opts, :transport_opts) || []
 
-    open_opts =
-      if gun_v2?() do
-        Map.put(open_opts, :tls_opts, transport_opts)
-      else
-        Map.put(open_opts, :transport_opts, transport_opts)
-      end
+    tls_opts = Keyword.merge(@default_transport_opts ++ ssl, transport_opts)
 
-    open_opts = Map.merge(opts, open_opts)
+    open_opts =
+      opts
+      |> Map.delete(:transport_opts)
+      |> Map.merge(%{transport: :ssl, protocols: [:http2], tls_opts: tls_opts})
 
     do_connect(channel, open_opts)
   end
 
   defp connect_insecurely(channel, opts) do
-    opts = Map.update(opts, :http2_opts, @default_http2_opts, &Map.merge(&1, @default_http2_opts))
+    opts =
+      Map.update(
+        opts,
+        :http2_opts,
+        %{settings_timeout: :infinity},
+        &Map.put(&1, :settings_timeout, :infinity)
+      )
 
-    transport_opts = Map.get(opts, :transport_opts, @default_transport_opts)
-    open_opts = %{transport: :tcp, protocols: [:http2]}
+    transport_opts = Map.get(opts, :transport_opts) || []
 
-    open_opts =
-      if gun_v2?() do
-        Map.put(open_opts, :tcp_opts, transport_opts)
-      else
-        Map.put(open_opts, :transport_opts, transport_opts)
-      end
+    tcp_opts = Keyword.merge(@default_transport_opts, transport_opts)
 
-    open_opts = Map.merge(opts, open_opts)
+    open_opts =
+      opts
+      |> Map.delete(:transport_opts)
+      |> Map.merge(%{transport: :tcp, protocols: [:http2], tcp_opts: tcp_opts})
 
     do_connect(channel, open_opts)
   end
 
   defp do_connect(%{host: host, port: port} = channel, open_opts) do
-    open_opts =
-      if gun_v2?() do
-        Map.merge(%{retry: @max_retries, retry_fun: &__MODULE__.retry_fun/2}, open_opts)
-      else
-        open_opts
-      end
+    open_opts = Map.merge(%{retry: @max_retries, retry_fun: &__MODULE__.retry_fun/2}, open_opts)
 
     {:ok, conn_pid} = open(host, port, open_opts)
 
@@ -267,17 +261,6 @@ defmodule GRPC.Client.Adapters.Gun do
     end
   end
 
-  @char_2 List.first('2')
-  def gun_v2?() do
-    case :application.get_key(:gun, :vsn) do
-      {:ok, [@char_2 | _]} ->
-        true
-
-      _ ->
-        false
-    end
-  end
-
   def retry_fun(retries, _opts) do
     curr = @max_retries - retries + 1
 

lib/grpc/credential.ex

@@ -1,8 +1,15 @@
 defmodule GRPC.Credential do
   @moduledoc """
-  Stores credentials for authentication. It can be used to establish secure connections
+  Stores credentials for authentication.
+
+  It can be used to establish secure connections
   by passed to `GRPC.Stub.connect/2` as an argument.
 
+  Some client and server adapter implementations may
+  choose to let request options override some of the
+  configuration here, but this is left as a choice
+  for each adapter.
+
   ## Examples
 
       iex> cred = GRPC.Credential.new(ssl: [cacertfile: ca_path])

lib/grpc/stub.ex

@@ -174,7 +174,7 @@ defmodule GRPC.Stub do
       accepted_compressors: accepted_compressors,
       headers: headers
     }
-    |> adapter.connect(opts[:adapter_opts])
+    |> adapter.connect(opts[:adapter_opts] || %{})
   end
 
   def retry_timeout(curr) when curr < 11 do

mix.exs

@@ -45,8 +45,8 @@ defmodule GRPC.Mixfile do
       {:gun, "~> 2.0.1", hex: :grpc_gun},
       {:cowlib, "~> 2.11"},
       {:protobuf, "~> 0.10", only: [:dev, :test]},
-      {:ex_doc, "~> 0.28", only: :dev},
-      {:dialyxir, "~> 1.1", only: [:dev, :test], runtime: false}
+      {:ex_doc, "~> 0.28.0", only: :dev},
+      {:dialyxir, "~> 1.1.0", only: [:dev, :test], runtime: false}
     ]
   end
 

test/grpc/adapter/gun_test.exs

@@ -0,0 +1,87 @@
+defmodule GRPC.Client.Adapters.GunTest do
+  use ExUnit.Case, async: true
+
+  import GRPC.Factory
+
+  alias GRPC.Client.Adapters.Gun
+
+  describe "connect/2" do
+    setup do
+      server_credential = build(:credential)
+      {:ok, _, port} = GRPC.Server.start(FeatureServer, 0, cred: server_credential)
+
+      on_exit(fn ->
+        :ok = GRPC.Server.stop(FeatureServer)
+      end)
+
+      %{
+        port: port,
+        credential:
+          build(:credential,
+            ssl: Keyword.take(server_credential.ssl, [:certfile, :keyfile, :versions])
+          )
+      }
+    end
+
+    test "connects insecurely (default options)", %{port: port, credential: credential} do
+      channel = build(:channel, port: port, host: "localhost", cred: credential)
+
+      assert {:ok, result} = Gun.connect(channel)
+
+      assert %{channel | adapter_payload: %{conn_pid: result.adapter_payload.conn_pid}} == result
+    end
+
+    test "connects insecurely (custom options)", %{port: port, credential: credential} do
+      channel = build(:channel, port: port, host: "localhost", cred: credential)
+
+      # Ensure that it works
+      assert {:ok, result} = Gun.connect(channel, %{transport_opts: [ip: :loopback]})
+      assert %{channel | adapter_payload: %{conn_pid: result.adapter_payload.conn_pid}} == result
+
+      # Ensure that changing one of the options breaks things
+      assert {:error, {:down, :badarg}} ==
+               Gun.connect(channel, %{transport_opts: [ip: "256.0.0.0"]})
+    end
+
+    test "connects securely (default options)", %{port: port, credential: credential} do
+      channel =
+        build(:channel,
+          port: port,
+          scheme: "https",
+          host: "localhost",
+          cred: credential
+        )
+
+      assert {:ok, result} = Gun.connect(channel, %{tls_opts: channel.cred.ssl})
+
+      assert %{channel | adapter_payload: %{conn_pid: result.adapter_payload.conn_pid}} == result
+    end
+
+    test "connects securely (custom options)", %{port: port, credential: credential} do
+      channel =
+        build(:channel,
+          port: port,
+          scheme: "https",
+          host: "localhost",
+          cred: credential
+        )
+
+      # Ensure that it works
+      assert {:ok, result} =
+               Gun.connect(channel, %{
+                 transport_opts: [certfile: credential.ssl[:certfile], ip: :loopback]
+               })
+
+      assert %{channel | adapter_payload: %{conn_pid: result.adapter_payload.conn_pid}} == result
+
+      # Ensure that changing one of the options breaks things
+      assert {:error, :timeout} ==
+               Gun.connect(channel, %{
+                 transport_opts: [
+                   certfile: credential.ssl[:certfile] <> "invalidsuffix",
+                   ip: :loopback
+                 ]
+               })
+    end
+  end
+end

test/support/factory.ex

@@ -0,0 +1,52 @@
+defmodule GRPC.Factory do
+  @moduledoc false
+
+  alias GRPC.Channel
+  alias GRPC.Credential
+
+  @cert_path Path.expand("./tls/server1.pem", :code.priv_dir(:grpc))
+  @key_path Path.expand("./tls/server1.key", :code.priv_dir(:grpc))
+  @ca_path Path.expand("./tls/ca.pem", :code.priv_dir(:grpc))
+
+  def build(resource, attrs \\ %{}) do
+    name = :"#{resource}_factory"
+
+    data =
+      if function_exported?(__MODULE__, name, 1) do
+        apply(__MODULE__, name, [attrs])
+      else
+        apply(__MODULE__, name, [])
+      end
+
+    Map.merge(data, Map.new(attrs))
+  end
+
+  def channel_factory do
+    %Channel{
+      host: "localhost",
+      port: 1337,
+      scheme: "http",
+      cred: build(:credential),
+      adapter: GRPC.Client.Adapters.Gun,
+      adapter_payload: %{},
+      codec: GRPC.Codec.Proto,
+      interceptors: [],
+      compressor: nil,
+      accepted_compressors: [],
+      headers: []
+    }
+  end
+
+  def credential_factory do
+    %Credential{
+      ssl: [
+        certfile: @cert_path,
+        cacertfile: @ca_path,
+        keyfile: @key_path,
+        verify: :verify_peer,
+        fail_if_no_peer_cert: true,
+        versions: [:"tlsv1.2"]
+      ]
+    }
+  end
+end