refactor(Dotcom.PageController): avoid fetching and iterating all alerts ever

lib/alerts/cache/store.ex

@@ -18,6 +18,8 @@ defmodule Alerts.Cache.Store do
   """
 
   alias Alerts.{Alert, Priority}
+  alias Routes.Route
+  alias Stops.Stop
 
   use GenServer
 
@@ -84,6 +86,51 @@ defmodule Alerts.Cache.Store do
     :ets.select(:stop_id_to_alert_ids, keys)
   end
 
+  @doc """
+  Retrieves all active alerts for a given effect.
+  """
+  @spec active_alerts_for_effect(Alert.effect()) :: [Alert.t()]
+  def active_alerts_for_effect(effect) do
+    :ets.select(:alert_id_to_alert, [
+      {{:_, :"$1"}, [{:==, {:map_get, :effect, :"$1"}, effect}], [:"$1"]}
+    ])
+    |> Enum.filter(&Dotcom.Alerts.in_effect_now?/1)
+  end
+
+  @doc """
+  Retrieves all stop ids associated with alerts for a given effect.
+  """
+  @spec active_stop_ids_for_effect(Alert.effect()) :: [Stop.id_t()]
+  def active_stop_ids_for_effect(effect) do
+    effect
+    |> active_alerts_for_effect()
+    |> Enum.map(&{{:"$1", &1.id}, [], [:"$1"]})
+    |> then(fn stop_ids_from_alert_ids_match_spec ->
+      :ets.select(:stop_id_to_alert_ids, stop_ids_from_alert_ids_match_spec)
+    end)
+    |> Enum.reject(&Kernel.is_nil/1)
+    |> Enum.uniq()
+  end
+
+  @doc """
+  Retrieves all the alerts of a given priority for the given list of alert ids.
+  """
+  @spec route_ids_for_high_priority_alerts_for_route_types([Route.type_int()]) :: [Route.id_t()]
+  def route_ids_for_high_priority_alerts_for_route_types(route_types) do
+    route_types
+    |> Enum.map(&{{:_, &1, :"$1"}, [], [:"$1"]})
+    |> then(fn alert_ids_from_route_type_match_spec ->
+      :ets.select(:route_id_and_type_to_alert_ids, alert_ids_from_route_type_match_spec)
+    end)
+    |> priority_alerts_from_alert_ids(:high)
+    |> Enum.filter(&Dotcom.Alerts.in_effect_now?/1)
+    |> Enum.map(&{{:"$1", :_, &1.id}, [], [:"$1"]})
+    |> then(fn route_ids_from_alert_ids_match_spec ->
+      :ets.select(:route_id_and_type_to_alert_ids, route_ids_from_alert_ids_match_spec)
+    end)
+    |> Enum.uniq()
+  end
+
   @doc """
   Retrieves all the alerts of a given priority for the given list of alert ids.
   """

lib/dotcom/alerts.ex

@@ -263,33 +263,15 @@ defmodule Dotcom.Alerts do
     end)
   end
 
-  def routes_with_high_priority_alerts_by_mode(alerts) do
-    modes = [:subway, :bus, :commuter_rail, :ferry]
-    empty_by_mode = Map.new(modes, fn mode -> {mode, MapSet.new()} end)
-
-    route_ids_by_mode =
-      alerts
-      |> Enum.filter(&(Alerts.Priority.priority(&1) == :high))
-      |> Enum.reduce(empty_by_mode, fn alert, acc ->
-        route_ids = Alert.get_entity(alert, :route) |> MapSet.delete(nil)
-
-        alert
-        |> alert_route_type()
-        |> Enum.map(&Route.type_atom/1)
-        |> Enum.reduce(acc, fn mode, acc2 ->
-          Map.update!(acc2, mode, &MapSet.union(&1, route_ids))
-        end)
-      end)
-
-    Enum.map(modes, fn mode_key ->
-      route_ids =
-        route_ids_by_mode
-        |> Map.fetch!(mode_key)
-        |> MapSet.to_list()
-
+  def routes_with_high_priority_alerts_by_mode() do
+    [:subway, :bus, :commuter_rail, :ferry]
+    |> Enum.map(fn mode_key ->
       {mode_key,
-       get_many(route_ids, &@routes_repo_module.get/1)
-       |> Stream.filter(&match?({:ok, %Route{}}, &1))
+       mode_key
+       |> Route.types_for_mode()
+       |> Alerts.Cache.Store.route_ids_for_high_priority_alerts_for_route_types()
+       |> get_many(&@routes_repo_module.get/1)
+       |> Stream.filter(&match?({:ok, %Route{listed?: true}}, &1))
        |> Stream.map(fn {:ok, route} -> route end)
        |> Enum.sort_by(& &1.sort_order)}
     end)
@@ -301,34 +283,39 @@ defmodule Dotcom.Alerts do
     Task.async_stream(ids, func, max_concurrency: 8, on_timeout: :kill_task, ordered: false)
   end
 
-  def stops_with_access_alerts_by_effect(alerts) do
-    access_effects = Alerts.Accessibility.effect_types()
-    empty_by_effect = Map.new(access_effects, &{&1, MapSet.new()})
-
-    stop_ids_by_effect =
-      alerts
-      |> Enum.reduce(empty_by_effect, fn alert, acc ->
-        if Map.has_key?(acc, alert.effect) do
-          stop_id = alert_stop_ids(alert) |> List.last()
-          Map.update!(acc, alert.effect, &MapSet.put(&1, stop_id))
-        else
-          acc
-        end
-      end)
-
-    Enum.map(access_effects, fn effect ->
+  def stops_with_access_alerts_by_effect() do
+    Alerts.Accessibility.effect_types()
+    |> Enum.map(fn effect ->
       stops =
-        stop_ids_by_effect
-        |> Map.fetch!(effect)
-        |> get_many(&@stops_repo_module.get_parent/1)
-        |> Stream.filter(&match?({:ok, %Stop{}}, &1))
-        |> Stream.map(fn {:ok, stop} -> stop end)
-        |> Enum.sort_by(& &1.name)
+        effect
+        |> Alerts.Cache.Store.active_stop_ids_for_effect()
+        |> Enum.reduce(%{ids: [], stop_names: []}, &parent_stops/2)
+        |> Map.get(:stop_names)
+        |> Enum.sort_by(fn {_, name} -> name end)
 
       {effect, stops}
     end)
   end
 
+  # alerts reference both parent stop and all child stop ids, and we don't want
+  # to repeat fetches for the same parent stops, so we keep track of already
+  # evaluated stops while we traverse the list of IDs
+  defp parent_stops(stop_id, stop_data) do
+    if stop_id in stop_data.ids do
+      stop_data
+    else
+      case @stops_repo_module.get_parent(stop_id) do
+        %Stops.Stop{id: id, child_ids: child_ids, name: name} ->
+          new_ids = [id | child_ids]
+          all_ids = Enum.concat(new_ids, stop_data.ids)
+          %{ids: all_ids, stop_names: [{id, name} | stop_data.stop_names]}
+
+        _ ->
+          stop_data
+      end
+    end
+  end
+
   def alert_route_type(alert) do
     alert
     |> Alert.get_entity(:route_type)

lib/dotcom_web/controllers/page_controller.ex

@@ -27,7 +27,6 @@ defmodule DotcomWeb.PageController do
     {promoted, remainder} = whats_happening_items()
     banner = banner()
     date = conn.assigns.date
-    date_time = conn.assigns.date_time
 
     conn
     |> assign(
@@ -39,15 +38,6 @@ defmodule DotcomWeb.PageController do
     |> assign(:whats_happening_items, remainder)
     |> async_assign_default(:news, &news/0, [])
     |> async_assign_default(:photo, &photo/0)
-    |> async_assign_default(
-      :alerts,
-      fn ->
-        date_time
-        |> Alerts.Repo.all()
-        |> Enum.filter(&Alerts.Match.any_time_match?(&1, date_time))
-      end,
-      []
-    )
     |> async_assign_default(
       :event_teasers,
       fn -> CMS.Repo.next_n_event_teasers(date, 6) end,

lib/dotcom_web/templates/page/_alerts.html.heex

@@ -49,8 +49,8 @@
               </p>
             <% else %>
               <ul class="m-homepage__alerts-stops">
-                <%= for stop <- stops do %>
-                  <li><a href={"#{ alerts_stop_url(stop) }"}>{stop.name}</a></li>
+                <%= for {stop_id, stop_name} <- stops do %>
+                  <li><a href={~p"/stops/#{stop_id}"}>{stop_name}</a></li>
                 <% end %>
               </ul>
             <% end %>

lib/dotcom_web/templates/page/_tabbed_nav.html.heex

@@ -88,7 +88,12 @@
       class="m-tabbed-nav__content-item"
       data-tab-content-type="alerts"
     >
-      {alerts(@conn.assigns.alerts)}
+      {render("_alerts.html",
+        routes_with_high_priority_alerts_by_mode:
+          Dotcom.Alerts.routes_with_high_priority_alerts_by_mode(),
+        stops_with_accessibility_alerts_by_issue:
+          Dotcom.Alerts.stops_with_access_alerts_by_effect()
+      )}
     </div>
   </div>
 </nav>

lib/dotcom_web/views/page_view.ex

@@ -11,22 +11,6 @@ defmodule DotcomWeb.PageView do
   alias CMS.Partial.Banner
   alias DotcomWeb.PartialView
 
-  @spec alerts([Alerts.Alert.t()]) :: Phoenix.HTML.Safe.t()
-  def alerts(alerts) do
-    [routes, stops] =
-      [
-        &Dotcom.Alerts.routes_with_high_priority_alerts_by_mode/1,
-        &Dotcom.Alerts.stops_with_access_alerts_by_effect/1
-      ]
-      |> Task.async_stream(& &1.(alerts), timeout: 10_000)
-      |> Enum.map(fn {:ok, result} -> result end)
-
-    render("_alerts.html",
-      routes_with_high_priority_alerts_by_mode: routes,
-      stops_with_accessibility_alerts_by_issue: stops
-    )
-  end
-
   @spec alerts_mode_url(Routes.Route.gtfs_route_type()) :: String.t()
   defp alerts_mode_url(mode) do
     path =
@@ -80,15 +64,6 @@ defmodule DotcomWeb.PageView do
     )
   end
 
-  @spec alerts_stop_url(Stops.Stop.t()) :: String.t()
-  defp alerts_stop_url(stop) do
-    DotcomWeb.Router.Helpers.stop_url(
-      DotcomWeb.Endpoint,
-      :show,
-      stop.id
-    )
-  end
-
   def shortcut_icons do
     [:commuter_rail, :subway, :bus, :ferry, :the_ride]
     |> Enum.map(&shortcut_icon/1)

test/dotcom/alerts_test.exs

@@ -494,13 +494,17 @@ defmodule Dotcom.AlertsTest do
 
   describe "routes_with_high_priority_alerts_by_mode/1" do
     setup do
-      {:ok, %{alerts: Factories.Alerts.Alert.build_list(50, :alert)}}
+      Factories.Alerts.Alert.build_list(50, :alert, priority: :high)
+      |> Enum.map(&Factories.Alerts.Alert.active_now/1)
+      |> Alerts.Cache.Store.update(nil)
+
+      {:ok, %{}}
     end
 
-    test "builds list of routes by mode", %{alerts: alerts} do
+    test "builds list of routes by mode" do
       stub(Routes.Repo.Mock, :get, fn _ -> Factories.Routes.Route.build(:route) end)
 
-      for {mode, routes} <- routes_with_high_priority_alerts_by_mode(alerts) do
+      for {mode, routes} <- routes_with_high_priority_alerts_by_mode() do
         # valid mode
         assert Routes.Route.types_for_mode(mode)
 
@@ -510,10 +514,10 @@ defmodule Dotcom.AlertsTest do
       end
     end
 
-    test "doesn't error if nil routes", %{alerts: alerts} do
+    test "doesn't error if nil routes" do
       stub(Routes.Repo.Mock, :get, fn _ -> nil end)
 
-      assert routes_with_high_priority_alerts_by_mode(alerts) == [
+      assert routes_with_high_priority_alerts_by_mode() == [
                subway: [],
                bus: [],
                commuter_rail: [],
@@ -524,27 +528,31 @@ defmodule Dotcom.AlertsTest do
 
   describe "stops_with_access_alerts_by_effect/1" do
     setup do
-      {:ok, %{alerts: Factories.Alerts.Alert.build_list(50, :alert)}}
+      Factories.Alerts.Alert.build_list(50, :alert,
+        effect: fn -> Faker.Util.pick(Alerts.Accessibility.effect_types()) end,
+        priority: :high
+      )
+      |> Enum.map(&Factories.Alerts.Alert.active_now/1)
+      |> Alerts.Cache.Store.update(nil)
+
+      {:ok, %{}}
     end
 
-    test "builds list of stops by accessibility effect", %{alerts: alerts} do
+    test "builds list of stops by accessibility effect" do
       stub(Stops.Repo.Mock, :get_parent, fn _ -> Factories.Stops.Stop.build(:stop) end)
 
-      stops_by_effect = stops_with_access_alerts_by_effect(alerts)
+      stops_by_effect = stops_with_access_alerts_by_effect()
 
       for {effect, stops} <- stops_by_effect do
         assert effect in Alerts.Accessibility.effect_types()
-
-        if stops != [] do
-          assert [%Stops.Stop{} | _] = stops
-        end
+        assert [{<<_stop_id::binary>>, <<_stop_name::binary>>} | _] = stops
       end
     end
 
-    test "doesn't error if nil stops", %{alerts: alerts} do
+    test "doesn't error if nil stops" do
       stub(Stops.Repo.Mock, :get_parent, fn _ -> nil end)
 
-      stops_by_effect = stops_with_access_alerts_by_effect(alerts)
+      stops_by_effect = stops_with_access_alerts_by_effect()
       assert stops_by_effect == [elevator_closure: [], escalator_closure: [], access_issue: []]
     end
   end