Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove unused routes-at-stop agent #2116

Merged
merged 1 commit into from
Jul 31, 2024
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
1 change: 0 additions & 1 deletion lib/screens/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ defmodule Screens.Application do
:hackney_pool.child_spec(:ex_aws_pool, []),
:hackney_pool.child_spec(:blue_bikes_pool, []),
:hackney_pool.child_spec(:api_v3_pool, max_connections: 100),
{Screens.Stops.StationsWithRoutesAgent, %{}},
# Turning this off because it's not in use, and the process is failing
# {Screens.BlueBikes.State, name: Screens.BlueBikes.State},
# Task supervisor for ScreensByAlert async updates
Expand Down
54 changes: 39 additions & 15 deletions lib/screens/routes/route.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
defmodule Screens.Routes.Route do
@moduledoc false

require Logger

alias Screens.Routes.Parser
alias Screens.RouteType
alias Screens.Stops.Stop
alias Screens.V3Api
Expand Down Expand Up @@ -39,7 +42,7 @@ defmodule Screens.Routes.Route do
@spec by_id(id()) :: {:ok, t()} | :error
def by_id(route_id) do
case V3Api.get_json("routes/" <> route_id) do
{:ok, %{"data" => data}} -> {:ok, Screens.Routes.Parser.parse_route(data)}
{:ok, %{"data" => data}} -> {:ok, Parser.parse_route(data)}
_ -> :error
end
end
Expand All @@ -53,41 +56,62 @@ defmodule Screens.Routes.Route do
|> Enum.into(%{})

case get_json_fn.("routes/", params) do
{:ok, %{"data" => data}} -> {:ok, Enum.map(data, &Screens.Routes.Parser.parse_route/1)}
{:ok, %{"data" => data}} -> {:ok, Enum.map(data, &Parser.parse_route/1)}
_ -> :error
end
end

@doc "Fetches routes that serve the given stop."
@spec serving_stop(Stop.id()) :: {:ok, t()} | :error
def serving_stop(
stop_id,
get_json_fn \\ &V3Api.get_json/2,
attempts_left \\ 3
)

def serving_stop(_stop_id, _get_json_fn, 0), do: :error

def serving_stop(
stop_id,
get_json_fn,
attempts_left
) do
case get_json_fn.("routes", %{"filter[stop]" => stop_id}) do
{:ok, %{"data" => []}, _} ->
Logger.warning("Route.serving_stop empty_retry attempts_left=#{attempts_left - 1}")
serving_stop(stop_id, get_json_fn, attempts_left - 1)

{:ok, %{"data" => data}} ->
{:ok, Enum.map(data, fn route -> Parser.parse_route(route) end)}

_ ->
:error
end
end

