perf(ScheduleController): Fetch schedules by individual stop for more…

… cache hits

As implemented, if any request for a stop fails, the entire request fails. This is to preserve the existing all-or-nothing behavior for fetching schedules for all stops together. If we want to have partial failure states in the future, we'll want to adopt a frontend changes to represent the partial failure state.

lib/mobile_app_backend_web/controllers/schedule_controller.ex

@@ -1,17 +1,31 @@
 defmodule MobileAppBackendWeb.ScheduleController do
   use MobileAppBackendWeb, :controller
+  require Logger
   alias MBTAV3API.JsonApi
   alias MBTAV3API.Repository
 
-  def schedules(conn, %{"stop_ids" => stop_ids, "date_time" => date_time}) do
-    if stop_ids == "" do
+  def schedules(conn, %{"stop_ids" => stop_ids_concat, "date_time" => date_time}) do
+    if stop_ids_concat == "" do
       json(conn, %{schedules: [], trips: %{}})
     else
-      {:ok, data} =
-        get_filter(stop_ids, date_time)
-        |> fetch_schedules()
+      stop_ids = String.split(stop_ids_concat, ",")
 
-      json(conn, data)
+      service_date = parse_service_date(date_time)
+
+      data =
+        stop_ids
+        |> Enum.map(&get_filter(&1, service_date))
+        |> fetch_schedules_parallel()
+
+      case data do
+        :error ->
+          conn
+          |> put_status(:internal_server_error)
+          |> json(%{error: "fetch_failed"})
+
+        data ->
+          json(conn, data)
+      end
     end
   end
 
@@ -39,11 +53,38 @@ defmodule MobileAppBackendWeb.ScheduleController do
     json(conn, response)
   end
 
-  @spec get_filter(String.t(), String.t()) :: [JsonApi.Params.filter_param()]
-  defp get_filter(stop_ids, date_time) do
-    date_time = Util.parse_datetime!(date_time)
-    service_date = Util.datetime_to_gtfs(date_time)
-    [stop: stop_ids, date: service_date]
+  @spec parse_service_date(String.t()) :: Date.t()
+  defp parse_service_date(date_string) do
+    date_string
+    |> Util.parse_datetime!()
+    |> Util.datetime_to_gtfs()
+  end
+
+  @spec get_filter(String.t(), Date.t()) :: [JsonApi.Params.filter_param()]
+  defp get_filter(stop_id, service_date) do
+    [stop: stop_id, date: service_date]
+  end
+
+  @spec fetch_schedules_parallel([[JsonApi.Params.filter_param()]]) ::
+          %{schedules: [MBTAV3API.Schedule.t()], trips: JsonApi.Object.trip_map()} | :error
+  defp fetch_schedules_parallel(filters) do
+    filters
+    |> Task.async_stream(fn filter_params ->
+      {filter_params, fetch_schedules(filter_params)}
+    end)
+    |> Enum.reduce_while(%{schedules: [], trips: %{}}, fn result, acc ->
+      case result do
+        {:ok, {_params, {:ok, %{schedules: schedules, trips: trips}}}} ->
+          {:cont, %{schedules: acc.schedules ++ schedules, trips: Map.merge(acc.trips, trips)}}
+
+        {_result_type, {params, api_response}} ->
+          Logger.warning(
+            "#{__MODULE__} skipped returning schedules due to error. params=#{inspect(params)} response=#{inspect(api_response)}"
+          )
+
+          {:halt, :error}
+      end
+    end)
   end
 
   @spec fetch_schedules([JsonApi.Params.filter_param()]) ::

test/mobile_app_backend_web/controllers/schedule_controller_test.exs

@@ -1,6 +1,7 @@
 defmodule MobileAppBackendWeb.ScheduleControllerTest do
   use MobileAppBackendWeb.ConnCase
   use HttpStub.Case
+  import ExUnit.CaptureLog
   import Mox
   import MobileAppBackend.Factory
   import Test.Support.Helpers
@@ -93,6 +94,153 @@ defmodule MobileAppBackendWeb.ScheduleControllerTest do
            } = json_response(conn, 200)
   end
 
