diff --git a/lib/mbta/api.ex b/lib/mbta/api.ex index f988ed8a36..cd00019298 100644 --- a/lib/mbta/api.ex +++ b/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 diff --git a/lib/mbta/api/route_patterns.ex b/lib/mbta/api/route_patterns.ex index 4dc69ca341..9ddbfd826b 100644 --- a/lib/mbta/api/route_patterns.ex +++ b/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 diff --git a/lib/mbta/api/routes.ex b/lib/mbta/api/routes.ex index a736b31e7f..fa0109972e 100644 --- a/lib/mbta/api/routes.ex +++ b/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) diff --git a/lib/mbta/api/services.ex b/lib/mbta/api/services.ex index 9a6f459a87..e410287a8d 100644 --- a/lib/mbta/api/services.ex +++ b/lib/mbta/api/services.ex @@ -1,5 +1,9 @@ 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) @@ -7,7 +11,11 @@ defmodule MBTA.Api.Services 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 diff --git a/lib/mbta/api/shapes.ex b/lib/mbta/api/shapes.ex index 6cebe7b643..2ed6c9b503 100644 --- a/lib/mbta/api/shapes.ex +++ b/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 diff --git a/lib/mbta/api/stops.ex b/lib/mbta/api/stops.ex index dd88d72f4a..3d1e388b3c 100644 --- a/lib/mbta/api/stops.ex +++ b/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 diff --git a/lib/mbta/api/trips.ex b/lib/mbta/api/trips.ex index ed050d8a00..975f2a23b4 100644 --- a/lib/mbta/api/trips.ex +++ b/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) diff --git a/test/dotcom_web/controllers/trip_plan_controller_test.exs b/test/dotcom_web/controllers/trip_plan_controller_test.exs index c62c89680d..c6e5075646 100644 --- a/test/dotcom_web/controllers/trip_plan_controller_test.exs +++ b/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)] } diff --git a/test/mbta/api/route_patterns_test.exs b/test/mbta/api/route_patterns_test.exs index 1f27e3bd3d..3023c565d9 100644 --- a/test/mbta/api/route_patterns_test.exs +++ b/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 == %{} diff --git a/test/mbta/api/routes_test.exs b/test/mbta/api/routes_test.exs index c7554dcc6f..079ab8895b 100644 --- a/test/mbta/api/routes_test.exs +++ b/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}" diff --git a/test/mbta/api/services_test.exs b/test/mbta/api/services_test.exs index 602fb714de..b035e73ed0 100644 --- a/test/mbta/api/services_test.exs +++ b/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}"