diff --git a/lib/screens/alerts/alert.ex b/lib/screens/alerts/alert.ex index 17f1d1b20..9ce740f20 100644 --- a/lib/screens/alerts/alert.ex +++ b/lib/screens/alerts/alert.ex @@ -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) @@ -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 @@ -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 + 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. @@ -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() diff --git a/test/screens/alerts/alert_test.exs b/test/screens/alerts/alert_test.exs index 72c75023d..e74c95141 100644 --- a/test/screens/alerts/alert_test.exs +++ b/test/screens/alerts/alert_test.exs @@ -8,68 +8,33 @@ 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"}, @@ -77,26 +42,10 @@ defmodule Screens.Alerts.AlertTest do %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")]