Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Make API calls to Jellyfish and receive server events:
client = Jellyfish.Client.new()

# Create room
{:ok, %Jellyfish.Room{id: room_id}} = Jellyfish.Room.create(client, max_peers: 10)
{:ok, %Jellyfish.Room{id: room_id}, jellyfish_address} = Jellyfish.Room.create(client, max_peers: 10)

room_id
# => "8878cd13-99a6-40d6-8d7e-8da23d803dab"
Expand Down
4 changes: 0 additions & 4 deletions config/config.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
import Config

config :jellyfish_server_sdk,
divo: "docker-compose-integration.yaml",
divo_wait: [dwell: 1_500, max_tries: 50]

import_config "#{config_env()}.exs"
4 changes: 3 additions & 1 deletion config/integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ import Config

config :jellyfish_server_sdk,
server_address: "jellyfish:5002",
server_api_token: "development"
server_api_token: "development",
divo: "docker-compose-integration.yaml",
divo_wait: [dwell: 1_500, max_tries: 50]
4 changes: 3 additions & 1 deletion config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ import Config

config :jellyfish_server_sdk,
server_address: "localhost:5002",
server_api_token: "development"
server_api_token: "development",
divo: "docker-compose-dev.yaml",
divo_wait: [dwell: 1_500, max_tries: 50]
2 changes: 2 additions & 0 deletions docker-compose-integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ services:
extends:
file: docker-compose-dev.yaml
service: jellyfish
environment:
- VIRTUAL_HOST=jellyfish
networks:
- network

Expand Down
20 changes: 20 additions & 0 deletions lib/jellyfish/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,24 @@ defmodule Jellyfish.Client do

%__MODULE__{http_client: http_client}
end

@spec update_address(t(), String.t()) :: t()
def update_address(client, new_address) do
Utils.check_prefixes(new_address)

Map.update!(
client,
:http_client,
&Map.update!(&1, :pre, fn pre_list ->
Enum.map(pre_list, fn
{Tesla.Middleware.BaseUrl, :call, [old_address]} ->
[protocol | _] = String.split(old_address, ":")
{Tesla.Middleware.BaseUrl, :call, ["#{protocol}://#{new_address}"]}

other ->
other
end)
end)
)
end
end
43 changes: 24 additions & 19 deletions lib/jellyfish/room.ex
Comment thread
sgfn marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,27 @@ defmodule Jellyfish.Room do

## Examples
```
iex> {:ok, room} = Jellyfish.Room.create(client, max_peers: 10)
{:ok,
%Jellyfish.Room{
components: [],
config: %{max_peers: 10},
id: "d3af274a-c975-4876-9e1c-4714da0249b8",
peers: []
}}

iex> {:ok, peer, peer_token} = Jellyfish.Room.add_peer(client, room.id, Jellyfish.Peer.WebRTC)
{:ok,
%Jellyfish.Peer{
id: "5a731f2e-f49f-4d58-8f64-16a5c09b520e",
status: :disconnected,
type: Jellyfish.Peer.WebRTC
}, "3LTQ3ZDEtYTRjNy0yZDQyZjU1MDAxY2FkAAdyb29tX2lkbQAAACQ0M"}

iex> client = Jellyfish.Client.new()
iex> assert {:ok, %Jellyfish.Room{
...> components: [],
...> config: %{max_peers: 10, video_codec: nil},
...> peers: []
...> } = room, _jellyfish_address} = Jellyfish.Room.create(client, max_peers: 10)
iex> room == %Jellyfish.Room{
...> id: room.id,
...> components: [],
...> config: %{max_peers: 10, video_codec: nil},
...> peers: []}
true
iex> assert {:ok,%Jellyfish.Peer{
...> status: :disconnected,
...> type: Jellyfish.Peer.WebRTC
...> } = peer, _peer_token} = Jellyfish.Room.add_peer(client, room.id, Jellyfish.Peer.WebRTC)
iex> %Jellyfish.Peer{
...> id: peer.id,
...> status: :disconnected,
...> type: Jellyfish.Peer.WebRTC} == peer
true
iex> :ok = Jellyfish.Room.delete(client, room.id)
:ok
```
Comment on lines 5 to 30
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doctest is a nice addition, but isn't it a bit too complicated to be used as an example? I think the previous version was more readable

