Add delete_attribute(s)/2. Move attributes API to the ExSDP module. (#40

)
membraneframework · Dec 14, 2023 · 97db6bd · 97db6bd
1 parent 66fa6be
commit 97db6bd
Show file tree

Hide file tree

Showing 8 changed files with 119 additions and 45 deletions.
diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ The package can be installed by adding `ex_sdp` to your list of dependencies in
 ```elixir
 def deps do
   [
-    {:ex_sdp, "~> 0.13.1"}
+    {:ex_sdp, "~> 0.14.0"}
   ]
 end
 ```

diff --git a/lib/ex_sdp.ex b/lib/ex_sdp.ex
@@ -93,21 +93,27 @@ defmodule ExSDP do
     }
   end
 
-  @spec add_media(sdp :: t(), media :: Media.t() | [Media.t()]) :: t()
+  @spec add_media(t(), Media.t() | [Media.t()]) :: t() | Media.t()
   def add_media(sdp, media), do: Map.update!(sdp, :media, &(&1 ++ Bunch.listify(media)))
 
-  @spec add_attribute(sdp :: t(), attribute :: Attribute.t()) :: t()
-  def add_attribute(sdp, attribute), do: add_attributes(sdp, [attribute])
+  @spec add_attribute(t() | Media.t(), Attribute.t()) :: t() | Media.t()
+  def add_attribute(sdp_or_media, attribute), do: add_attributes(sdp_or_media, [attribute])
 
-  @spec add_attributes(sdp :: t(), attributes :: [Attribute.t()]) :: t()
-  def add_attributes(sdp, attributes) when is_list(attributes),
-    do: Map.update!(sdp, :attributes, &(&1 ++ attributes))
+  @spec add_attributes(t() | Media.t(), [Attribute.t()]) :: t() | Media.t()
+  def add_attributes(sdp_or_media, attributes) when is_list(attributes),
+    do: Map.update!(sdp_or_media, :attributes, &(&1 ++ attributes))
 
-  @spec get_attribute(sdp :: t(), key :: module() | atom() | binary()) :: Attribute.t() | nil
-  def get_attribute(sdp, key), do: Utils.get_attribute(sdp, key)
+  @spec get_attribute(t() | Media.t(), Attribute.key()) :: Attribute.t() | nil
+  def get_attribute(sdp_or_media, key), do: Utils.get_attribute(sdp_or_media, key)
 
-  @spec get_attributes(sdp :: t(), key :: module() | atom() | binary()) :: [Attribute.t()]
-  def get_attributes(sdp, key), do: Utils.get_attributes(sdp, key)
+  @spec get_attributes(t() | Media.t(), Attribute.key()) :: [Attribute.t()]
+  def get_attributes(sdp_or_media, key), do: Utils.get_attributes(sdp_or_media, key)
+
+  @spec delete_attribute(t() | Media.t(), Attribute.key()) :: t() | Media.t()
+  def delete_attribute(sdp_or_media, key), do: delete_attributes(sdp_or_media, [key])
+
+  @spec delete_attributes(t() | Media.t(), [Attribute.key()]) :: t() | Media.t()
+  def delete_attributes(sdp_or_media, keys), do: Utils.delete_attributes(sdp_or_media, keys)
 end
 
 defimpl String.Chars, for: ExSDP do

diff --git a/lib/ex_sdp/attribute.ex b/lib/ex_sdp/attribute.ex
@@ -73,6 +73,8 @@ defmodule ExSDP.Attribute do
           | {String.t(), String.t()}
           | String.t()
 
+  @type key() :: module() | binary() | atom()
+
   @flag_attributes_strings @flag_attributes |> Enum.map(&to_string/1)
 
   @doc """

diff --git a/lib/ex_sdp/media.ex b/lib/ex_sdp/media.ex
@@ -47,12 +47,8 @@ defmodule ExSDP.Media do
   """
   @type type :: :audio | :video | :text | :application | :message | binary()
 
