diff --git a/lib/realtime/server.ex b/lib/realtime/server.ex index 7178b3ea64..706ac81e38 100644 --- a/lib/realtime/server.ex +++ b/lib/realtime/server.ex @@ -38,7 +38,6 @@ 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()} @@ -46,14 +45,12 @@ defmodule Realtime.Server do @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() } @@ -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 @@ -194,11 +176,6 @@ 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()] @@ -206,9 +183,6 @@ defmodule Realtime.Server do @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()] @@ -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()] @@ -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 @@ -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 @@ -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}) diff --git a/lib/skate_web/channels/vehicle_channel.ex b/lib/skate_web/channels/vehicle_channel.ex index 1d5be298d0..2bccbc1870 100644 --- a/lib/skate_web/channels/vehicle_channel.ex +++ b/lib/skate_web/channels/vehicle_channel.ex @@ -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 diff --git a/lib/skate_web/channels/vehicles_search_channel.ex b/lib/skate_web/channels/vehicles_search_channel.ex index c2b35a205a..a5ddc57fff 100644 --- a/lib/skate_web/channels/vehicles_search_channel.ex +++ b/lib/skate_web/channels/vehicles_search_channel.ex @@ -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) @@ -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 -> @@ -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) @@ -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 } ]) diff --git a/test/realtime/server_test.exs b/test/realtime/server_test.exs index 3ba831e919..20fee6a92a 100644 --- a/test/realtime/server_test.exs +++ b/test/realtime/server_test.exs @@ -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, @@ -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" ) @@ -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) @@ -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 ) @@ -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 @@ -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 ) @@ -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 @@ -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 diff --git a/test/skate_web/channels/vehicle_channel_test.exs b/test/skate_web/channels/vehicle_channel_test.exs index cda980fde4..730296eb42 100644 --- a/test/skate_web/channels/vehicle_channel_test.exs +++ b/test/skate_web/channels/vehicle_channel_test.exs @@ -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]})