Copy link
Copy Markdown
Contributor Author

@Rados13 Rados13 Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, the previous version was more readable, but the doctest prevents the example from being outdated. I think an outdated example is useless so I would leave doctest, but I can make it less complicated by removing additional checks. It would look like this

Suggested change
## Examples
```
iex> {:ok, room} = Jellyfish.Room.create(client, max_peers: 10)
{:ok,
%Jellyfish.Room{
components: [],
config: %{max_peers: 10},
id: "d3af274a-c975-4876-9e1c-4714da0249b8",
peers: []
}}
iex> {:ok, peer, peer_token} = Jellyfish.Room.add_peer(client, room.id, Jellyfish.Peer.WebRTC)
{:ok,
%Jellyfish.Peer{
id: "5a731f2e-f49f-4d58-8f64-16a5c09b520e",
status: :disconnected,
type: Jellyfish.Peer.WebRTC
}, "3LTQ3ZDEtYTRjNy0yZDQyZjU1MDAxY2FkAAdyb29tX2lkbQAAACQ0M"}
iex> client = Jellyfish.Client.new()
iex> assert {:ok, %Jellyfish.Room{
...> components: [],
...> config: %{max_peers: 10, video_codec: nil},
...> peers: []
...> } = room, _jellyfish_address} = Jellyfish.Room.create(client, max_peers: 10)
iex> room == %Jellyfish.Room{
...> id: room.id,
...> components: [],
...> config: %{max_peers: 10, video_codec: nil},
...> peers: []}
true
iex> assert {:ok,%Jellyfish.Peer{
...> status: :disconnected,
...> type: Jellyfish.Peer.WebRTC
...> } = peer, _peer_token} = Jellyfish.Room.add_peer(client, room.id, Jellyfish.Peer.WebRTC)
iex> %Jellyfish.Peer{
...> id: peer.id,
...> status: :disconnected,
...> type: Jellyfish.Peer.WebRTC} == peer
true
iex> :ok = Jellyfish.Room.delete(client, room.id)
:ok
```
iex> client = Jellyfish.Client.new()
iex> assert {:ok, %Jellyfish.Room{
...> components: [],
...> config: %{max_peers: 10, video_codec: nil},
...> peers: []
...> } = room, _jellyfish_address} = Jellyfish.Room.create(client, max_peers: 10)
iex> assert {:ok,%Jellyfish.Peer{
...> status: :disconnected,
...> type: Jellyfish.Peer.WebRTC
...> } = peer, _peer_token} = Jellyfish.Room.add_peer(client, room.id, Jellyfish.Peer.WebRTC)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, it's fine either way. You decide

Expand Down Expand Up @@ -103,7 +107,7 @@ defmodule Jellyfish.Room do
@doc """
Create a room.
"""
@spec create(Client.t(), options()) :: {:ok, t()} | {:error, atom() | String.t()}
@spec create(Client.t(), options()) :: {:ok, t(), String.t()} | {:error, atom() | String.t()}
def create(client, opts \\ []) do
with {:ok, %Env{status: 201, body: body}} <-
Tesla.post(
Expand All @@ -115,8 +119,9 @@ defmodule Jellyfish.Room do
}
),
{:ok, data} <- Map.fetch(body, "data"),
{:ok, jellyfish_address} <- Map.fetch(body, "jellyfish_address"),
result <- from_json(data) do
{:ok, result}
{:ok, result, jellyfish_address}
else
:error -> raise StructureError
error -> handle_response_error(error)
Expand Down
7 changes: 6 additions & 1 deletion lib/jellyfish/utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ defmodule Jellyfish.Utils do
secure? =
Keyword.get(opts, :secure?, Application.get_env(:jellyfish_server_sdk, :secure?, false))

