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.

Sign up for GitHub

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

Jump to bottom

feat: switch to in-memory Alerts cache instead of V3 API #2122

Merged
merged 1 commit into from
Aug 21, 2024
+62 −82
Merged

feat: switch to in-memory Alerts cache instead of V3 API #2122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 43 additions & 12 deletions lib/screens/alerts/alert.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,18 @@ defmodule Screens.Alerts.Alert do
]

@spec fetch(keyword()) :: {:ok, list(t())} | :error
def fetch(opts \\ [], get_json_fn \\ &V3Api.get_json/2) do
Screens.Telemetry.span([:screens, :alerts, :alert, :fetch], fn ->
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 ->
params =
opts
|> Enum.flat_map(&format_query_param/1)
Expand All @@ -193,15 +203,17 @@ defmodule Screens.Alerts.Alert do
end

def fetch_from_cache(filters \\ [], get_all_alerts \\ &Cache.all/0) do
alerts = get_all_alerts.()
Screens.Telemetry.span([:screens, :alerts, :alert, :fetch_from_cache], fn ->
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)}
{:ok, Screens.Alerts.Cache.Filter.filter_by(alerts, filters)}
end)
end

def fetch_from_cache_or_empty_list(filters \\ [], get_all_alerts \\ &Cache.all/0) do
Expand Down Expand Up @@ -232,6 +244,25 @@ 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
sloanelybutsurely marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -266,10 +297,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_json_fn \\ &V3Api.get_json/2) do
def fetch_by_stop_and_route(stop_ids, route_ids, get_all_alerts \\ &Cache.all/0) do
with {:ok, stop_based_alerts} <-
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
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
merged_alerts =
[stop_based_alerts, route_based_alerts]
|> Enum.concat()
Expand Down
89 changes: 19 additions & 70 deletions test/screens/alerts/alert_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,95 +8,44 @@ defmodule Screens.Alerts.AlertTest do

setup :verify_on_exit!

defp alert_json(id) do
defp ie(opts) do
%{
"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
}
stop: opts[:stop],
route: opts[:route],
route_type: opts[:route_type],
activities: opts[:activities] || ~w[BOARD EXIT RIDE]
}
end

describe "fetch_by_stop_and_route/3" do
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")]
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

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_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)
]} = Alert.fetch_by_stop_and_route(stop_ids, route_ids, get_all_alerts_fn)
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
Loading