+  test "returns schedules for multiple stops", %{conn: conn} do
+    s1 =
+      %MBTAV3API.Schedule{
+        id: "schedule-60565179-70159-90",
+        arrival_time: ~B[2024-03-13 01:07:00],
+        departure_time: ~B[2024-03-13 01:07:00],
+        drop_off_type: :regular,
+        pick_up_type: :regular,
+        stop_sequence: 90,
+        route_id: "Green-B",
+        stop_id: "70159",
+        trip_id: "60565179"
+      }
+
+    s2 = %MBTAV3API.Schedule{
+      id: "schedule-60565145-70158-590",
+      arrival_time: "2024-03-13T01:15:00-04:00",
+      departure_time: "2024-03-13T01:15:00-04:00",
+      drop_off_type: :regular,
+      pick_up_type: :regular,
+      stop_sequence: 590,
+      route_id: "Green-C",
+      stop_id: "70158",
+      trip_id: "60565145"
+    }
+
+    t1 = build(:trip, id: s1.trip_id)
+    t2 = build(:trip, id: s2.trip_id)
+
+    RepositoryMock
+    |> expect(:schedules, fn [
+                               filter: [
+                                 stop: "place-boyls",
+                                 date: ~D[2024-03-12]
+                               ],
+                               include: :trip,
+                               sort: {:departure_time, :asc}
+                             ],
+                             _opts ->
+      ok_response([s1], [t1])
+    end)
+    |> expect(:schedules, fn [
+                               filter: [
+                                 stop: "place-pktrm",
+                                 date: ~D[2024-03-12]
+                               ],
+                               include: :trip,
+                               sort: {:departure_time, :asc}
+                             ],
+                             _opts ->
+      ok_response([s2], [t2])
+    end)
+
+    conn =
+      get(conn, "/api/schedules", %{
+        stop_ids: "place-boyls,place-pktrm",
+        date_time: "2024-03-13T01:06:30-04:00"
+      })
+
+    assert %{
+             "schedules" => [
+               %{
+                 "arrival_time" => "2024-03-13T01:07:00-04:00",
+                 "departure_time" => "2024-03-13T01:07:00-04:00",
+                 "drop_off_type" => "regular",
+                 "id" => "schedule-60565179-70159-90",
+                 "pick_up_type" => "regular",
+                 "route_id" => "Green-B",
+                 "stop_id" => "70159",
+                 "stop_sequence" => 90,
+                 "trip_id" => "60565179"
+               },
+               %{
+                 "arrival_time" => "2024-03-13T01:15:00-04:00",
+                 "departure_time" => "2024-03-13T01:15:00-04:00",
+                 "drop_off_type" => "regular",
+                 "id" => "schedule-60565145-70158-590",
+                 "pick_up_type" => "regular",
+                 "route_id" => "Green-C",
+                 "stop_id" => "70158",
+                 "stop_sequence" => 590,
+                 "trip_id" => "60565145"
+               }
+             ],
+             "trips" => %{
+               "60565145" => %{},
+               "60565179" => %{}
+             }
+           } = json_response(conn, 200)
+  end
+
+  @tag :capture_log
+  test "when any stop fetch errors, then returns error", %{conn: conn} do
+    s1 =
+      %MBTAV3API.Schedule{
+        id: "schedule-60565179-70159-90",
+        arrival_time: ~B[2024-03-13 01:07:00],
+        departure_time: ~B[2024-03-13 01:07:00],
+        drop_off_type: :regular,
+        pick_up_type: :regular,
+        stop_sequence: 90,
+        route_id: "Green-B",
+        stop_id: "70159",
+        trip_id: "60565179"
+      }
+
+    t1 = build(:trip, id: s1.trip_id)
+
+    RepositoryMock
+    |> expect(:schedules, fn params, _opts ->
+      assert [
+               filter: [
+                 stop: "place-boyls",
+                 date: ~D[2024-03-12]
+               ],
+               include: :trip,
+               sort: {:departure_time, :asc}
+             ] = params
+
+      ok_response([s1], [t1])
+    end)
+    |> expect(:schedules, fn params, _opts ->
+      assert [
+               filter: [
+                 stop: "place-pktrm",
+                 date: ~D[2024-03-12]
+               ],
+               include: :trip,
+               sort: {:departure_time, :asc}
+             ] = params
+
+      {:error, :some_error_message}
+    end)
+
+    {conn, log} =
+      with_log([level: :warning], fn ->
+        get(conn, "/api/schedules", %{
+          stop_ids: "place-boyls,place-pktrm",
+          date_time: "2024-03-13T01:06:30-04:00"
+        })
+      end)
+
+    assert %{"error" => "fetch_failed"} = json_response(conn, 500)
+
+    assert log =~ "skipped returning schedules due to error"
+  end
+
   test "gracefully handles empty stops", %{conn: conn} do
     conn = get(conn, "/api/schedules", %{stop_ids: "", date_time: "2024-10-28T15:29:06-04:00"})
     assert json_response(conn, 200) == %{"schedules" => [], "trips" => %{}}