From b47007ffd4feff91a4e2c1a1c7f15670afc7d74a Mon Sep 17 00:00:00 2001
From: Jakub Pisarek <99591440+sgfn@users.noreply.github.com>
Date: Wed, 23 Aug 2023 15:07:15 +0200
Subject: [PATCH 1/2] [RTC-333] Limit HLS components to 1 per room

---
 lib/jellyfish/room.ex                         | 39 ++++++++++++++-----
 .../controllers/component_controller.ex       |  3 ++
 .../controllers/component_controller_test.exs |  8 +++-
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/lib/jellyfish/room.ex b/lib/jellyfish/room.ex
index bb4058b8..8252d972 100644
--- a/lib/jellyfish/room.ex
+++ b/lib/jellyfish/room.ex
@@ -97,7 +97,9 @@ defmodule Jellyfish.Room do
   end
 
   @spec add_component(id(), Component.component(), map()) ::
-          {:ok, Component.t()} | :error | {:error, :incompatible_codec}
+          {:ok, Component.t()}
+          | :error
+          | {:error, :incompatible_codec | :reached_components_limit}
   def add_component(room_id, component_type, options \\ %{}) do
     GenServer.call(registry_id(room_id), {:add_component, component_type, options})
   end
@@ -216,18 +218,14 @@ defmodule Jellyfish.Room do
   end
 
   @impl true
-  def handle_call(
-        {:add_component, component_type, options},
-        _from,
-        %{config: %{video_codec: video_codec}} = state
-      ) do
+  def handle_call({:add_component, component_type, options}, _from, state) do
     options =
       Map.merge(
         %{engine_pid: state.engine_pid, room_id: state.id},
         options
       )
 
-    with :ok <- check_video_codec(video_codec, component_type),
+    with :ok <- check_component_allowed(component_type, state),
          {:ok, component} <- Component.new(component_type, options) do
       state = put_in(state, [:components, component.id], component)
 
@@ -241,6 +239,13 @@ defmodule Jellyfish.Room do
         Logger.warn("Unable to add component: incompatible codec, HLS needs 'h264' video codec.")
         {:reply, {:error, :incompatible_codec}, state}
 
+      {:error, :reached_components_limit} ->
+        Logger.warn(
+          "Unable to add component: reached components limit, max 1 HLS component allowed per room."
+        )
+
+        {:reply, {:error, :reached_components_limit}, state}
+
       {:error, reason} ->
         Logger.warn("Unable to add component: #{inspect(reason)}")
         {:reply, :error, state}
@@ -386,7 +391,21 @@ defmodule Jellyfish.Room do
 
   defp registry_id(room_id), do: {:via, Registry, {Jellyfish.RoomRegistry, room_id}}
 
-  defp check_video_codec(:h264, Jellyfish.Component.HLS), do: :ok
-  defp check_video_codec(_codec, Jellyfish.Component.HLS), do: {:error, :incompatible_codec}
-  defp check_video_codec(_codec, _component), do: :ok
+  defp check_component_allowed(Component.HLS, %{
+         config: %{video_codec: video_codec},
+         components: components
+       }) do
+    cond do
+      video_codec != :h264 ->
+        {:error, :incompatible_codec}
+
+      Map.values(components) |> Enum.any?(&(&1.type == Component.HLS)) ->
+        {:error, :reached_components_limit}
+
+      true ->
+        :ok
+    end
+  end
+
+  defp check_component_allowed(_component_type, _state), do: :ok
 end
diff --git a/lib/jellyfish_web/controllers/component_controller.ex b/lib/jellyfish_web/controllers/component_controller.ex
index 2a4da461..1944ffb1 100644
--- a/lib/jellyfish_web/controllers/component_controller.ex
+++ b/lib/jellyfish_web/controllers/component_controller.ex
@@ -80,6 +80,9 @@ defmodule JellyfishWeb.ComponentController do
 
       {:error, :incompatible_codec} ->
         {:error, :bad_request, "HLS component needs room with video codec 'h264' enforced"}
+
+      {:error, :reached_components_limit} ->
+        {:error, :bad_request, "Max 1 HLS component allowed per room"}
     end
   end
 
diff --git a/test/jellyfish_web/controllers/component_controller_test.exs b/test/jellyfish_web/controllers/component_controller_test.exs
index 2ce8f44b..16a690eb 100644
--- a/test/jellyfish_web/controllers/component_controller_test.exs
+++ b/test/jellyfish_web/controllers/component_controller_test.exs
@@ -36,7 +36,7 @@ defmodule JellyfishWeb.ComponentControllerTest do
   end
 
   describe "create hls component" do
-    test "renders component when data is valid", %{conn: conn} do
+    test "renders component when data is valid, allows max 1 hls per room", %{conn: conn} do
       room_conn = post(conn, ~p"/room", videoCodec: "h264")
       assert %{"id" => room_id} = json_response(room_conn, :created)["data"]
 
@@ -57,6 +57,12 @@ defmodule JellyfishWeb.ComponentControllerTest do
                ]
              } = json_response(conn, :ok)["data"]
 
+      # Try to add another hls component
+      conn = post(conn, ~p"/room/#{room_id}/component", type: "hls")
+
+      assert json_response(conn, :bad_request)["errors"] ==
+               "Max 1 HLS component allowed per room"
+
       room_conn = delete(conn, ~p"/room/#{room_id}")
       assert response(room_conn, :no_content)
     end

From 11a2cf4a836b9ec824c724c2e7563e4a1d30294e Mon Sep 17 00:00:00 2001
From: Jakub Pisarek <99591440+sgfn@users.noreply.github.com>
Date: Wed, 23 Aug 2023 20:26:51 +0200
Subject: [PATCH 2/2] Review suggestion

---
 lib/jellyfish/room.ex | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/jellyfish/room.ex b/lib/jellyfish/room.ex
index 8252d972..76331b66 100644
--- a/lib/jellyfish/room.ex
+++ b/lib/jellyfish/room.ex
@@ -399,7 +399,7 @@ defmodule Jellyfish.Room do
       video_codec != :h264 ->
         {:error, :incompatible_codec}
 
-      Map.values(components) |> Enum.any?(&(&1.type == Component.HLS)) ->
+      hls_component_already_present?(components) ->
         {:error, :reached_components_limit}
 
       true ->
@@ -408,4 +408,7 @@ defmodule Jellyfish.Room do
   end
 
   defp check_component_allowed(_component_type, _state), do: :ok
+
+  defp hls_component_already_present?(components),
+    do: components |> Map.values() |> Enum.any?(&(&1.type == Component.HLS))
 end