Move config to struct, fix HlsSkip (#130)

* Move config to struct, fix HlsSkip

* Update description

* Validate config in controller

* Update descriptions

lib/jellyfish/room.ex

@@ -12,6 +12,7 @@ defmodule Jellyfish.Room do
   alias Jellyfish.Component.{HLS, RTSP}
   alias Jellyfish.Event
   alias Jellyfish.Peer
+  alias Jellyfish.Room.Config
 
   alias Membrane.ICE.TURNManager
   alias Membrane.RTC.Engine
@@ -34,8 +35,6 @@ defmodule Jellyfish.Room do
   defstruct @enforce_keys ++ [components: %{}, peers: %{}]
 
   @type id :: String.t()
-  @type max_peers :: non_neg_integer() | nil
-  @type video_codec :: :h264 | :vp8 | nil
 
   @typedoc """
   This module contains:
@@ -47,11 +46,7 @@ defmodule Jellyfish.Room do
   """
   @type t :: %__MODULE__{
           id: id(),
-          config: %{
-            max_peers: max_peers(),
-            video_codec: video_codec(),
-            simulcast?: boolean()
-          },
+          config: Config.t(),
           components: %{Component.id() => Component.t()},
           peers: %{Peer.id() => Peer.t()},
           engine_pid: pid(),
@@ -63,13 +58,13 @@ defmodule Jellyfish.Room do
 
   def registry_id(room_id), do: {:via, Registry, {Jellyfish.RoomRegistry, room_id}}
 
-  @spec start(max_peers(), video_codec()) :: {:ok, pid(), id()}
-  def start(max_peers, video_codec) do
+  @spec start(Config.t()) :: {pid(), id()}
+  def start(config) do
     id = UUID.uuid4()
 
-    {:ok, pid} = GenServer.start(__MODULE__, [id, max_peers, video_codec], name: registry_id(id))
+    {:ok, pid} = GenServer.start(__MODULE__, [id, config], name: registry_id(id))
 
-    {:ok, pid, id}
+    {pid, id}
   end
 
   @spec get_state(id()) :: t() | nil
@@ -141,8 +136,8 @@ defmodule Jellyfish.Room do
   end
 
   @impl true
-  def init([id, max_peers, video_codec]) do
-    state = new(id, max_peers, video_codec)
+  def init([id, config]) do
+    state = new(id, config)
     Logger.metadata(room_id: id)
     Logger.info("Initialize room")
 
@@ -460,7 +455,7 @@ defmodule Jellyfish.Room do
     :ok
   end
 
-  defp new(id, max_peers, video_codec) do
+  defp new(id, config) do
     rtc_engine_options = [
       id: id
     ]
@@ -492,7 +487,7 @@ defmodule Jellyfish.Room do
 
     %__MODULE__{
       id: id,
-      config: %{max_peers: max_peers, video_codec: video_codec},
+      config: config,
       engine_pid: pid,
       network_options: [turn_options: turn_options]
     }

lib/jellyfish/room/config.ex

@@ -0,0 +1,61 @@
+defmodule Jellyfish.Room.Config do
+  @moduledoc """
+  Room configuration
+  """
+  @enforce_keys [:max_peers, :video_codec, :webhook_url]
+
+  defstruct @enforce_keys
+
+  @type max_peers :: non_neg_integer() | nil
+  @type video_codec :: :h264 | :vp8 | nil
+  @type webhook_url :: String.t()
+
+  @type t :: %__MODULE__{
+          max_peers: max_peers(),
+          video_codec: video_codec(),
+          webhook_url: URI.t()
+        }
+
+  @spec from_params(map()) :: {:ok, __MODULE__.t()} | {:error, atom()}
+  def from_params(params) do
+    max_peers = Map.get(params, "maxPeers")
+    video_codec = Map.get(params, "videoCodec")
+    webhook_url = Map.get(params, "webhookUrl")
+
+    with :ok <- validate_max_peers(max_peers),
+         {:ok, video_codec} <- codec_to_atom(video_codec),
+         :ok <- validate_webhook_url(webhook_url) do
+      {:ok,
+       %__MODULE__{
+         max_peers: max_peers,
+         video_codec: video_codec,
+         webhook_url: webhook_url
+       }}
+    else
+      error -> error
+    end
+  end
+
+  defp validate_max_peers(nil), do: :ok
+  defp validate_max_peers(max_peers) when is_integer(max_peers) and max_peers >= 0, do: :ok
+  defp validate_max_peers(_max_peers), do: {:error, :invalid_max_peers}
+
+  defp validate_webhook_url(nil), do: :ok
+
+  defp validate_webhook_url(uri) do
+    uri
+    |> URI.parse()
+    |> Map.take([:host, :path, :scheme])
+    |> Enum.all?(fn {_key, value} -> not is_nil(value) end)
+    |> if do
+      :ok
+    else
+      {:error, :invalid_webhook_url}
+    end
+  end
+
+  defp codec_to_atom("h264"), do: {:ok, :h264}
+  defp codec_to_atom("vp8"), do: {:ok, :vp8}
+  defp codec_to_atom(nil), do: {:ok, nil}
+  defp codec_to_atom(_codec), do: {:error, :invalid_video_codec}
+end

lib/jellyfish/room_service.ex

@@ -53,9 +53,8 @@ defmodule Jellyfish.RoomService do
     |> Enum.reject(&(&1 == nil))
   end
 
-  @spec create_room(Room.max_peers(), String.t(), String.t()) ::
-          {:ok, Room.t(), String.t()} | {:error, :invalid_max_peers | :invalid_video_codec}
-  def create_room(max_peers, video_codec, webhook_url) do
+  @spec create_room(Room.Config.t()) :: {Room.t(), String.t()}
+  def create_room(config) do
     {node_resources, failed_nodes} =
       :rpc.multicall(Jellyfish.RoomService, :get_resource_usage, [])
 
@@ -79,9 +78,9 @@ defmodule Jellyfish.RoomService do
 
     if Enum.count(node_resources) > 1 do
       Logger.info("Node with least used resources is #{inspect(min_node)}")
-      GenServer.call({__MODULE__, min_node}, {:create_room, max_peers, video_codec, webhook_url})
+      GenServer.call({__MODULE__, min_node}, {:create_room, config})
     else
-      GenServer.call(__MODULE__, {:create_room, max_peers, video_codec, webhook_url})
+      GenServer.call(__MODULE__, {:create_room, config})
     end
   end
 
@@ -130,34 +129,21 @@ defmodule Jellyfish.RoomService do
   end
 
   @impl true
-  def handle_call({:create_room, max_peers, video_codec, webhook_url}, _from, state) do
-    with :ok <- validate_max_peers(max_peers),
-         {:ok, video_codec} <- codec_to_atom(video_codec),
-         :ok <- validate_webhook_url(webhook_url) do
-      {:ok, room_pid, room_id} = Room.start(max_peers, video_codec)
+  def handle_call({:create_room, config}, _from, state) do
+    {room_pid, room_id} = Room.start(config)
 
-      room = Room.get_state(room_id)
-      Process.monitor(room_pid)
-
-      state = put_in(state, [:rooms, room_pid], room_id)
-
-      WebhookNotifier.add_webhook(room_id, webhook_url)
+    room = Room.get_state(room_id)
+    Process.monitor(room_pid)
 
-      Logger.info("Created room #{inspect(room.id)}")
+    state = put_in(state, [:rooms, room_pid], room_id)
 
-      Event.broadcast_server_notification({:room_created, room_id})
+    WebhookNotifier.add_webhook(room_id, config.webhook_url)
 
-      {:reply, {:ok, room, Application.fetch_env!(:jellyfish, :address)}, state}
-    else
-      {:error, :max_peers} ->
-        {:reply, {:error, :invalid_max_peers}, state}
+    Logger.info("Created room #{inspect(room.id)}")
 
-      {:error, :video_codec} ->
-        {:reply, {:error, :invalid_video_codec}, state}
+    Event.broadcast_server_notification({:room_created, room_id})
 
-      {:error, :invalid_webhook_url} ->
-        {:reply, {:error, :invalid_webhook_url}, state}
-    end
+    {:reply, {room, Application.fetch_env!(:jellyfish, :address)}, state}
   end
 
   @impl true
@@ -234,27 +220,4 @@ defmodule Jellyfish.RoomService do
         Logger.warning("Room process with id #{inspect(room_id)} doesn't exist")
     end
   end
-
-  defp validate_max_peers(nil), do: :ok
-  defp validate_max_peers(max_peers) when is_integer(max_peers) and max_peers >= 0, do: :ok
-  defp validate_max_peers(_max_peers), do: {:error, :max_peers}
-
-  defp validate_webhook_url(nil), do: :ok
-
-  defp validate_webhook_url(uri) do
-    uri
-    |> URI.parse()
-    |> Map.take([:host, :path, :scheme])
-    |> Enum.all?(fn {_key, value} -> not is_nil(value) end)
-    |> if do
-      :ok
-    else
-      {:error, :invalid_webhook_url}
-    end
-  end
-
-  defp codec_to_atom("h264"), do: {:ok, :h264}
-  defp codec_to_atom("vp8"), do: {:ok, :vp8}
-  defp codec_to_atom(nil), do: {:ok, nil}
-  defp codec_to_atom(_codec), do: {:error, :video_codec}
 end

lib/jellyfish_web/api_spec/hls.ex

@@ -33,9 +33,8 @@ defmodule JellyfishWeb.ApiSpec.HLS do
 
       OpenApiSpex.schema(%{
         type: :string,
-        enum: ["YES"],
         example: "YES",
-        description: "Is delta manifest requested",
+        description: "Set to \"YES\" if delta manifest should be requested",
         nullable: true
       })
     end

lib/jellyfish_web/api_spec/room.ex

@@ -46,11 +46,13 @@ defmodule JellyfishWeb.ApiSpec.Room do
       config: Config,
       components: %Schema{
         type: :array,
-        items: Component
+        items: Component,
+        description: "List of all components"
       },
       peers: %Schema{
         type: :array,
-        items: Peer
+        items: Peer,
+        description: "List of all peers"
       }
     },
     required: [:id, :config, :components, :peers]

lib/jellyfish_web/controllers/room_controller.ex

@@ -2,6 +2,7 @@ defmodule JellyfishWeb.RoomController do
   use JellyfishWeb, :controller
   use OpenApiSpex.ControllerSpecs
 
+  alias Jellyfish.Room
   alias Jellyfish.RoomService
   alias JellyfishWeb.ApiSpec
   alias OpenApiSpex.Response
@@ -71,11 +72,9 @@ defmodule JellyfishWeb.RoomController do
   end
 
   def create(conn, params) do
-    with max_peers <- Map.get(params, "maxPeers"),
-         video_codec <- Map.get(params, "videoCodec"),
-         webhook_url <- Map.get(params, "webhookUrl"),
-         {:ok, room, jellyfish_address} <-
-           RoomService.create_room(max_peers, video_codec, webhook_url) do
+    with {:ok, config} <- Room.Config.from_params(params) do
+      {room, jellyfish_address} = RoomService.create_room(config)
+
       conn
       |> put_resp_content_type("application/json")
       |> put_status(:created)

lib/jellyfish_web/controllers/room_json.ex

@@ -27,7 +27,7 @@ defmodule JellyfishWeb.RoomJSON do
   defp room_data(room) do
     %{
       id: room.id,
-      config: room.config |> ParserJSON.camel_case_keys(),
+      config: room.config |> Map.from_struct() |> ParserJSON.camel_case_keys(),
       components: room.components |> Enum.map(&ComponentJSON.data/1),
       peers: room.peers |> Enum.map(&PeerJSON.data/1)
     }

openapi.yaml

@@ -254,9 +254,7 @@ components:
       type: string
       x-struct: Elixir.JellyfishWeb.ApiSpec.HLS.Response
     HlsSkip:
-      description: Is delta manifest requested
-      enum:
-        - YES
+      description: Set to "YES" if delta manifest should be requested
       example: YES
       nullable: true
       title: HlsSkip
@@ -346,6 +344,7 @@ components:
       description: Description of the room state
       properties:
         components:
+          description: List of all components
           items:
             $ref: '#/components/schemas/Component'
           type: array
@@ -356,6 +355,7 @@ components:
           example: room-1
           type: string
         peers:
+          description: List of all peers
           items:
             $ref: '#/components/schemas/Peer'
           type: array