RTC-377 SIP Component by Rados13 · Pull Request #147 · fishjam-dev/fishjam

Rados13 · 2024-02-02T09:21:19Z

Acknowledging the stipulations set forth:

I hereby confirm that a Pull Request involving updates to the Software Development Kit (SDK) has been smoothly merged, currently awaits processing, or is otherwise deemed unnecessary in this context.
I also affirm that another Pull Request, specifically addressing updates to the documentation body (commonly referred to as 'docs'), has either been successfully incorporated, is in the process of review, or is considered superfluous under the prevailing circumstances.

codecov · 2024-02-09T12:26:43Z

Codecov Report

Merging #147 (ccfd64a) into main (090a338) will decrease coverage by 0.16%.
The diff coverage is 89.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
- Coverage   86.84%   86.69%   -0.16%     
==========================================
  Files          64       68       +4     
  Lines        1247     1293      +46     
==========================================
+ Hits         1083     1121      +38     
- Misses        164      172       +8

Files	Coverage Δ
lib/jellyfish/component.ex	`100.00% <100.00%> (ø)`
lib/jellyfish/component/file.ex	`100.00% <ø> (ø)`
lib/jellyfish/component/hls.ex	`96.77% <100.00%> (+5.46%)`	⬆️
lib/jellyfish/component/hls/manager.ex	`97.05% <ø> (ø)`
lib/jellyfish/component/rtsp.ex	`83.33% <100.00%> (-2.39%)`	⬇️
lib/jellyfish/config_reader.ex	`94.05% <100.00%> (+0.44%)`	⬆️
lib/jellyfish/peer/webrtc.ex	`48.27% <100.00%> (ø)`
lib/jellyfish_web/api_spec/component.ex	`100.00% <ø> (ø)`
lib/jellyfish_web/api_spec/component/sip.ex	`100.00% <100.00%> (ø)`
lib/jellyfish_web/api_spec/dial.ex	`100.00% <100.00%> (ø)`
... and 5 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 090a338...ccfd64a. Read the comment docs.

lib/jellyfish/component.ex

lib/jellyfish/component/file.ex

lib/jellyfish/component/hls/manager.ex

lib/jellyfish/config_reader.ex

lib/jellyfish_web/controllers/sip_call_controller.ex

lib/jellyfish_web/router.ex

lib/jellyfish_web/controllers/sip_call_controller.ex

test/jellyfish_web/controllers/component/sip_component_test.exs

roznawsk · 2024-02-14T10:09:28Z

lib/jellyfish_web/api_spec/component/sip.ex

