Skip to content

Commit

Permalink
refactor: remove old logged out vehicle functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@lemald
lemald committed Jan 4, 2024
1 parent 5a812f8 commit ec0803d
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 149 deletions.
46 changes: 2 additions & 44 deletions lib/realtime/server.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,19 @@ defmodule Realtime.Server do
| {:search, search_params()}
| {:limited_search, limited_search_params()}
| {:vehicle, String.t()}
| {:vehicle_with_logged_out, String.t()}
| {:run_ids, [Run.id()]}
| {:block_ids, [Block.id()]}
| {:alerts, Route.id()}

@type search_params :: %{
:text => String.t(),
:property => search_property(),
optional(:include_logged_out_vehicles) => boolean(),
optional(:limit) => pos_integer()
}

@type limited_search_params :: %{
:text => String.t(),
:property => search_property(),
optional(:include_logged_out_vehicles) => boolean(),
optional(:limit) => pos_integer()
}

Expand Down Expand Up @@ -142,21 +139,6 @@ defmodule Realtime.Server do
)}
end

@spec subscribe_to_vehicle_with_logged_out(String.t(), GenServer.server()) ::
{subscription_key(),
[
VehicleOrGhost.t()
]}
def subscribe_to_vehicle_with_logged_out(vehicle_id, server \\ default_name()) do
subscription_key = {:vehicle_with_logged_out, vehicle_id}

{subscription_key,
subscribe(
server,
subscription_key
)}
end

@spec subscribe_to_run_ids([Run.id()], GenServer.server()) ::
{subscription_key(), [VehicleOrGhost.t()]}
def subscribe_to_run_ids(run_ids, server \\ default_name()) do
Expand Down Expand Up @@ -194,21 +176,13 @@ defmodule Realtime.Server do
lookup({ets, {:vehicle, vehicle_or_ghost_id}})
end

def peek_at_vehicle_by_id_with_logged_out(vehicle_or_ghost_id, server \\ default_name()) do
{_registry_key, ets} = GenServer.call(server, :subscription_info)
lookup({ets, {:vehicle_with_logged_out, vehicle_or_ghost_id}})
end

@spec subscribe(GenServer.server(), {:route_id, Route.id()}) :: [VehicleOrGhost.t()]
@spec subscribe(GenServer.server(), :all_shuttles) :: [Vehicle.t()]
@spec subscribe(GenServer.server(), :logged_in_vehicles) :: [Vehicle.t()]
@spec subscribe(GenServer.server(), {:search, search_params()}) :: [VehicleOrGhost.t()]
@spec subscribe(GenServer.server(), {:limited_search, search_params()}) ::
limited_search_result()
@spec subscribe(GenServer.server(), {:vehicle, String.t()}) :: [VehicleOrGhost.t()]
@spec subscribe(GenServer.server(), {:vehicle_with_logged_out, String.t()}) :: [
VehicleOrGhost.t()
]
@spec subscribe(GenServer.server(), :all_pull_backs) :: [VehicleOrGhost.t()]
@spec subscribe(GenServer.server(), {:run_ids, [Run.id()]}) :: [VehicleOrGhost.t()]
@spec subscribe(GenServer.server(), {:block_ids, [Block.id()]}) :: [VehicleOrGhost.t()]
Expand Down Expand Up @@ -258,7 +232,6 @@ defmodule Realtime.Server do
@spec lookup({:ets.tid(), {:search, search_params()}}) :: [VehicleOrGhost.t()]
@spec lookup({:ets.tid(), {:limited_search, search_params()}}) :: limited_search_result()
@spec lookup({:ets.tid(), {:vehicle, String.t()}}) :: [VehicleOrGhost.t()]
@spec lookup({:ets.tid(), {:vehicle_with_logged_out, String.t()}}) :: [VehicleOrGhost.t()]
@spec lookup({:ets.tid(), :all_pull_backs}) :: [Vehicle.t()]
@spec lookup({:ets.tid(), {:run_ids, [Run.id()]}}) :: [VehicleOrGhost.t()]
@spec lookup({:ets.tid(), {:block_ids, [Block.id()]}}) :: [VehicleOrGhost.t()]
Expand All @@ -267,11 +240,7 @@ defmodule Realtime.Server do
logged_in_vehicles = lookup({table, :logged_in_vehicles})