-  @spec new(
-          type :: type(),
-          port :: :inet.port_number(),
-          protocol :: binary(),
-          fmt :: binary() | 0..127 | [0..127],
-          opts :: [port_count: non_neg_integer()]
+  @spec new(type(), :inet.port_number(), binary(), binary() | 0..127 | [0..127],
+          port_count: non_neg_integer()
         ) :: t()
   def new(type, port, protocol, fmt, opts \\ []) do
     %__MODULE__{
@@ -64,17 +60,21 @@ defmodule ExSDP.Media do
     }
   end
 
-  @spec add_attribute(media :: t(), attribute :: Attribute.t()) :: t()
+  @deprecated "Use ExSDP.add_attribute/2 instead"
+  @spec add_attribute(t(), Attribute.t()) :: t()
   def add_attribute(media, attribute), do: add_attributes(media, [attribute])
 
-  @spec add_attributes(media :: t(), attributes :: [Attribute.t()]) :: t()
+  @deprecated "Use ExSDP.add_attributes/2 instead"
+  @spec add_attributes(t(), [Attribute.t()]) :: t()
   def add_attributes(media, attributes) when is_list(attributes),
     do: Map.update!(media, :attributes, &(&1 ++ attributes))
 
-  @spec get_attribute(media :: t(), key :: module() | atom() | binary()) :: Attribute.t() | nil
+  @deprecated "Use ExSDP.get_attribute/2 instead"
+  @spec get_attribute(t(), Attribute.key()) :: Attribute.t() | nil
   def get_attribute(media, key), do: Utils.get_attribute(media, key)
 
-  @spec get_attributes(media :: t(), key :: module() | atom() | binary()) :: [Attribute.t()]
+  @deprecated "Use ExSDP.get_attributes/2 instead"
+  @spec get_attributes(t(), Attribute.key()) :: [Attribute.t()]
   def get_attributes(media, key), do: Utils.get_attributes(media, key)
 
   @spec parse(binary()) :: {:ok, t()} | {:error, :invalid_media_spec | :malformed_port_number}

diff --git a/lib/ex_sdp/utils.ex b/lib/ex_sdp/utils.ex
@@ -26,7 +26,7 @@ defmodule ExSDP.Utils do
     :ssrc_group => SSRCGroup
   }
 
-  @spec get_attribute(sdp_or_media :: ExSDP.t() | Media.t(), key :: module() | atom() | binary()) ::
+  @spec get_attribute(sdp_or_media :: ExSDP.t() | Media.t(), Attribute.key()) ::
           Attribute.t() | nil
   def get_attribute(sdp_or_media, key) do
     key = Map.get(@struct_attr_keys, key, key)
@@ -40,8 +40,7 @@ defmodule ExSDP.Utils do
     end)
   end
 
-  @spec get_attributes(sdp_or_media :: ExSDP.t() | Media.t(), key :: module() | atom() | binary()) ::
-          [Attribute.t()]
+  @spec get_attributes(sdp_or_media :: ExSDP.t() | Media.t(), Attribute.key()) :: [Attribute.t()]
   def get_attributes(sdp_or_media, key) do
     key = Map.get(@struct_attr_keys, key, key)
 
@@ -54,6 +53,22 @@ defmodule ExSDP.Utils do
     end)
   end
 
+  @spec delete_attributes(ExSDP.t() | ExSDP.Media.t(), [Attribute.key()]) ::
+          ExSDP.t() | ExSDP.Media.t()
+  def delete_attributes(sdp_or_mline, keys) when is_list(keys) do
+    keys = Enum.map(keys, fn key -> Map.get(@struct_attr_keys, key, key) end)
+
+    new_attrs =
+      Enum.reject(sdp_or_mline.attributes, fn
+        %module{} -> module in keys
+        {k, _v} -> k in keys
+        # flag attributes
+        k -> k in keys
+      end)
+
+    Map.put(sdp_or_mline, :attributes, new_attrs)
+  end
+
   @spec split(String.t(), String.t() | [String.t()] | :binary.cp() | Regex.t(), any) ::
           {:error, :too_few_fields} | {:ok, [String.t()]}
   def split(origin, delim, expected_len) do

diff --git a/mix.exs b/mix.exs
@@ -1,7 +1,7 @@
 defmodule ExSDP.MixProject do
   use Mix.Project
 
-  @version "0.13.1"
+  @version "0.14.0"
   @github_url "https://github.com/membraneframework/ex_sdp"
 
   def project do

diff --git a/test/ex_sdp/media_test.exs b/test/ex_sdp/media_test.exs
@@ -256,7 +256,7 @@ defmodule ExSDP.MediaTest do
   end
 
   describe "Utils functions" do
-    test "gets attribute by atom or module" do
+    test "gets and deletes attribute by atom, binary or module" do
       rtpmap = %RTPMapping{clock_rate: 8000, encoding: "L8", params: 1, payload_type: 96}
 
       ssrc = %SSRC{id: 12_345, attribute: "cname", value: "HPd3XfRHXYUxzfsJ"}
@@ -274,26 +274,68 @@ defmodule ExSDP.MediaTest do
 
       media =
         Media.new(:video, 51_372, "RTP/AVP", [99])