@doc """
Fetches routes that serve the given stop. `now` is used to determine whether
each route is actively running on the current day.
Similar to `serving_stop` but also determines whether each route has any scheduled service at
the given stop on the current day. Only route IDs and the `active?` flag are returned.
"""
@spec fetch_routes_by_stop(String.t()) ::
@spec serving_stop_with_active(Stop.id()) ::
{:ok, list(%{route_id: id(), active?: boolean()})} | :error
def fetch_routes_by_stop(
def serving_stop_with_active(
stop_id,
now \\ DateTime.utc_now(),
type_filters \\ [],
get_json_fn \\ &V3Api.get_json/2,
fetch_routes_fn \\ &fetch_routes/3
) do
Screens.Telemetry.span(
~w[screens routes route fetch_routes_by_stop]a,
~w[screens routes route serving_stop_with_active]a,
%{stop_id: stop_id, type_filters: type_filters},
fn ->
with {:ok, routes} <- fetch_routes_fn.(stop_id, get_json_fn, type_filters),
{:ok, active_route_ids} <- fetch_active_route_ids(stop_id, now, get_json_fn) do
active_set = MapSet.new(active_route_ids)

routes_at_stop =
Enum.map(
routes,
&(&1
|> Map.from_struct()
|> Map.put(:active?, MapSet.member?(active_set, &1.id))
|> Map.put(:route_id, &1.id)
|> Map.delete(:id))
)
Enum.map(routes, &%{route_id: &1.id, active?: MapSet.member?(active_set, &1.id)})

{:ok, routes_at_stop}
else
Expand Down
16 changes: 0 additions & 16 deletions lib/screens/stops/stations_with_routes_agent.ex

This file was deleted.

74 changes: 3 additions & 71 deletions lib/screens/stops/stop.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ defmodule Screens.Stops.Stop do

alias Screens.LocationContext
alias Screens.RoutePatterns.RoutePattern
alias Screens.{Routes, Stops}
alias Screens.Routes.Route
alias Screens.RouteType
alias Screens.Stops.StationsWithRoutesAgent
alias Screens.Stops
alias Screens.Util
alias Screens.V3Api
alias ScreensConfig.V2.{BusEink, BusShelter, Dup, GlEink, PreFare, Triptych}
Expand Down Expand Up @@ -271,74 +270,6 @@ defmodule Screens.Stops.Stop do
end
end

def fetch_routes_serving_stop(
station_id,
get_json_fn \\ &V3Api.get_json/2,
attempts_left \\ 3
)

def fetch_routes_serving_stop(_station_id, _get_json_fn, 0), do: :bad_response

def fetch_routes_serving_stop(
station_id,
get_json_fn,
attempts_left
) do
case get_json_fn.("routes", %{"filter[stop]" => station_id}) do
{:ok, %{"data" => []}, _} ->
fetch_routes_serving_stop(station_id, get_json_fn, attempts_left - 1)

{:ok, %{"data" => data}} ->
{:ok, Enum.map(data, fn route -> Routes.Parser.parse_route(route) end)}

_ ->
:error
end
end

# Returns a list of Route structs that serve the provided ID
@spec create_station_with_routes_map(String.t()) :: list(Routes.Route.t())
def create_station_with_routes_map(station_id) do
case StationsWithRoutesAgent.get(station_id) do
nil ->
get_routes_serving_stop(station_id)

routes ->
get_routes_serving_stop(station_id, routes)
end
end

defp get_routes_serving_stop(station_id, default_routes \\ []) do
case fetch_routes_serving_stop(station_id) do
{:ok, new_routes} ->
new_routes

:bad_response ->
Logger.error(
"[create_station_with_routes_map no routes] Received an empty list from API: stop_id=#{station_id}"
)

default_routes

:error ->
Logger.error(
"[create_station_with_routes_map fetch error] Received an error from API: stop_id=#{station_id}"
)

default_routes
end
end

def get_routes_serving_stop_ids(stop_ids) do
stop_ids
|> Enum.flat_map(fn stop_id ->
stop_id
|> create_station_with_routes_map()
|> Enum.map(& &1.id)
end)
|> Enum.uniq()
end

def fetch_stop_name(stop_id) do
Screens.Telemetry.span(~w[screens stops stop fetch_stop_name]a, %{stop_id: stop_id}, fn ->
case Screens.V3Api.get_json("stops", %{"filter[id]" => stop_id}) do
Expand Down Expand Up @@ -479,7 +410,8 @@ defmodule Screens.Stops.Stop do
%{app: app, stop_id: stop_id},
fn ->
with alert_route_types <- get_route_type_filter(app, stop_id),
{:ok, routes_at_stop} <- Route.fetch_routes_by_stop(stop_id, now, alert_route_types),
{:ok, routes_at_stop} <-
Route.serving_stop_with_active(stop_id, now, alert_route_types),
{:ok, tagged_stop_sequences} <-
fetch_tagged_stop_sequences_by_app(app, stop_id, routes_at_stop) do
stop_name = fetch_stop_name(stop_id)
Expand Down
18 changes: 9 additions & 9 deletions lib/screens/v2/candidate_generator/dup/departures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
fetch_departures_fn \\ &Departure.fetch/2,
fetch_alerts_fn \\ &Alert.fetch_or_empty_list/1,
fetch_schedules_fn \\ &Screens.Schedules.Schedule.fetch/2,
create_station_with_routes_map_fn \\ &Screens.Stops.Stop.create_station_with_routes_map/1,
fetch_routes_serving_stop_fn \\ &Screens.Routes.Route.serving_stop/1,
fetch_vehicles_fn \\ &Screens.Vehicles.Vehicle.by_route_and_direction/2
) do
primary_departures_instances =
primary_sections
|> get_sections_data(
fetch_departures_fn,
fetch_alerts_fn,
create_station_with_routes_map_fn,
fetch_routes_serving_stop_fn,
now
)
|> sections_data_to_departure_instances(
Expand Down Expand Up @@ -64,7 +64,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
|> get_sections_data(
fetch_departures_fn,
fetch_alerts_fn,
create_station_with_routes_map_fn,
fetch_routes_serving_stop_fn,
now
)
|> sections_data_to_departure_instances(
Expand Down Expand Up @@ -208,7 +208,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
sections,
fetch_departures_fn,
fetch_alerts_fn,
create_station_with_routes_map_fn,
fetch_routes_serving_stop_fn,
now
) do
Screens.Telemetry.span(
Expand All @@ -222,7 +222,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
&1,
fetch_departures_fn,
fetch_alerts_fn,
create_station_with_routes_map_fn,
fetch_routes_serving_stop_fn,
now,
ctx
),
Expand All @@ -249,15 +249,15 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
section,
fetch_departures_fn,
fetch_alerts_fn,
create_station_with_routes_map_fn,
fetch_routes_serving_stop_fn,
now,
ctx
) do
Screens.Telemetry.span(
[:screens, :v2, :candidate_generator, :dup, :departures, :get_section_data],
ctx,
fn ->
routes = get_routes_serving_section(params, create_station_with_routes_map_fn)
routes = get_routes_serving_section(params, fetch_routes_serving_stop_fn)
# DUP sections will always show no more than one mode.
# For subway, each route will have its own section.
# If the stop is served by two different subway/light rail routes, route_ids must be populated for each section
Expand Down Expand Up @@ -634,11 +634,11 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do

defp get_routes_serving_section(
%{route_ids: route_ids, stop_ids: stop_ids},
create_station_with_routes_map_fn
fetch_routes_serving_stop_fn
) do
routes =
stop_ids
|> Enum.flat_map(&create_station_with_routes_map_fn.(&1))
|> Enum.flat_map(&fetch_routes_serving_stop_fn.(&1))
|> Enum.uniq()

if route_ids == [] do
Expand Down
10 changes: 5 additions & 5 deletions lib/screens/v2/candidate_generator/pre_fare.ex
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ defmodule Screens.V2.CandidateGenerator.PreFare do
def audio_only_instances(
widgets,
config,
fetch_routes_by_stop_fn \\ &Route.fetch_routes_by_stop/1
routes_serving_stop_fn \\ &Route.serving_stop/1
) do
[
fn -> content_summary_instances(widgets, config, fetch_routes_by_stop_fn) end,
fn -> content_summary_instances(widgets, config, routes_serving_stop_fn) end,
fn -> alerts_intro_instances(widgets, config) end,
fn -> alerts_outro_instances(widgets, config) end
]
Expand Down Expand Up @@ -133,14 +133,14 @@ defmodule Screens.V2.CandidateGenerator.PreFare do
[%ShuttleBusInfoWidget{screen: config, now: now}]
end

defp content_summary_instances(widgets, config, fetch_routes_by_stop_fn) do
defp content_summary_instances(widgets, config, routes_serving_stop_fn) do
config.app_params.content_summary.parent_station_id
|> fetch_routes_by_stop_fn.()
|> routes_serving_stop_fn.()
|> case do
{:ok, routes_at_station} ->
subway_lines_at_station =
routes_at_station
|> Enum.map(& &1.route_id)
|> Enum.map(& &1.id)
|> Enum.map(fn
"Red" -> :red
"Orange" -> :orange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ defmodule Screens.V2.CandidateGenerator.Widgets.ElevatorClosures do
@moduledoc false

alias Screens.Alerts.Alert
alias Screens.Routes.Route
alias Screens.Stops.Stop
alias Screens.V2.WidgetInstance.ElevatorStatus, as: ElevatorStatusWidget
alias ScreensConfig.Screen
Expand Down Expand Up @@ -51,7 +52,7 @@ defmodule Screens.V2.CandidateGenerator.Widgets.ElevatorClosures do
|> MapSet.new()
|> MapSet.put(home_parent_station_id)
|> Enum.map(fn station_id ->
{station_id, station_id |> Stop.create_station_with_routes_map() |> routes_to_icons()}
{station_id, station_id |> Route.serving_stop() |> routes_to_icons()}
end)
|> Enum.into(%{})
end
Expand Down
Loading
Loading