vehicles_to_search =
if Map.get(search_params, :include_logged_out_vehicles, false) do
logged_in_vehicles ++ lookup({table, :logged_out_vehicles})
else
logged_in_vehicles
end
logged_in_vehicles ++ lookup({table, :logged_out_vehicles})

VehicleOrGhost.find_by(vehicles_to_search, search_params)
end
Expand All @@ -280,11 +249,7 @@ defmodule Realtime.Server do
logged_in_vehicles = lookup({table, :logged_in_vehicles})

vehicles_to_search =
if Map.get(search_params, :include_logged_out_vehicles, false) do
logged_in_vehicles ++ lookup({table, :logged_out_vehicles})
else
logged_in_vehicles
end
logged_in_vehicles ++ lookup({table, :logged_out_vehicles})

VehicleOrGhost.take_limited_matches(vehicles_to_search, search_params)
end
Expand Down Expand Up @@ -318,13 +283,6 @@ defmodule Realtime.Server do
end

def lookup({table, {:vehicle, vehicle_or_ghost_id}}) do
{table, :logged_in_vehicles}
|> lookup()
|> Enum.find(&(&1.id == vehicle_or_ghost_id))
|> List.wrap()
end

def lookup({table, {:vehicle_with_logged_out, vehicle_or_ghost_id}}) do
logged_in_vehicles = lookup({table, :logged_in_vehicles})
logged_out_vehicles = lookup({table, :logged_out_vehicles})

Expand Down
20 changes: 4 additions & 16 deletions lib/skate_web/channels/vehicle_channel.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,14 @@ defmodule SkateWeb.VehicleChannel do
end

def join_authenticated("vehicle:id:" <> vehicle_or_ghost_id, _message, socket) do
%{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket)

user_in_test_group? = Skate.Settings.User.is_in_test_group(user_id, "map-beta")

vehicle_or_ghost =
if user_in_test_group? do
vehicle_or_ghost_id
|> Realtime.Server.peek_at_vehicle_by_id_with_logged_out()
|> List.first()
else
vehicle_or_ghost_id |> Realtime.Server.peek_at_vehicle_by_id() |> List.first()
end
vehicle_or_ghost_id
|> Realtime.Server.peek_at_vehicle_by_id()
|> List.first()

{lookup_key, _vehicle_or_ghost} =
if vehicle_or_ghost do
if user_in_test_group? do
Server.subscribe_to_vehicle_with_logged_out(vehicle_or_ghost.id)
else
Server.subscribe_to_vehicle(vehicle_or_ghost.id)
end
Server.subscribe_to_vehicle(vehicle_or_ghost.id)
else
{nil, nil}
end
Expand Down
8 changes: 2 additions & 6 deletions lib/skate_web/channels/vehicles_search_channel.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ defmodule SkateWeb.VehiclesSearchChannel do
)

username = username_from_socket!.(socket)
%{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket)

%{property: property, text: text} = search_params_from_subtopic(subtopic)

Expand All @@ -36,8 +35,7 @@ defmodule SkateWeb.VehiclesSearchChannel do
subscribe_args = %{
property: property,
text: text,
limit: limit,
include_logged_out_vehicles: Skate.Settings.User.is_in_test_group(user_id, "map-beta")
limit: limit
}

Logger.info(fn ->
Expand Down Expand Up @@ -65,7 +63,6 @@ defmodule SkateWeb.VehiclesSearchChannel do
&SkateWeb.AuthManager.username_from_socket!/1
)

%{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket)
username = username_from_socket!.(socket)

%{property: property, text: text} = search_params_from_subtopic(subtopic)
Expand All @@ -75,8 +72,7 @@ defmodule SkateWeb.VehiclesSearchChannel do
%{
property: property,
text: text,
limit: limit,
include_logged_out_vehicles: Skate.Settings.User.is_in_test_group(user_id, "map-beta")
limit: limit
}
])

