feat(MBTA.Api): guard checking type and length (#2086)

lib/mbta/api.ex

@@ -7,6 +7,10 @@ defmodule MBTA.Api do
 
   @req Application.compile_env(:dotcom, :req_module)
 
+  defmacro is_valid_potential_id(id) do
+    quote do: is_binary(unquote(id)) and unquote(id) != ""
+  end
+
   @impl MBTA.Api.Behaviour
   def get_json(url, params \\ []) do
     case client() |> @req.get(url: URI.encode(url), params: params) do

lib/mbta/api/route_patterns.ex

@@ -3,6 +3,8 @@ defmodule MBTA.Api.RoutePatterns do
   Responsible for fetching Route Pattern data from the V3 API.
   """
 
+  import MBTA.Api, only: [is_valid_potential_id: 1]
+
   alias Routes.Route
 
   @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
@@ -15,7 +17,11 @@ defmodule MBTA.Api.RoutePatterns do
   end
 
   @spec get(Route.id_t(), keyword()) :: api_response_t()
-  def get(id, params \\ []) do
+  def get(id, params \\ [])
+
+  def get(id, params) when is_valid_potential_id(id) do
     @mbta_api.get_json("/route_patterns/#{id}", params)
   end
+
+  def get(id, _), do: {:error, {:invalid_id, id}}
 end

lib/mbta/api/routes.ex

@@ -3,6 +3,8 @@ defmodule MBTA.Api.Routes do
   Responsible for fetching Route data from the V3 API.
   """
 
+  import MBTA.Api, only: [is_valid_potential_id: 1]
+
   alias Routes.Route
   alias Stops.Stop
 
@@ -16,10 +18,14 @@ defmodule MBTA.Api.Routes do
   end
 
   @spec get(Route.id_t(), keyword()) :: api_response_t()
-  def get(id, params \\ []) do
+  def get(id, params \\ [])
+
+  def get(id, params) when is_valid_potential_id(id) do
     @mbta_api.get_json("/routes/#{id}", params)
   end
 
+  def get(id, _), do: {:error, {:invalid_id, id}}
+
   @spec by_type(Route.type_int(), keyword()) :: api_response_t()
   def by_type(type, params \\ []) do
     params = put_in(params[:type], type)

lib/mbta/api/services.ex

@@ -1,13 +1,21 @@
 defmodule MBTA.Api.Services do
-  @moduledoc false
+  @moduledoc """
+  Fetches Service data from the MBTA V3 API.
+  """
+
+  import MBTA.Api, only: [is_valid_potential_id: 1]
 
   @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
 
   def all(params \\ []) do
     @mbta_api.get_json("/services/", params)
   end
 
-  def get(id, params \\ []) do
+  def get(id, params \\ [])
+
+  def get(id, params) when is_valid_potential_id(id) do
     @mbta_api.get_json("/services/#{id}", params)
   end
+
+  def get(id, _), do: {:error, {:invalid_id, id}}
 end

lib/mbta/api/shapes.ex

@@ -3,13 +3,17 @@ defmodule MBTA.Api.Shapes do
   Responsible for fetching Shape data from the V3 API.
   """
 
+  import MBTA.Api, only: [is_valid_potential_id: 1]
+
   @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
 
   def all(params \\ []) do
     @mbta_api.get_json("/shapes/", params)
   end
 
-  def by_id(id) do
-    @mbta_api.get_json("/shapes/" <> id)
+  def by_id(id) when is_valid_potential_id(id) do
+    @mbta_api.get_json("/shapes/#{id}")
   end
+
+  def by_id(id), do: {:error, {:invalid_id, id}}
 end

lib/mbta/api/stops.ex

@@ -3,13 +3,19 @@ defmodule MBTA.Api.Stops do
   Responsible for fetching Stop data from the V3 API.
   """
 
+  import MBTA.Api, only: [is_valid_potential_id: 1]
+
   @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
 
   def all(params \\ []) do
     @mbta_api.get_json("/stops/", params)
   end
 
-  def by_gtfs_id(gtfs_id, params \\ []) do
+  def by_gtfs_id(gtfs_id, params \\ [])
+
+  def by_gtfs_id(gtfs_id, params) when is_valid_potential_id(gtfs_id) do
     @mbta_api.get_json("/stops/#{gtfs_id}", params)
   end
+
+  def by_gtfs_id(gtfs_id, _), do: {:error, {:invalid_id, gtfs_id}}
 end

lib/mbta/api/trips.ex

@@ -3,12 +3,18 @@ defmodule MBTA.Api.Trips do
   Responsible for fetching Trip data from the MBTA Api.
   """
 
+  import MBTA.Api, only: [is_valid_potential_id: 1]
+
   @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
 
-  def by_id(id, params \\ []) do
-    @mbta_api.get_json("/trips/" <> id, params)
+  def by_id(id, params \\ [])
+
+  def by_id(id, params) when is_valid_potential_id(id) do
+    @mbta_api.get_json("/trips/#{id}", params)
   end
 
+  def by_id(id, _), do: {:error, {:invalid_id, id}}
+
   def by_route(route_id, params \\ []) do
     params = Kernel.put_in(params[:route], route_id)
 

test/dotcom_web/controllers/trip_plan_controller_test.exs

@@ -813,7 +813,7 @@ defmodule DotcomWeb.TripPlanControllerTest do
     end
 
     test "doesn't set custom_route? flag for regular routes", %{itineraries: itineraries} do
-      expect(MBTA.Api.Mock, :get_json, fn "/routes/" <> _, _ ->
+      stub(MBTA.Api.Mock, :get_json, fn "/routes/" <> _, _ ->
         %JsonApi{
           data: [build(:route_item)]
         }

test/mbta/api/route_patterns_test.exs

@@ -25,7 +25,7 @@ defmodule Mbta.Api.RoutePatternsTest do
 
   test "get/1 returns a route pattern" do
     # Setup
-    id = 1
+    id = Faker.Internet.slug()
 
     expect(Mock, :get_json, fn url, _ ->
       assert url == "/route_patterns/#{id}"
@@ -34,7 +34,7 @@ defmodule Mbta.Api.RoutePatternsTest do
     end)
 
     # Exercise
-    route_pattern = RoutePatterns.get(1)
+    route_pattern = RoutePatterns.get(id)
 
     # Verify
     assert route_pattern == %{}

test/mbta/api/routes_test.exs

@@ -25,7 +25,7 @@ defmodule MBTA.Api.RoutesTest do
 
   test "get/1 returns a route" do
     # Setup
-    id = :rand.uniform(100)
+    id = Faker.Internet.slug()
 
     expect(Mock, :get_json, fn url, _ ->
       assert url == "/routes/#{id}"

test/mbta/api/services_test.exs

@@ -25,7 +25,7 @@ defmodule MBTA.Api.ServicesTest do
 
   test "get/1 returns a service" do
     # Setup
-    id = :rand.uniform(100)
+    id = Faker.Internet.slug()
 
     expect(Mock, :get_json, fn url, _ ->
       assert url == "/services/#{id}"