-        |> Media.add_attribute(rtpmap)
-        |> Media.add_attribute(ssrc)
-        |> Media.add_attribute(fmtp)
-        |> Media.add_attribute(msid)
-        |> Media.add_attribute(extmap)
+        |> ExSDP.add_attribute(rtpmap)
+        |> ExSDP.add_attribute(ssrc)
+        |> ExSDP.add_attribute(fmtp)
+        |> ExSDP.add_attribute(msid)
+        |> ExSDP.add_attribute(extmap)
+        |> ExSDP.add_attribute({"key", "value"})
 
-      assert rtpmap == Media.get_attribute(media, RTPMapping)
-      assert rtpmap == Media.get_attribute(media, :rtpmap)
+      assert rtpmap == ExSDP.get_attribute(media, RTPMapping)
+      assert rtpmap == ExSDP.get_attribute(media, :rtpmap)
 
-      assert ssrc == Media.get_attribute(media, SSRC)
-      assert ssrc == Media.get_attribute(media, :ssrc)
+      assert ssrc == ExSDP.get_attribute(media, SSRC)
+      assert ssrc == ExSDP.get_attribute(media, :ssrc)
 
-      assert fmtp == Media.get_attribute(media, FMTP)
-      assert fmtp == Media.get_attribute(media, :fmtp)
+      assert fmtp == ExSDP.get_attribute(media, FMTP)
+      assert fmtp == ExSDP.get_attribute(media, :fmtp)
 
-      assert msid == Media.get_attribute(media, MSID)
-      assert msid == Media.get_attribute(media, :msid)
+      assert msid == ExSDP.get_attribute(media, MSID)
+      assert msid == ExSDP.get_attribute(media, :msid)
 
-      assert extmap == Media.get_attribute(media, Extmap)
-      assert extmap == Media.get_attribute(media, :extmap)
+      assert extmap == ExSDP.get_attribute(media, Extmap)
+      assert extmap == ExSDP.get_attribute(media, :extmap)
+
+      assert {"key", "value"} == ExSDP.get_attribute(media, "key")
+
+      assert [%SSRC{}, %FMTP{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, RTPMapping).attributes
+
+      assert [%SSRC{}, %FMTP{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, :rtpmap).attributes
+
+      assert [%RTPMapping{}, %FMTP{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, SSRC).attributes
+
+      assert [%RTPMapping{}, %FMTP{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, :ssrc).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, FMTP).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %MSID{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, :fmtp).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %FMTP{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, MSID).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %FMTP{}, %Extmap{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, :msid).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %FMTP{}, %MSID{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, Extmap).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %FMTP{}, %MSID{}, {"key", "value"}] =
+               ExSDP.delete_attribute(media, :extmap).attributes
+
+      assert [%RTPMapping{}, %SSRC{}, %FMTP{}, %MSID{}, %Extmap{}] =
+               ExSDP.delete_attribute(media, "key").attributes
+
+      assert [] ==
+               ExSDP.delete_attributes(media, [RTPMapping, SSRC, FMTP, MSID, Extmap, "key"]).attributes
+
+      assert [] ==
+               ExSDP.delete_attributes(media, [:rtpmap, :ssrc, :fmtp, :msid, :extmap, "key"]).attributes
     end
   end
 end
diff --git a/test/sdp_test.exs b/test/sdp_test.exs
@@ -188,8 +188,8 @@ defmodule ExSDPTest do
     end
   end
 
-  describe "get_attribute/2" do
-    test "gets attribute by atom, binary or module" do
+  describe "Utils functions" do
+    test "gets and deletes attribute by atom, binary or module" do
       sdp =
         ExSDP.new()
         |> ExSDP.add_attribute({"key", "value"})
@@ -208,10 +208,19 @@ defmodule ExSDPTest do
       assert nil == ExSDP.get_attribute(sdp, :non_existing_atom)
       assert nil == ExSDP.get_attribute(sdp, ExSDP.Attribute.NonExistingModule)
       assert nil == ExSDP.get_attribute(sdp, "non_existing_string")
+
+      assert [:recvonly, %ExSDP.Attribute.Group{}] = ExSDP.delete_attribute(sdp, "key").attributes
+
+      assert [{"key", "value"}, %ExSDP.Attribute.Group{}] =
+               ExSDP.delete_attribute(sdp, :recvonly).attributes
+
+      assert [{"key", "value"}, :recvonly] =
+               ExSDP.delete_attribute(sdp, ExSDP.Attribute.Group).attributes
+
+      assert [] =
+               ExSDP.delete_attributes(sdp, ["key", :recvonly, ExSDP.Attribute.Group]).attributes
     end
-  end
 
-  describe "get_attributes/2" do
     test "gets multiple attributes by module" do
       sdp =
         ExSDP.new()