Expand Down
105 changes: 28 additions & 77 deletions test/realtime/server_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ defmodule Realtime.ServerTest do

@logged_out_vehicle build(:vehicle,
route_id: "1",
id: "v1",
id: "v3",
label: "v1-label",
run_id: nil,
block_id: nil,
Expand All @@ -47,7 +47,7 @@ defmodule Realtime.ServerTest do
route_id: "1",
block_is_active: false,
block_id: "inactive_block",
id: "v2",
id: "v4",
label: "v2-label",
run_id: "456-7890"
)
Expand Down Expand Up @@ -313,49 +313,39 @@ defmodule Realtime.ServerTest do
setup do
{:ok, server_pid} = Server.start_link([])

:ok = Server.update_vehicles({@vehicles_by_route_id, [], []}, server_pid)
:ok = Server.update_vehicles({@vehicles_by_route_id, [], [@logged_out_vehicle]}, server_pid)

%{server_pid: server_pid}
end

test "clients get vehicle by ID upon subscribing", %{server_pid: pid} do
test "clients get vehicle by ID upon subscribing, logged in vehicle", %{server_pid: pid} do
{lookup_key, vehicle} = Server.subscribe_to_vehicle(@vehicle.id, pid)
assert vehicle == [@vehicle]
assert lookup_key == {:vehicle, @vehicle.id}
end

test "clients get updated data pushed to them", %{server_pid: pid} do
{lookup_key, _} = Server.subscribe_to_vehicle(@vehicle.id, pid)

Server.update_vehicles({@vehicles_by_route_id, [], []}, pid)

assert_receive {:new_realtime_data, ets}
assert Server.lookup({ets, lookup_key}) == [@vehicle]
end
end

describe "subscribe_to_vehicle_with_logged_out/2" do
setup do
{:ok, server_pid} = Server.start_link([])

:ok = Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, server_pid)

%{server_pid: server_pid}
end

test "clients get vehicle by ID upon subscribing", %{server_pid: pid} do
test "clients get vehicle by ID upon subscribing, logged out vehicle", %{server_pid: pid} do
{lookup_key, logged_out_vehicles} =
Server.subscribe_to_vehicle_with_logged_out(@logged_out_vehicle.id, pid)
Server.subscribe_to_vehicle(@logged_out_vehicle.id, pid)

assert logged_out_vehicles == [
@logged_out_vehicle
]

assert lookup_key == {:vehicle_with_logged_out, @logged_out_vehicle.id}
assert lookup_key == {:vehicle, @logged_out_vehicle.id}
end

test "clients get updated data pushed to them", %{server_pid: pid} do
{lookup_key, _} = Server.subscribe_to_vehicle_with_logged_out(@logged_out_vehicle.id, pid)
test "clients get updated data pushed to them, logged in vehicle", %{server_pid: pid} do
{lookup_key, _} = Server.subscribe_to_vehicle(@vehicle.id, pid)

Server.update_vehicles({@vehicles_by_route_id, [], []}, pid)

assert_receive {:new_realtime_data, ets}
assert Server.lookup({ets, lookup_key}) == [@vehicle]
end

test "clients get updated data pushed to them, logged out vehicle", %{server_pid: pid} do
{lookup_key, _} = Server.subscribe_to_vehicle(@logged_out_vehicle.id, pid)

Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, pid)

Expand Down Expand Up @@ -410,11 +400,11 @@ defmodule Realtime.ServerTest do
assert Server.lookup({ets, lookup_key}) == [@vehicle_on_inactive_block]
end

test "logged out vehicles are returned when include_logged_out_vehicles is true",
test "logged out vehicles are returned",
%{server_pid: pid} do
{lookup_key, _} =
Server.subscribe_to_search(
%{property: :vehicle, text: "123", include_logged_out_vehicles: true},
%{property: :vehicle, text: "123"},
pid
)

