Skip to content

Commit

Permalink
Revert feat: switch to in-memory Alerts cache instead of V3 API (#2122)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sloanelybutsurely
sloanelybutsurely committed Aug 23, 2024
1 parent 6e69fdc commit 0935a17
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 62 deletions.
55 changes: 12 additions & 43 deletions lib/screens/alerts/alert.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,18 +175,8 @@ defmodule Screens.Alerts.Alert do
]

@spec fetch(keyword()) :: {:ok, list(t())} | :error
def fetch(opts \\ [], get_json_fn \\ &V3Api.get_json/2, get_all_alerts \\ &Cache.all/0) do
Screens.Telemetry.span_with_stop_meta([:screens, :alerts, :alert, :fetch], fn ->
if supported_by_cache?(opts) do
{fetch_from_cache(opts, get_all_alerts), %{from: :cache}}
else
{fetch_from_v3_api(opts, get_json_fn), %{from: :v3_api}}
end
end)
end

def fetch_from_v3_api(opts \\ [], get_json_fn \\ &V3Api.get_json/2) do
Screens.Telemetry.span([:screens, :alerts, :alert, :fetch_from_v3_api], fn ->
def fetch(opts \\ [], get_json_fn \\ &V3Api.get_json/2) do
Screens.Telemetry.span([:screens, :alerts, :alert, :fetch], fn ->
params =
opts
|> Enum.flat_map(&format_query_param/1)
Expand All @@ -203,17 +193,15 @@ defmodule Screens.Alerts.Alert do
end

def fetch_from_cache(filters \\ [], get_all_alerts \\ &Cache.all/0) do
Screens.Telemetry.span([:screens, :alerts, :alert, :fetch_from_cache], fn ->
alerts = get_all_alerts.()
alerts = get_all_alerts.()

filters =
filters
|> Enum.map(&format_cache_filter/1)
|> Enum.reject(&is_nil/1)
|> Enum.into(%{})
filters =
filters
|> Enum.map(&format_cache_filter/1)
|> Enum.reject(&is_nil/1)
|> Enum.into(%{})

{:ok, Screens.Alerts.Cache.Filter.filter_by(alerts, filters)}
end)
{:ok, Screens.Alerts.Cache.Filter.filter_by(alerts, filters)}
end

def fetch_from_cache_or_empty_list(filters \\ [], get_all_alerts \\ &Cache.all/0) do
Expand Down Expand Up @@ -244,25 +232,6 @@ defmodule Screens.Alerts.Alert do

defp format_cache_filter(filter), do: filter

defp supported_by_cache?(opts) do
Enum.all?(opts, fn {key, _} -> supported_cache_filter?(key) end)
end

for supported_filter <- ~w[routes
route_id
route_ids
stops
stop_id
stop_ids
route_type
route_types
direction_id
activities]a do
defp supported_cache_filter?(unquote(supported_filter)), do: true
end

defp supported_cache_filter?(_), do: false

@doc """
Convenience for cases when it's safe to treat an API alert data outage
as if there simply aren't any alerts for the given parameters.
Expand Down Expand Up @@ -297,10 +266,10 @@ defmodule Screens.Alerts.Alert do
https://app.asana.com/0/0/1200476247539238/f
"""
@spec fetch_by_stop_and_route(list(Stop.id()), list(Route.id())) :: {:ok, list(t())} | :error
def fetch_by_stop_and_route(stop_ids, route_ids, get_all_alerts \\ &Cache.all/0) do
def fetch_by_stop_and_route(stop_ids, route_ids, get_json_fn \\ &V3Api.get_json/2) do
with {:ok, stop_based_alerts} <-
fetch_from_cache([stop_ids: stop_ids, route_ids: route_ids], get_all_alerts),
{:ok, route_based_alerts} <- fetch_from_cache([route_ids: route_ids], get_all_alerts) do
fetch([stop_ids: stop_ids, route_ids: route_ids], get_json_fn),
{:ok, route_based_alerts} <- fetch([route_ids: route_ids], get_json_fn) do
merged_alerts =
[stop_based_alerts, route_based_alerts]
|> Enum.concat()
Expand Down
89 changes: 70 additions & 19 deletions test/screens/alerts/alert_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,95 @@ defmodule Screens.Alerts.AlertTest do

setup :verify_on_exit!

defp ie(opts) do
defp alert_json(id) do
%{
stop: opts[:stop],
route: opts[:route],
route_type: opts[:route_type],
activities: opts[:activities] || ~w[BOARD EXIT RIDE]
"id" => id,
"attributes" => %{
"active_period" => [],
"created_at" => nil,
"updated_at" => nil,
"cause" => nil,
"effect" => nil,
"header" => nil,
"informed_entity" => [],
"lifecycle" => nil,
"severity" => nil,
"timeframe" => nil,
"url" => nil,
"description" => nil
}
}
end

describe "fetch_by_stop_and_route/3" do
test "returns {:ok, merged_alerts} if fetch function succeeds in both cases" do
stub(Route.Mock, :by_id, fn _id -> nil end)
stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end)

get_all_alerts_fn = fn ->
[
%Alert{id: "1", informed_entities: [ie(stop: "1265")]},
%Alert{id: "2", informed_entities: [ie(stop: "1266")]},
%Alert{id: "3", informed_entities: [ie(stop: "10413", route: "22")]},
%Alert{id: "4", informed_entities: [ie(route: "29")]},
%Alert{id: "5", informed_entities: [ie(route: "44")]}
]
end
setup do
stop_based_alerts = [alert_json("1"), alert_json("2"), alert_json("3")]
route_based_alerts = [alert_json("4"), alert_json("3"), alert_json("5")]

stop_ids = ~w[1265 1266 10413 11413 17411]
route_ids = ~w[22 29 44]

stop_ids_param = Enum.join(stop_ids, ",")
route_ids_param = Enum.join(route_ids, ",")

%{
stop_ids: ~w[1265 1266 10413 11413 17411],
route_ids: ~w[22 29 44],
get_json_fn: fn
_, %{"filter[stop]" => ^stop_ids_param, "filter[route]" => ^route_ids_param} ->
{:ok, %{"data" => stop_based_alerts}}

_, %{"filter[route]" => ^route_ids_param} ->
{:ok, %{"data" => route_based_alerts}}
end,
x_get_json_fn1: fn
_, %{"filter[stop]" => ^stop_ids_param, "filter[route]" => ^route_ids_param} -> :error
_, %{"filter[route]" => ^route_ids_param} -> {:ok, %{"data" => route_based_alerts}}
end,
x_get_json_fn2: fn
_, %{"filter[stop]" => ^stop_ids_param, "filter[route]" => ^route_ids_param} ->
{:ok, %{"data" => stop_based_alerts}}

_, %{"filter[route]" => ^route_ids_param} ->
:error
end
}
end

test "returns {:ok, merged_alerts} if fetch function succeeds in both cases", context do
%{
stop_ids: stop_ids,
route_ids: route_ids,
get_json_fn: get_json_fn
} = context

assert {:ok,
[
%Alert{id: "1"},
%Alert{id: "2"},
%Alert{id: "3"},
%Alert{id: "4"},
%Alert{id: "5"}
]} = Alert.fetch_by_stop_and_route(stop_ids, route_ids, get_all_alerts_fn)
]} = Alert.fetch_by_stop_and_route(stop_ids, route_ids, get_json_fn)
end

test "returns :error if fetch function returns :error", context do
%{
stop_ids: stop_ids,
route_ids: route_ids,
x_get_json_fn1: x_get_json_fn1,
x_get_json_fn2: x_get_json_fn2
} = context

assert :error == Alert.fetch_by_stop_and_route(stop_ids, route_ids, x_get_json_fn1)
assert :error == Alert.fetch_by_stop_and_route(stop_ids, route_ids, x_get_json_fn2)
end
end

defp ie(opts) do
%{stop: opts[:stop], route: opts[:route], route_type: opts[:route_type]}
end

describe "informed_entities/1" do
test "returns informed entities list from the widget's alert" do
ies = [ie(stop: "123"), ie(stop: "1129", route: "39")]
Expand Down

0 comments on commit 0935a17

Please sign in to comment.