From cfb0d252322956d0c6106a0d9a988c363e853e61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20=C5=9Aled=C5=BA?= <michal.sledz@swmansion.com>
Date: Fri, 3 Nov 2023 13:55:28 +0100
Subject: [PATCH] Document and refactor PeerConnection options

---
 lib/ex_webrtc/peer_connection.ex              | 26 ++++-----
 .../peer_connection/configuration.ex          | 55 ++++++++++++++-----
 2 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/lib/ex_webrtc/peer_connection.ex b/lib/ex_webrtc/peer_connection.ex
index e12aeac4..139c2bf6 100644
--- a/lib/ex_webrtc/peer_connection.ex
+++ b/lib/ex_webrtc/peer_connection.ex
@@ -1,5 +1,7 @@
 defmodule ExWebRTC.PeerConnection do
-  @moduledoc false
+  @moduledoc """
+  PeerConnection
+  """
 
   use GenServer
 
@@ -47,12 +49,15 @@ defmodule ExWebRTC.PeerConnection do
               ]
 
   #### API ####
-
-  def start_link(configuration \\ []) do
+  @spec start_link(Configuration.options()) :: GenServer.on_start()
+  def start_link(options \\ []) do
+    configuration = Configuration.from_options!(options)
     GenServer.start_link(__MODULE__, {self(), configuration})
   end
 
-  def start(configuration \\ []) do
+  @spec start(Configuration.options()) :: GenServer.on_start()
+  def start(options \\ []) do
+    configuration = Configuration.from_options!(options)
     GenServer.start(__MODULE__, {self(), configuration})
   end
 
@@ -104,16 +109,7 @@ defmodule ExWebRTC.PeerConnection do
 
   @impl true
   def init({owner, config}) do
-    config = struct(Configuration, config)
-    :ok = Configuration.check_support(config)
-
-    # ATM, ExICE does not support relay via TURN
-    stun_servers =
-      config.ice_servers
-      |> Enum.flat_map(&if(is_list(&1.urls), do: &1.urls, else: [&1.urls]))
-      |> Enum.filter(&String.starts_with?(&1, "stun:"))
-
-    {:ok, ice_agent} = ICEAgent.start_link(:controlled, stun_servers: stun_servers)
+    {:ok, ice_agent} = ICEAgent.start_link(:controlled, stun_servers: config.stun_servers)
     {:ok, dtls_client} = ExDTLS.start_link(client_mode: false, dtls_srtp: true)
 
     state = %__MODULE__{
@@ -438,8 +434,6 @@ defmodule ExWebRTC.PeerConnection do
       state = set_description(:remote, type, sdp, state)
 
       {:ok, %{state | transceivers: new_transceivers}}
-    else
-      error -> error
     end
   end
 
diff --git a/lib/ex_webrtc/peer_connection/configuration.ex b/lib/ex_webrtc/peer_connection/configuration.ex
index c197389e..f68fd351 100644
--- a/lib/ex_webrtc/peer_connection/configuration.ex
+++ b/lib/ex_webrtc/peer_connection/configuration.ex
@@ -1,10 +1,9 @@
 defmodule ExWebRTC.PeerConnection.Configuration do
-  @moduledoc false
+  @moduledoc """
+  PeerConnection configuration
+  """
 
-  @type bundle_policy() ::
-          :balanced
-          | :max_compat
-          | :max_bundle
+  @type bundle_policy() :: :balanced | :max_compat | :max_bundle
 
   @type ice_server() :: %{
           optional(:credential) => String.t(),
@@ -15,15 +14,14 @@ defmodule ExWebRTC.PeerConnection.Configuration do
   # TODO implement
   @type certificate() :: :TODO
 
-  @type ice_transport_policy() ::
-          :all
-          | :relay
+  @type ice_transport_policy() :: :all | :relay
 
-  @type rtcp_mux_policy() ::
-          :negotiate
-          | :require
+  @type rtcp_mux_policy() :: :negotiate | :require
 
-  @type t() :: %__MODULE__{
+  @typedoc """
+  Options that can be passed to `ExWebRTC.PeerConnection.start_link/1`.
+  """
+  @type options() :: [
           bundle_policy: bundle_policy(),
           certificates: [certificate()],
           ice_candidate_pool_size: non_neg_integer(),
@@ -31,7 +29,17 @@ defmodule ExWebRTC.PeerConnection.Configuration do
           ice_transport_policy: ice_transport_policy(),
           peer_identity: String.t(),
           rtcp_mux_policy: rtcp_mux_policy()
-        }
+        ]
+
+  @typep t() :: %__MODULE__{
+           bundle_policy: bundle_policy(),
+           certificates: [certificate()],
+           ice_candidate_pool_size: non_neg_integer(),
+           ice_servers: [ice_server()],
+           ice_transport_policy: ice_transport_policy(),
+           peer_identity: String.t(),
+           rtcp_mux_policy: rtcp_mux_policy()
+         }
 
   defstruct bundle_policy: :max_bundle,
             certificates: nil,
@@ -41,8 +49,25 @@ defmodule ExWebRTC.PeerConnection.Configuration do
             peer_identity: nil,
             rtcp_mux_policy: :require
 
-  @spec check_support(t()) :: :ok
-  def check_support(config) do
+  @doc false
+  @spec from_options!(options()) :: t()
+  def from_options!(options) do
+    config = struct(__MODULE__, options)
+
+    :ok = validate!(config)
+
+    # ATM, ExICE does not support relay via TURN
+    stun_servers =
+      config.ice_servers
+      |> Enum.flat_map(&if(is_list(&1.urls), do: &1.urls, else: [&1.urls]))
+      |> Enum.filter(&String.starts_with?(&1, "stun:"))
+
+    %__MODULE__{config | ice_servers: stun_servers}
+  end
+
+  @doc false
+  @spec validate!(t()) :: :ok
+  def validate!(config) do
     if config.ice_transport_policy != :all do
       raise "#{inspect(config.ice_transport_policy)} ice transport policy is currently not supported"
     end