fix: avoid overriding user config if possible

lib/grpc/client/adapters/gun.ex

@@ -1,57 +1,51 @@
 defmodule GRPC.Client.Adapters.Gun do
-  @moduledoc false
+  @moduledoc """
+  A client adapter using Gun
+  """
 
   @behaviour GRPC.Client.Adapter
 
-  # A client adapter using Gun.
-  # conn_pid and stream_ref is stored in `GRPC.Server.Stream`.
+  # conn_pid and stream_ref are stored in `GRPC.Server.Stream`
 
   @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 =
-      Map.update(open_opts, :tls_opts, transport_opts, fn current ->
-        Keyword.merge(current, 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))
 
-    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 =
-      Map.update(open_opts, :tls_opts, transport_opts, fn current ->
-        Keyword.merge(current, 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)
 
@@ -262,17 +256,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
 

mix.exs

@@ -40,9 +40,9 @@ defmodule GRPC.Mixfile do
       {:gun, "~> 2.0.0-rc.2"},
       {:cowlib, "~> 2.11"},
       {:protobuf, "~> 0.10", only: [:dev, :test]},
-      {:ex_doc, "~> 0.28", only: :dev},
-      {:inch_ex, "~> 2.0", only: [:dev, :test]},
-      {:dialyxir, "~> 1.1", only: [:dev, :test], runtime: false}
+      {:ex_doc, "~> 0.28.0", only: :dev},
+      {:inch_ex, "~> 2.0.0", only: [:dev, :test]},
+      {: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/grpc/integration/connection_test.exs

@@ -5,14 +5,6 @@ defmodule GRPC.Integration.ConnectionTest do
   @key_path Path.expand("./tls/server1.key", :code.priv_dir(:grpc))
   @ca_path Path.expand("./tls/ca.pem", :code.priv_dir(:grpc))
 
-  defmodule FeatureServer do
-    use GRPC.Server, service: Routeguide.RouteGuide.Service
-
-    def get_feature(point, _stream) do
-      Routeguide.Feature.new(location: point, name: "#{point.latitude},#{point.longitude}")
-    end
-  end
-
   test "reconnection works" do
     server = FeatureServer
     {:ok, _, port} = GRPC.Server.start(server, 0)

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

test/support/feature_server.ex

@@ -0,0 +1,7 @@
+defmodule FeatureServer do
+  use GRPC.Server, service: Routeguide.RouteGuide.Service
+
+  def get_feature(point, _stream) do
+    Routeguide.Feature.new(location: point, name: "#{point.latitude},#{point.longitude}")
+  end
+end