feat(Routes.Repo): use a behaviour (#2108)

* feat(Routes.Repo): use a behaviour

* remove unused filter shape priority

* remove extraneous routes repo get call

* refactor(Routes.Repo): remove extra caching logic

* refactor(Test.Support.Factory.Repo): consolidate factories

* tests: use Routes.Repo.Mock and Repo factory

* fixup (use different factories folder structure)

* rename Mbta -> MBTA

config/config.exs

@@ -15,6 +15,7 @@ config :dotcom, :location_service, LocationService
 config :dotcom, :repo_modules,
   predictions: Predictions.Repo,
   route_patterns: RoutePatterns.Repo,
+  routes: Routes.Repo,
   stops: Stops.Repo
 
 config :dotcom, :redis, Dotcom.Cache.Multilevel.Redis

config/dotcom/dotcom.exs

@@ -31,9 +31,6 @@ config :dotcom,
 
 config :dotcom, route_populate_caches?: config_env() == :prod
 
-routes_repo = if config_env() == :test, do: Routes.MockRepoApi, else: Routes.Repo
-config :dotcom, :routes_repo_api, routes_repo
-
 predictions_broadcast_interval_ms = if config_env() == :test, do: 50, else: 10_000
 
 config :dotcom, predictions_broadcast_interval_ms: predictions_broadcast_interval_ms

config/test.exs

@@ -14,6 +14,7 @@ config :dotcom, :location_service, LocationService.Mock
 config :dotcom, :repo_modules,
   predictions: Predictions.Repo.Mock,
   route_patterns: RoutePatterns.Repo.Mock,
+  routes: Routes.Repo.Mock,
   stops: Stops.Repo.Mock
 
 config :dotcom, :redis, Dotcom.Redis.Mock

lib/alerts/historical_alert.ex

@@ -21,6 +21,7 @@ defmodule Alerts.HistoricalAlert do
 
   @type entity_key :: :route | :stop
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   @spec from_alert(Alert.t()) :: t()
@@ -45,7 +46,7 @@ defmodule Alerts.HistoricalAlert do
   defp get_name(id, key) do
     module =
       case key do
-        :route -> Routes.Repo
+        :route -> @routes_repo
         :stop -> @stops_repo
       end
 

lib/algolia/object.ex

@@ -5,13 +5,14 @@ defprotocol Algolia.Object do
 end
 
 defimpl Algolia.Object, for: Stops.Stop do
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   def object_id(stop), do: "stop-" <> stop.id
   def url(stop), do: Util.site_path(:stop_path, [:show, stop])
 
   def data(stop) do
-    routes_for_stop = Routes.Repo.by_stop(stop.id)
+    routes_for_stop = @routes_repo.by_stop(stop.id)
 
     %{
       _geoloc: %{

lib/detailed_stop_group.ex

@@ -8,6 +8,7 @@ defmodule DetailedStopGroup do
   alias Routes.Route
   alias Stops.Stop
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   @type t :: {Route.t(), [DetailedStop.t()]}
@@ -36,7 +37,7 @@ defmodule DetailedStopGroup do
   defp stops_for_mode(mode) do
     mode
     |> Route.types_for_mode()
-    |> Routes.Repo.by_type()
+    |> @routes_repo.by_type()
     |> Task.async_stream(&{&1, @stops_repo.by_route(&1.id, 0)})
     |> Enum.map(fn {:ok, stops} -> stops end)
   end

lib/dotcom/realtime_schedule.ex

@@ -12,12 +12,12 @@ defmodule Dotcom.RealtimeSchedule do
   alias Dotcom.TransitNearMe
   alias Predictions.Prediction
   alias RoutePatterns.RoutePattern
-  alias Routes.Repo, as: RoutesRepo
   alias Routes.Route
   alias Schedules.RepoCondensed, as: SchedulesRepo
   alias Schedules.ScheduleCondensed
   alias Stops.Stop
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   # the long timeout is to address a worst-case scenario of cold schedule cache
@@ -28,7 +28,6 @@ defmodule Dotcom.RealtimeSchedule do
   @predictions_repo Application.compile_env!(:dotcom, :repo_modules)[:predictions]
 
   @default_opts [
-    routes_fn: &RoutesRepo.by_stop_with_route_pattern/1,
     schedules_fn: &SchedulesRepo.by_route_ids/2,
     alerts_fn: &Alerts.Repo.by_route_ids/2
   ]
@@ -50,12 +49,11 @@ defmodule Dotcom.RealtimeSchedule do
   @spec do_stop_data([Stop.id_t()], DateTime.t(), Keyword.t()) :: [map]
   defp do_stop_data(stop_ids, now, opts) do
     opts = Keyword.merge(@default_opts, opts)
-    routes_fn = Keyword.fetch!(opts, :routes_fn)
     schedules_fn = Keyword.fetch!(opts, :schedules_fn)
     alerts_fn = Keyword.fetch!(opts, :alerts_fn)
 
     # stage 1, get routes
-    routes_task = Task.async(fn -> get_routes(stop_ids, routes_fn) end)
+    routes_task = Task.async(fn -> get_routes(stop_ids) end)
     route_with_patterns = Task.await(routes_task)
 
     # stage 2, get stops, predictions, schedules, and alerts
@@ -94,13 +92,13 @@ defmodule Dotcom.RealtimeSchedule do
     Map.take(stop, [:id, :name, :accessibility, :address, :parking_lots])
   end
 
-  @spec get_routes([Stop.id_t()], fun()) :: [route_with_patterns_t]
-  defp get_routes(stop_ids, routes_fn) do
+  @spec get_routes([Stop.id_t()]) :: [route_with_patterns_t]
+  defp get_routes(stop_ids) do
     stop_ids
     |> Enum.map(
       &Task.async(fn ->
         &1
-        |> routes_fn.()
+        |> @routes_repo.by_stop_with_route_pattern()
         |> Enum.map(fn {route, route_patterns} ->
           {&1, JsonHelpers.stringified_route(route), route_patterns}
         end)

lib/dotcom/trip_plan/alerts.ex

@@ -13,9 +13,9 @@ defmodule Dotcom.TripPlan.Alerts do
   alias TripPlan.{Itinerary, Leg, TransitDetail}
 
   @default_opts [
-    route_by_id: &Routes.Repo.get/1,
     trip_by_id: &Schedules.Repo.trip/1
   ]
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
 
   @doc "Filters a list of Alerts to those relevant to the Itinerary"
   @spec filter_for_itinerary([Alert.t()], Itinerary.t(), Keyword.t()) :: [Alert.t()]
@@ -50,7 +50,7 @@ defmodule Dotcom.TripPlan.Alerts do
   end
 
   defp mode_entities(%TransitDetail{route_id: route_id, trip_id: trip_id}, opts) do
-    route = Keyword.get(opts, :route_by_id).(route_id)
+    route = @routes_repo.get(route_id)
     trip = Keyword.get(opts, :trip_by_id).(trip_id)
 
     route_type =

lib/dotcom/trip_plan/itinerary_row.ex

@@ -32,21 +32,19 @@ defmodule Dotcom.TripPlan.ItineraryRow do
           duration: Integer.t()
         }
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   defmodule Dependencies do
     @moduledoc false
 
-    defstruct route_mapper: &Routes.Repo.get/1,
-              trip_mapper: &Schedules.Repo.trip/1,
+    defstruct trip_mapper: &Schedules.Repo.trip/1,
               alerts_repo: &Alerts.Repo.all/1
 
-    @type route_mapper :: (Routes.Route.id_t() -> Routes.Route.t() | nil)
     @type trip_mapper :: (Schedules.Trip.id_t() -> Schedules.Trip.t() | nil)
     @type alerts_repo :: (DateTime.t() -> [Alerts.Alert.t()] | nil)
 
     @type t :: %__MODULE__{
-            route_mapper: route_mapper,
             trip_mapper: trip_mapper,
             alerts_repo: alerts_repo
           }
@@ -67,7 +65,7 @@ defmodule Dotcom.TripPlan.ItineraryRow do
   @spec from_leg(Leg.t(), Dependencies.t(), Leg.t() | nil) :: t
   def from_leg(leg, deps, next_leg) do
     trip = leg |> Leg.trip_id() |> parse_trip_id(deps.trip_mapper)
-    route = leg |> Leg.route_id() |> parse_route_id(deps.route_mapper)
+    route = leg |> Leg.route_id() |> parse_route_id()
     stop = name_from_position(leg.from)
 
     %__MODULE__{
@@ -77,7 +75,7 @@ defmodule Dotcom.TripPlan.ItineraryRow do
       trip: trip,
       departure: leg.start,
       steps: get_steps(leg.mode, next_leg),
-      additional_routes: get_additional_routes(route, trip, leg, stop, deps),
+      additional_routes: get_additional_routes(route, trip, leg, stop),
       duration: leg.duration,
       distance: leg.distance
     }
@@ -180,10 +178,10 @@ defmodule Dotcom.TripPlan.ItineraryRow do
     end
   end
 
-  @spec parse_route_id(:error | {:ok, String.t()}, Dependencies.route_mapper()) ::
+  @spec parse_route_id(:error | {:ok, String.t()}) ::
           Routes.Route.t() | nil
-  defp parse_route_id(:error, _route_mapper), do: nil
-  defp parse_route_id({:ok, route_id}, route_mapper), do: route_mapper.(route_id)
+  defp parse_route_id(:error), do: nil
+  defp parse_route_id({:ok, route_id}), do: @routes_repo.get(route_id)
 
   @spec parse_trip_id(:error | {:ok, String.t()}, Dependencies.trip_mapper()) ::
           Schedules.Trip.t() | nil
@@ -216,29 +214,27 @@ defmodule Dotcom.TripPlan.ItineraryRow do
           Route.t(),
           Schedules.Trip.t(),
           Leg.t(),
-          name_and_id,
-          Dependencies.t()
+          name_and_id
         ) :: [Route.t()]
   defp get_additional_routes(
          %Route{id: "Green" <> _line = route_id},
          trip,
          leg,
-         {_name, from_stop_id},
-         deps
+         {_name, from_stop_id}
        )
        when not is_nil(trip) do
     stop_pairs = GreenLine.stops_on_routes(trip.direction_id)
     {_to_stop_name, to_stop_id} = name_from_position(leg.to)
-    available_routes(route_id, from_stop_id, to_stop_id, stop_pairs, deps.route_mapper)
+    available_routes(route_id, from_stop_id, to_stop_id, stop_pairs)
   end
 
-  defp get_additional_routes(_route, _trip, _leg, _from, _deps), do: []
+  defp get_additional_routes(_route, _trip, _leg, _from), do: []
 
-  defp available_routes(current_route_id, from_stop_id, to_stop_id, stop_pairs, route_mapper) do
+  defp available_routes(current_route_id, from_stop_id, to_stop_id, stop_pairs) do
     GreenLine.branch_ids()
     |> List.delete(current_route_id)
     |> Enum.filter(&both_stops_on_route?(&1, from_stop_id, to_stop_id, stop_pairs))
-    |> Enum.map(route_mapper)
+    |> Enum.map(&@routes_repo.get/1)
   end
 
   defp both_stops_on_route?(route_id, from_stop_id, to_stop_id, stop_pairs) do

lib/dotcom/trip_plan/itinerary_row_list.ex

@@ -28,12 +28,12 @@ defmodule Dotcom.TripPlan.ItineraryRowList do
   @doc """
   Builds a ItineraryRowList from the given itinerary
   """
-  @spec from_itinerary(Itinerary.t(), ItineraryRow.Dependencies.t(), opts) :: t
+  @spec from_itinerary(Itinerary.t(), opts) :: t
   def from_itinerary(
         %Itinerary{legs: legs, accessible?: accessible?} = itinerary,
-        deps,
         opts \\ []
       ) do
+    deps = %ItineraryRow.Dependencies{}
     alerts = get_alerts(itinerary, deps)
     rows = get_rows(itinerary, deps, opts, alerts)
 
@@ -65,7 +65,6 @@ defmodule Dotcom.TripPlan.ItineraryRowList do
     |> deps.alerts_repo.()
     |> Dotcom.TripPlan.Alerts.filter_for_itinerary(
       itinerary,
-      route_by_id: deps.route_mapper,
       trip_by_id: deps.trip_mapper
     )
   end

lib/dotcom/trip_plan/related_link.ex

@@ -13,9 +13,6 @@ defmodule Dotcom.TripPlan.RelatedLink do
   alias TripPlan.{Itinerary, Leg}
 
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
-  @default_opts [
-    route_by_id: &Routes.Repo.get/1
-  ]
 
   defstruct text: "",
             url: "",
@@ -65,8 +62,6 @@ defmodule Dotcom.TripPlan.RelatedLink do
   @doc "Builds a list of related links for an Itinerary"
   @spec links_for_itinerary(Itinerary.t(), Keyword.t()) :: [t]
   def links_for_itinerary(itinerary, opts \\ []) do
-    opts = Keyword.merge(@default_opts, opts)
-
     for func <- [&route_links/2, &fare_links/2],
         link <- func.(itinerary, opts) do
       link
@@ -135,7 +130,7 @@ defmodule Dotcom.TripPlan.RelatedLink do
     new(["View ", text, " fare information"], url)
   end
 
-  defp fare_link_text(type) when type in [:commuter_rail, :ferry, :bus, :subway] do
+  defp fare_link_text(type) do
     Atom.to_string(type) |> String.replace("_", " ")
   end
 
@@ -155,6 +150,8 @@ defmodule Dotcom.TripPlan.RelatedLink do
     {"#{type}-fares", []}
   end
 
+  defp fare_link_url_opts(_, _leg), do: {"", []}
+
   # if there are multiple fare links, show fare overview link
   defp simplify_fare_text(fare_links) when Kernel.length(fare_links) > 1,
     do: [fare_overview_link()]

lib/dotcom_web/ambiguous_alert.ex

@@ -19,6 +19,7 @@ defprotocol DotcomWeb.AmbiguousAlert do
 end
 
 defimpl DotcomWeb.AmbiguousAlert, for: Alerts.Alert do
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   def alert_start_date(%{active_period: [{start_date, _} | _]}) do
@@ -32,7 +33,7 @@ defimpl DotcomWeb.AmbiguousAlert, for: Alerts.Alert do
     |> Alerts.Alert.get_entity(:route)
     |> MapSet.delete(nil)
     |> Enum.map(fn id ->
-      case Routes.Repo.get(id) do
+      case @routes_repo.get(id) do
         %{name: name} -> name
         _ -> id
       end

lib/dotcom_web/channels/vehicle_map_marker_channel.ex

@@ -7,6 +7,7 @@ defmodule DotcomWeb.VehicleMapMarkerChannel do
   alias Vehicles.Vehicle
 
   @predictions_repo Application.compile_env!(:dotcom, :repo_modules)[:predictions]
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   intercept(["reset", "add", "update", "remove"])
@@ -44,7 +45,7 @@ defmodule DotcomWeb.VehicleMapMarkerChannel do
         }
   @spec build_marker(Vehicles.Vehicle.t()) :: data_map
   def build_marker(%Vehicles.Vehicle{} = vehicle) do
-    route = Routes.Repo.get(vehicle.route_id)
+    route = @routes_repo.get(vehicle.route_id)
     stop_name = get_stop_name(vehicle.stop_id)
     trip = Schedules.Repo.trip(vehicle.trip_id)
 

lib/dotcom_web/controllers/alert_controller.ex

@@ -9,6 +9,7 @@ defmodule DotcomWeb.AlertController do
   plug(DotcomWeb.Plugs.AlertsByTimeframe)
   plug(DotcomWeb.Plug.Mticket)
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   @valid_ids ~w(subway commuter-rail bus ferry access)s
@@ -125,7 +126,7 @@ defmodule DotcomWeb.AlertController do
   end
 
   defp routes(%{assigns: %{route_type: route_type}} = conn, _opts),
-    do: assign(conn, :routes, Routes.Repo.by_type(route_type))
+    do: assign(conn, :routes, @routes_repo.by_type(route_type))
 
   defp routes(conn, _opts), do: conn
 

lib/dotcom_web/controllers/customer_support_controller.ex

@@ -78,6 +78,8 @@ defmodule DotcomWeb.CustomerSupportController do
     }
   }
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
+
   plug(:set_service_options)
   plug(:assign_ip)
   plug(:meta_description)
@@ -421,7 +423,7 @@ defmodule DotcomWeb.CustomerSupportController do
     bus_ferry_cr_options =
       for route_type <- 2..4, into: %{} do
         options =
-          Routes.Repo.by_type(route_type)
+          @routes_repo.by_type(route_type)
           |> Enum.map(fn route ->
             route.name
           end)

lib/dotcom_web/controllers/fare_controller.ex

@@ -7,6 +7,8 @@ defmodule DotcomWeb.FareController do
 
   plug(:meta_description)
 
+  @routes_repo Application.compile_env!(:dotcom, :repo_modules)[:routes]
+
   @options %{
     nearby_fn: %{
       retail: &Fares.RetailLocations.get_nearby/1,
@@ -127,7 +129,7 @@ defmodule DotcomWeb.FareController do
     one_way_fares =
       stop_id
       # Get fare_class for all routes at this stop and at connecting stops
-      |> Routes.Repo.by_stop(include: "stop.connecting_stops")
+      |> @routes_repo.by_stop(include: "stop.connecting_stops")
       |> Enum.map(&display_fare_class/1)
       |> Enum.uniq()
       # Sort in same order as @display_fare_classes