if String.starts_with?(server_address, @protocol_prefixes), do: raise(ProtocolPrefixError)
check_prefixes(server_address)

{server_address, server_api_token, secure?}
end

@spec check_prefixes(String.t()) :: nil
def check_prefixes(server_address) do
if String.starts_with?(server_address, @protocol_prefixes), do: raise(ProtocolPrefixError)
end
end
38 changes: 38 additions & 0 deletions test/jellyfish/client_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,42 @@ defmodule Jellyfish.ClientTest do
)
end
end

test "update client address" do
client =
Client.new(
server_address: @server_address,
server_api_token: @server_api_token,
secure?: true
)

addres_with_prefix = "https://#{@server_address}"

assert %Client{
http_client: %Tesla.Client{
adapter: {Tesla.Adapter.Mint, :call, [[]]},
pre: [
{Tesla.Middleware.BaseUrl, :call, [^addres_with_prefix]},
{Tesla.Middleware.BearerAuth, :call, [[token: @server_api_token]]},
{Tesla.Middleware.JSON, :call, [[]]}
]
}
} = client
Comment on lines +97 to +105
Copy link
Copy Markdown
Collaborator

@LVala LVala Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that's what I did in other tests (couldn't really think of other way), but asserting on Tesla's internal structures is a code smell. I would at least match on the least amount of fields (the JSON or adapter: parts are really not necessary here). Don't repeat my mistakes 😃, for now you can leave as it is to take care of the whole module some other time, up to you.


new_address = "jellyfish2:5005"
addres_with_prefix = "https://#{new_address}"

client = Client.update_address(client, new_address)

assert %Client{
http_client: %Tesla.Client{
adapter: {Tesla.Adapter.Mint, :call, [[]]},
pre: [
{Tesla.Middleware.BaseUrl, :call, [^addres_with_prefix]},
{Tesla.Middleware.BearerAuth, :call, [[token: @server_api_token]]},
{Tesla.Middleware.JSON, :call, [[]]}
]
}
} = client
end
end
14 changes: 6 additions & 8 deletions test/jellyfish/notifier_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ defmodule Jellyfish.NotifierTest do
end

test "when room gets created and then deleted", %{client: client} do
{:ok, %Jellyfish.Room{id: room_id}} =
{:ok, %Jellyfish.Room{id: room_id}, _jellyfish_address} =
Room.create(client, max_peers: @max_peers, video_codec: @video_codec)

assert_receive {:jellyfish, %RoomCreated{room_id: ^room_id}}
Expand All @@ -53,14 +53,12 @@ defmodule Jellyfish.NotifierTest do
end

test "when peer connects and then disconnects", %{client: client} do
{:ok, %Jellyfish.Room{id: room_id}} =
{:ok, %Jellyfish.Room{id: room_id}, jellyfish_address} =
Room.create(client, max_peers: @max_peers, video_codec: @video_codec)

{:ok, %Jellyfish.Peer{id: peer_id}, peer_token} = Room.add_peer(client, room_id, @peer_opts)

url = Application.fetch_env!(:jellyfish_server_sdk, :server_address)

{:ok, peer_ws} = WS.start_link("ws://#{url}/socket/peer/websocket")
{:ok, peer_ws} = WS.start_link("ws://#{jellyfish_address}/socket/peer/websocket")

auth_request = %PeerMessage{content: {:auth_request, %AuthRequest{token: peer_token}}}

Expand All @@ -84,12 +82,12 @@ defmodule Jellyfish.NotifierTest do
end

test "with one peer", %{client: client} do
{:ok, %Jellyfish.Room{id: room_id}} = Room.create(client, max_peers: @max_peers)
{:ok, %Jellyfish.Room{id: room_id}, jellyfish_address} =
Room.create(client, max_peers: @max_peers)