Expand All @@ -433,22 +423,6 @@ defmodule Realtime.ServerTest do
assert_receive {:new_realtime_data, ets}
assert Server.lookup({ets, lookup_key}) == [logged_in_vehicle, logged_out_vehicle]
end

test "logged out vehicles are not returned when include_logged_out_vehicles is not set",
%{server_pid: pid} do
{lookup_key, _} = Server.subscribe_to_search(%{property: :vehicle, text: "123"}, pid)

logged_in_vehicle =
build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id")

logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil)

Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}, pid)

assert_receive {:new_realtime_data, ets}

assert Server.lookup({ets, lookup_key}) == [logged_in_vehicle]
end
end

describe "subscribe_to_limited_search/2" do
Expand Down Expand Up @@ -504,11 +478,11 @@ defmodule Realtime.ServerTest do
Server.lookup({ets, lookup_key})
end

test "logged out vehicles are returned when include_logged_out_vehicles is true",
test "logged out vehicles are returned",
%{server_pid: pid} do
{lookup_key, _} =
Server.subscribe_to_limited_search(
%{property: :vehicle, text: "123", include_logged_out_vehicles: true, limit: 4},
%{property: :vehicle, text: "123", limit: 4},
pid
)

Expand All @@ -531,24 +505,6 @@ defmodule Realtime.ServerTest do
has_more_matches: false
} == Server.lookup({ets, lookup_key})
end

test "logged out vehicles are not returned when include_logged_out_vehicles is not set",
%{server_pid: pid} do
{lookup_key, _} =
Server.subscribe_to_limited_search(%{property: :vehicle, text: "123", limit: 5}, pid)

logged_in_vehicle =
build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id")

logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil)

Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}, pid)

assert_receive {:new_realtime_data, ets}

assert %{matching_vehicles: [logged_in_vehicle], has_more_matches: false} ==
Server.lookup({ets, lookup_key})
end
end

describe "update_limited_search_subscription/2" do
Expand Down Expand Up @@ -870,22 +826,17 @@ defmodule Realtime.ServerTest do
describe "peek_at_vehicles_by_id/2" do
test "looks up the vehicle or ghost with given ID" do
{:ok, server_pid} = Server.start_link([])
Server.update_vehicles({@vehicles_by_route_id, [@shuttle], []}, server_pid)

Server.update_vehicles(
{@vehicles_by_route_id, [@shuttle], [@logged_out_vehicle]},
server_pid
)

assert Server.peek_at_vehicle_by_id("no_such_vehicle", server_pid) == []
assert Server.peek_at_vehicle_by_id("v1", server_pid) == [@vehicle]
assert Server.peek_at_vehicle_by_id("g1", server_pid) == [@ghost]
end
end

describe "peek_at_vehicles_by_id_with_logged_out/2" do
test "looks up the vehicle or ghost with given ID" do
{:ok, server_pid} = Server.start_link([])
Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, server_pid)

assert Server.peek_at_vehicle_by_id_with_logged_out("no_such_vehicle", server_pid) == []

assert Server.peek_at_vehicle_by_id_with_logged_out(@logged_out_vehicle.id, server_pid) == [
assert Server.peek_at_vehicle_by_id(@logged_out_vehicle.id, server_pid) == [
@logged_out_vehicle
]
end
Expand Down
8 changes: 2 additions & 6 deletions test/skate_web/channels/vehicle_channel_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,9 @@ defmodule SkateWeb.VehicleChannelTest do
subscribe_and_join(socket, VehicleChannel, "vehicle:id:#{@vehicle.id}")
end

test "subscribes to the logged out vehicle for given ID when user is in the test group", %{
socket: socket,
user: user
test "subscribes to logged out vehicle for given ID", %{
socket: socket
} do
{:ok, test_group} = Skate.Settings.TestGroup.create("map-beta")
Skate.Settings.TestGroup.update(%{test_group | users: [user]})

logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil)

Realtime.Server.update_vehicles({%{}, [], [logged_out_vehicle]})
Expand Down

0 comments on commit ec0803d

Please sign in to comment.