+    require OpenApiSpex
+
+    OpenApiSpex.schema(%{
+      title: "Credentials",


I suggest we rename it to SIPCredentials, to indicate that those are sip-specific credentials.

Karolk99 · 2024-02-15T11:00:15Z

test/jellyfish/component/hls/manager_test.exs

    for filename <- @files, do: :ok = hls_dir |> Path.join(filename) |> File.touch!()

-    on_exit(fn -> File.rm_rf!(hls_dir) end)
+    on_exit(fn -> hls_dir |> File.rm_rf!() end)


As far as I know, this is anti-pattern

Karolk99 · 2024-02-15T11:17:18Z

lib/jellyfish/component/sip.ex

+    with {:ok, valid_opts} <- OpenApiSpex.cast_value(options, Options.schema()) do
+      endpoint_spec =
+        Map.from_struct(valid_opts)
+        # OpenApiSpex will remove invalid options, so the following conversion, while ugly, is memory-safe
+        |> Map.new(fn {k, v} ->
+          {Atom.to_string(k) |> Macro.underscore() |> String.to_atom(), v}
+        end)
+        |> Map.put(:rtc_engine, engine)
+        |> Map.put(:external_ip, external_ip)
+        |> Map.update!(:registrar_credentials, fn credentials ->
+          credentials
+          |> Map.to_list()
+          |> Keyword.new()
+          |> RegistrarCredentials.new()
+        end)
+        |> then(&struct(SIP, &1))
+
+      properties = valid_opts |> Map.from_struct()
+
+      {:ok, %{endpoint: endpoint_spec, properties: properties}}
+    else
+      {:error, [%OpenApiSpex.Cast.Error{reason: :missing_field, name: name} | _rest]} ->
+        {:error, {:missing_parameter, name}}
+
+      {:error, _reason} = error ->
+        error


I would use function from HLS component

Suggested change

with {:ok, valid_opts} <- OpenApiSpex.cast_value(options, Options.schema()) do

endpoint_spec =

Map.from_struct(valid_opts)

# OpenApiSpex will remove invalid options, so the following conversion, while ugly, is memory-safe

|> Map.new(fn {k, v} ->

{Atom.to_string(k) |> Macro.underscore() |> String.to_atom(), v}

end)

|> Map.put(:rtc_engine, engine)

|> Map.put(:external_ip, external_ip)

|> Map.update!(:registrar_credentials, fn credentials ->

credentials

|> Map.to_list()

|> Keyword.new()

|> RegistrarCredentials.new()

end)

|> then(&struct(SIP, &1))

properties = valid_opts |> Map.from_struct()

{:ok, %{endpoint: endpoint_spec, properties: properties}}

else

{:error, [%OpenApiSpex.Cast.Error{reason: :missing_field, name: name} | _rest]} ->

{:error, {:missing_parameter, name}}

{:error, _reason} = error ->

error

with {:ok, valid_opts} <- OpenApiSpex.cast_value(options, Options.schema()),

{:ok, serialized_opts} <- serialize_options(options) do

# endpoint_spec =

# serialized_opts

# |> Map.put(:rtc_engine, engine)

# |> Map.put(:external_ip, external_ip)

# |> Map.update!(:registrar_credentials, fn credentials ->

# credentials

# |> Map.to_list()

# |> Keyword.new()

# |> RegistrarCredentials.new()

# end)

# |> then(&struct(SIP, &1))

endpoint_spec = %SIP{...}

{:ok, %{endpoint: endpoint_spec, properties: serialized_opts}}

else

{:error, [%OpenApiSpex.Cast.Error{reason: :missing_field, name: name} | _rest]} ->

{:error, {:missing_parameter, name}}

{:error, _reason} = error ->

error

sgfn · 2024-02-19T11:22:59Z

lib/jellyfish/component.ex

+  defmacro __using__(_opts) do
+    quote location: :keep do
+      @behaviour Jellyfish.Component
+
+      @impl true
+      def after_init(_room_state, _component, _component_options), do: :ok
+
+      @impl true
+      def on_remove(_room_state, _component), do: :ok
+
+      defoverridable after_init: 3, on_remove: 2
+
+      def serialize_options(opts, opts_schema) do
+        with {:ok, valid_opts} <- OpenApiSpex.Cast.cast(opts_schema, opts) do
+          valid_opts =
+            valid_opts
+            |> Map.from_struct()
+            |> Map.new(fn {k, v} -> {underscore(k), serialize(v)} end)
+
+          {:ok, valid_opts}
+        end
+      end
+
+      defp serialize(v) when is_struct(v),
+        do: v |> Map.from_struct() |> Map.new(fn {k, v} -> {underscore(k), v} end)
+
+      defp serialize(v), do: v
+
+      defp underscore(k), do: k |> Atom.to_string() |> Macro.underscore() |> String.to_atom()
+    end
+  end


💪 💪 💪

sgfn · 2024-02-19T11:32:14Z

lib/jellyfish_web/controllers/sip_call_controller.ex

+      not_found: ApiSpec.error("Room doesn't exist")
+    ]


+unauthorized

lib/jellyfish_web/router.ex

Co-authored-by: Jakub Pisarek <99591440+sgfn@users.noreply.github.com>

Rados13 self-assigned this Feb 2, 2024

Rados13 force-pushed the SIP-component branch from 3671be2 to 66b6e8b Compare February 8, 2024 14:36

Rados13 requested review from Karolk99 and sgfn February 9, 2024 12:55

Rados13 force-pushed the SIP-component branch from 5927cd8 to adcb972 Compare February 12, 2024 09:08

sgfn reviewed Feb 12, 2024

View reviewed changes

Rados13 changed the title ~~SIP Component~~ RTC-377 SIP Component Feb 13, 2024

roznawsk reviewed Feb 14, 2024

View reviewed changes

Karolk99 approved these changes Feb 15, 2024

View reviewed changes

Rados13 added 10 commits February 15, 2024 16:12

WiP

2677471

WiP x2

52605bb

WIP x3

69deb4f

Improve SIP configuration

2be6715

Generate new api.spec

c7a932d

Fix dialyzer issues

3496abe

Rename SIP properties

547cc4e

Little fixes

0bbdfab

Fixes after review

08332e1

Generate new OpenAPI spec

0a4259a

Rados13 force-pushed the SIP-component branch from c9094d8 to 0a4259a Compare February 15, 2024 15:18

Add default implementation of Jellyfish.Component behaviour

a3617a0

Rados13 requested review from roznawsk and sgfn February 16, 2024 09:04

Rados13 force-pushed the SIP-component branch 3 times, most recently from 6bd1842 to d07c085 Compare February 16, 2024 10:33

Fix ApiSpec.Component.SIP

bc3ff2b

Rados13 force-pushed the SIP-component branch from d07c085 to bc3ff2b Compare February 16, 2024 10:39

Fix OpenAPISpex

6b60c77

sgfn approved these changes Feb 19, 2024

View reviewed changes

Rados13 and others added 3 commits February 19, 2024 12:44

Update lib/jellyfish_web/router.ex

6f415ba

Co-authored-by: Jakub Pisarek <99591440+sgfn@users.noreply.github.com>

Add missing unauthorized response

99d31b0

Fix SIP resource

ccfd64a

Rados13 merged commit 9dff586 into main Feb 19, 2024

Rados13 deleted the SIP-component branch February 19, 2024 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RTC-377 SIP Component#147

RTC-377 SIP Component#147
Rados13 merged 16 commits intomainfrom
SIP-component

Rados13 commented Feb 2, 2024 •

edited

Loading

Uh oh!

codecov bot commented Feb 9, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roznawsk Feb 14, 2024

Uh oh!

Karolk99 Feb 15, 2024

Uh oh!

Karolk99 Feb 15, 2024

Uh oh!

sgfn Feb 19, 2024

Uh oh!

sgfn Feb 19, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Rados13 commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Acknowledging the stipulations set forth:

Uh oh!

codecov bot commented Feb 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roznawsk Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

Karolk99 Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

Karolk99 Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

sgfn Feb 19, 2024

Choose a reason for hiding this comment

Uh oh!

sgfn Feb 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Rados13 commented Feb 2, 2024 •

edited

Loading

codecov bot commented Feb 9, 2024 •

edited

Loading