{:ok, %Jellyfish.Peer{id: peer_id}, peer_token} = Room.add_peer(client, room_id, @peer_opts)

url = Application.fetch_env!(:jellyfish_server_sdk, :server_address)
{:ok, peer_ws} = WS.start_link("ws://#{url}/socket/peer/websocket")
{:ok, peer_ws} = WS.start_link("ws://#{jellyfish_address}/socket/peer/websocket")

auth_request = %PeerMessage{content: {:auth_request, %AuthRequest{token: peer_token}}}
:ok = WS.send_frame(peer_ws, auth_request)
Expand Down
25 changes: 17 additions & 8 deletions test/jellyfish/room_test.exs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
defmodule Jellyfish.RoomTest do
use ExUnit.Case

doctest Jellyfish.Room
alias Jellyfish.{Client, Component, Peer, Room}

@server_api_token "development"
Expand Down Expand Up @@ -42,14 +42,18 @@ defmodule Jellyfish.RoomTest do

describe "auth" do
test "correct token", %{client: client} do
assert {:ok, room} = Room.create(client, max_peers: @max_peers, video_codec: @video_codec)
assert {:ok, room, jellyfish_address} =
Room.create(client, max_peers: @max_peers, video_codec: @video_codec)

assert %Jellyfish.Room{
components: [],
config: %{max_peers: 10, video_codec: @video_codec},
id: _id,
peers: []
} = room

server_address = Application.fetch_env!(:jellyfish_server_sdk, :server_address)
assert ^server_address = jellyfish_address
end

test "invalid token" do
Expand All @@ -60,14 +64,17 @@ defmodule Jellyfish.RoomTest do

describe "Room.create/2" do
test "when request is valid", %{client: client} do
assert {:ok, room} = Room.create(client, max_peers: @max_peers)
assert {:ok, room, jellyfish_address} = Room.create(client, max_peers: @max_peers)

assert %Jellyfish.Room{
components: [],
config: %{max_peers: 10},
id: _id,
peers: []
} = room

server_address = Application.fetch_env!(:jellyfish_server_sdk, :server_address)
assert ^server_address = jellyfish_address
end

test "when request is invalid, max peers", %{client: client} do
Expand Down Expand Up @@ -128,17 +135,19 @@ defmodule Jellyfish.RoomTest do
describe "Room.add_component/3" do
setup [:create_room]

test "when request is valid", %{client: client, room_id: room_id} do
test "when request is valid with opts", %{client: client, room_id: room_id} do
assert {:ok, component} = Room.add_component(client, room_id, @hls_component_opts)
assert %Component{type: Component.HLS, metadata: %{playable: false}} = component

assert {:ok, component} = Room.add_component(client, room_id, @hls_component_opts_module)
assert %Component{type: Component.HLS, metadata: %{playable: false}} = component

assert {:ok, component} = Room.add_component(client, room_id, @rtsp_component_opts)
assert %Component{type: Component.RTSP, metadata: %{}} = component
end

test "when request is valid with opts module", %{client: client, room_id: room_id} do
assert {:ok, component} = Room.add_component(client, room_id, @hls_component_opts_module)
assert %Component{type: Component.HLS, metadata: %{playable: false}} = component
end

test "when request is invalid", %{client: client} do
assert_raise FunctionClauseError, fn ->
Room.add_component(client, @room_id, %InvalidComponentOpts{})
Expand Down Expand Up @@ -199,7 +208,7 @@ defmodule Jellyfish.RoomTest do
end

defp create_room(state) do
assert {:ok, %Jellyfish.Room{id: id}} =
assert {:ok, %Jellyfish.Room{id: id}, _jellyfish_address} =
Room.create(state.client, max_peers: @max_peers, video_codec: @video_codec)

%{room_id: id}
Expand Down