Rewrite MBTA Api using Req instead of HTTPoison

.env.template

@@ -9,9 +9,9 @@ OPEN_TRIP_PLANNER_URL=http://otp2-local.mbtace.com
 RECAPTCHA_PUBLIC_KEY=6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
 RECAPTCHA_PRIVATE_KEY=6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe
 
-# V3_API_KEY=
-V3_API_VERSION=2019-07-01
-V3_URL=https://api-dev.mbtace.com
+# MBTA_API_KEY=
+MBTA_API_VERSION=2019-07-01
+MBTA_API_BASE_URL=https://api-dev.mbtace.com
 
 # You can optionally set a Redis host and port.
 # The default host is 127.0.0.1

.github/workflows/algolia-update.yml

@@ -9,8 +9,8 @@ on:
 env:
   MIX_ENV: dev
   USE_SERVER_SENT_EVENTS: "false"
-  V3_URL: ${{ secrets.V3_URL }}
-  V3_API_KEY: ${{ secrets.V3_API_KEY }}
+  MBTA_API_BASE_URL: ${{ secrets.MBTA_API_BASE_URL }}
+  MBTA_API_KEY: ${{ secrets.MBTA_API_KEY }}
   WARM_CACHES: "false"
 
 jobs:

.github/workflows/tests.yml

@@ -15,8 +15,8 @@ concurrency:
 
 env:
   MIX_ENV: test
-  V3_URL: ${{ secrets.V3_URL }}
-  V3_API_KEY: ${{ secrets.V3_API_KEY }}
+  MBTA_API_BASE_URL: ${{ secrets.MBTA_API_BASE_URL }}
+  MBTA_API_KEY: ${{ secrets.MBTA_API_KEY }}
 
 jobs:
   # Report file changes by extensions

README.md

@@ -23,8 +23,8 @@ From a development standpoint, polyfills and code transforms are implemented via
 - [Getting Started](#getting-started)
 - [Running the Server](#running-the-server)
 - [Environment Variables](docs/ENVIRONMENT.md)
-    - [`V3_URL`](docs/ENVIRONMENT.md#v3_url)
-    - [`V3_API_KEY`](docs/ENVIRONMENT.md#v3_api_key)
+    - [`MBTA_API_BASE_URL`](docs/ENVIRONMENT.md#mbta_api_base_url)
+    - [`MBTA_API_KEY`](docs/ENVIRONMENT.md#mbta_api_key)
     - [`DRUPAL_ROOT`](docs/ENVIRONMENT.md#drupal_root)
     - [`ALGOLIA_APP_ID`, `ALGOLIA_SEARCH_KEY`, and `ALGOLIA_WRITE_KEY`](docs/ENVIRONMENT.md#algolia_app_id-algolia_search_key-and-algolia_write_key)
 - [Additional documentation](#additional-resources)

config/config.exs

@@ -4,11 +4,13 @@ config :elixir, ansi_enabled: true
 
 config :dotcom, :httpoison, HTTPoison
 
+config :dotcom, :mbta_api_module, MBTA.Api
+
 config :dotcom, :redis, Dotcom.Cache.Multilevel.Redis
 config :dotcom, :redix, Redix
 config :dotcom, :redix_pub_sub, Redix.PubSub
 
-config :dotcom, :mbta_api, MBTA.Api
+config :dotcom, :req_module, Req
 
 for config_file <- Path.wildcard("config/{deps,dotcom}/*.exs") do
   import_config("../#{config_file}")

config/runtime.exs

@@ -107,10 +107,11 @@ else
     ]
 end
 
-config :dotcom,
-  v3_api_base_url: System.get_env("V3_URL"),
-  v3_api_key: System.get_env("V3_API_KEY"),
-  v3_api_version: System.get_env("V3_API_VERSION", "2019-07-01")
+config :dotcom, :mbta_api,
+  base_url: System.get_env("MBTA_API_BASE_URL"),
+  enable_experimental_features: System.get_env("MBTA_API_ENABLE_EXPERIMENTAL_FEATURES"),
+  key: System.get_env("MBTA_API_KEY"),
+  version: System.get_env("MBTA_API_VERSION", "2019-07-01")
 
 config :dotcom, aws_index_prefix: System.get_env("AWS_PLACE_INDEX_PREFIX") || "dotcom-dev"
 
@@ -207,9 +208,6 @@ config :dotcom, DotcomWeb.ViewHelpers,
   google_tag_manager_auth: System.get_env("GOOGLE_TAG_MANAGER_AUTH"),
   google_tag_manager_preview: System.get_env("GOOGLE_TAG_MANAGER_PREVIEW")
 
-config :dotcom,
-  enable_experimental_features: System.get_env("ENABLE_EXPERIMENTAL_FEATURES")
-
 config :recaptcha,
   public_key: System.get_env("RECAPTCHA_PUBLIC_KEY"),
   secret: System.get_env("RECAPTCHA_PRIVATE_KEY")

config/test.exs

@@ -1,11 +1,15 @@
 import Config
 
 config :dotcom, :cache, Dotcom.Cache.TestCache
+
+config :dotcom, :httpoison, HTTPoison.Mock
+
+config :dotcom, :mbta_api_module, MBTA.Api.Mock
+
 config :dotcom, :redis, Dotcom.Redis.Mock
 config :dotcom, :redix, Dotcom.Redix.Mock
 config :dotcom, :redix_pub_sub, Dotcom.Redix.PubSub.Mock
-config :dotcom, :trip_plan_feedback_cache, Dotcom.Cache.TestCache
 
-config :dotcom, :httpoison, HTTPoison.Mock
+config :dotcom, :req_module, Req.Mock
 
-config :dotcom, :mbta_api, MBTA.Api.Mock
+config :dotcom, :trip_plan_feedback_cache, Dotcom.Cache.TestCache

coveralls.json

@@ -1,6 +1,6 @@
 {
   "coverage_options": {
-    "minimum_coverage": 62,
+    "minimum_coverage": 60,
     "treat_no_relevant_lines_as_covered": true
   },
   "custom_stop_words": [

docs/ENVIRONMENT.md

@@ -21,16 +21,15 @@ There are many ways to set the environment variables described here:
 
 ## Required
 
-### `V3_URL`
+### `MBTA_API_BASE_URL`
 
 The URL of the MBTA V3 API server, e.g. `https://api-dev.mbtace.com`.
 
-### `V3_API_KEY`
+### `MBTA_API_KEY`
 
 The key to use with the MBTA API (see `README`). This is a practical requirement
 for development since requests without an API key have a very low rate limit.
 
-
 ## Optional
 
 ### `DRUPAL_ROOT`

integration/health_checks/v3-api.js → integration/health_checks/mbta-api.js

@@ -1,7 +1,7 @@
 const { status200 } = require("../utils");
 
 const options = {
-  baseURL: process.env.V3_URL,
+  baseURL: process.env.MBTA_API_BASE_URL,
   url: "/status",
 };
 

lib/json_api.ex

@@ -40,15 +40,22 @@ defmodule JsonApi do
     }
   end
 
-  @spec parse(String.t()) :: JsonApi.t() | {:error, any}
+  @spec parse(String.t() | map()) :: {:error, any()} | JsonApi.t()
+  def parse(body) when is_binary(body) do
+    case Jason.decode(body) do
+      {:ok, parsed} -> parse(parsed)
+      {:error, error} -> {:error, error}
+    end
+  end
+
   def parse(body) do
-    with {:ok, parsed} <- Jason.decode(body),
-         {:ok, data} <- parse_data(parsed) do
-      %JsonApi{
-        links: parse_links(parsed),
-        data: data
-      }
-    else
+    case parse_data(body) do
+      {:ok, data} ->
+        %JsonApi{
+          links: parse_links(body),
+          data: data
+        }
+
       {:error, [_ | _] = errors} ->
         {:error, parse_errors(errors)}
 

lib/mbta/api.ex

@@ -3,152 +3,28 @@ defmodule MBTA.Api do
   Handles fetching and caching generic JSON:API responses from the MBTA API.
   """
 
-  require Logger
-
-  use HTTPoison.Base
-
-  alias MBTA.SentryExtra
-  alias Util
-
   @behaviour MBTA.Api.Behaviour
 
-  @default_timeout Application.compile_env!(:dotcom, :v3_api_default_timeout)
-  @httpoison Application.compile_env!(:dotcom, :httpoison)
-  @http_pool Application.compile_env!(:dotcom, :v3_api_http_pool)
+  @req Application.compile_env(:dotcom, :req_module)
 
   @impl MBTA.Api.Behaviour
-  def get_json(url, params \\ [], opts \\ []) do
-    _ =
-      Logger.debug(fn ->
-        "MBTA.Api.get_json url=#{url} params=#{params |> Map.new() |> Poison.encode!()}"
-      end)
-
-    body = ""
-    opts = Keyword.merge(default_options(), opts)
-
-    with {time, response} <- timed_get(url, params, opts),
-         :ok <- log_response(url, params, time, response),
-         {:ok, http_response} <- response,
-         {:ok, body} <- body(http_response) do
-      body
-      |> JsonApi.parse()
-      |> maybe_log_parse_error(url, params, body)
-    else
-      {:error, error} ->
-        _ = log_response_error(url, params, body)
-        {:error, error}
-
-      error ->
-        _ = log_response_error(url, params, body)
-        {:error, error}
+  def get_json(url, params \\ []) do
+    case client() |> @req.get(url: url, params: params) do
+      {:ok, response} -> JsonApi.parse(response.body)
+      {:error, reason} -> {:error, reason}
     end
   end
 
-  defp timed_get(url, params, opts) do
-    api_key = Keyword.fetch!(opts, :api_key)
-    base_url = Keyword.fetch!(opts, :base_url)
-
-    headers = MBTA.Headers.build(api_key)
-
-    url = base_url <> URI.encode(url)
-    timeout = Keyword.fetch!(opts, :timeout)
-
-    {time, response} =
-      :timer.tc(fn ->
-        @httpoison.get(url, headers,
-          params: params,
-          timeout: timeout,
-          recv_timeout: timeout,
-          hackney: [pool: @http_pool]
-        )
-      end)
-
-    {time, response}
-  end
-
-  @spec maybe_log_parse_error(JsonApi.t() | {:error, any}, String.t(), Keyword.t(), String.t()) ::
-          JsonApi.t() | {:error, any}
-  defp maybe_log_parse_error({:error, error}, url, params, body) do
-    _ = log_response_error(url, params, body)
-    {:error, error}
-  end
-
-  defp maybe_log_parse_error(response, _, _, _) do
-    response
-  end
-
-  @spec log_response(String.t(), Keyword.t(), integer, any) :: :ok
-  defp log_response(url, params, time, response) do
-    entry = fn ->
-      "MBTA.Api.get_json_response url=#{inspect(url)} " <>
-        "params=#{params |> Map.new() |> Poison.encode!()} " <>
-        log_body(response) <>
-        " duration=#{time / 1000}" <>
-        " request_id=#{Logger.metadata() |> Keyword.get(:request_id)}"
-    end
-
-    _ = SentryExtra.log_context("api-response", entry)
-    _ = Logger.info(entry)
-    :ok
-  end
-
-  @spec log_response_error(String.t(), Keyword.t(), String.t()) :: :ok
-  defp log_response_error(url, params, body) do
-    entry = fn ->
-      "MBTA.Api.get_json_response url=#{inspect(url)} " <>
-        "params=#{params |> Map.new() |> Poison.encode!()} response=" <> body
-    end
-
-    _ = SentryExtra.log_context("api-response-error", entry)
-    _ = Logger.info(entry)
-    :ok
-  end
-
-  defp log_body({:ok, response}) do
-    "status=#{response.status_code} content_length=#{byte_size(response.body)}"
-  end
-
-  defp log_body({:error, error}) do
-    ~s(status=error error="#{inspect(error)}")
-  end
-
-  def body(%{headers: headers, body: body}) do
-    case Enum.find(
-           headers,
-           &(String.downcase(elem(&1, 0)) == "content-encoding")
-         ) do
-      {_, "gzip"} ->
-        {:ok, :zlib.gunzip(body)}
-
-      _ ->
-        {:ok, body}
-    end
-  rescue
-    e in ErlangError -> {:error, e.original}
-  end
-
-  def body(other) do
-    other
-  end
-
-  @impl HTTPoison.Base
-  def process_request_headers(headers) do
-    [
-      {"accept-encoding", "gzip"},
-      {"accept", "application/vnd.api+json"}
-      | headers
-    ]
-  end
-
-  defp default_options do
-    [
-      base_url: config(:v3_api_base_url),
-      api_key: config(:v3_api_key),
-      timeout: @default_timeout
-    ]
-  end
+  defp client do
+    config = Application.get_env(:dotcom, :mbta_api)
 
-  defp config(key) do
-    Util.config(:dotcom, key)
+    @req.new(
+      base_url: config[:base_url],
+      headers: [
+        {"MBTA-Version", config[:version]},
+        {"x-api-key", config[:key]},
+        {"x-enable-experimental-features", config[:enable_experimental_features]}
+      ]
+    )
   end
 end

lib/mbta/api/alerts.ex

@@ -3,7 +3,7 @@ defmodule MBTA.Api.Alerts do
   Responsible for fetching Alert data from the V3 API.
   """
 
-  @mbta_api Application.compile_env!(:dotcom, :mbta_api)
+  @mbta_api Application.compile_env!(:dotcom, :mbta_api_module)
 
   @spec all() :: JsonApi.t() | {:error, any}
   def all(params \\ []) do

lib/mbta/api/behaviour.ex

@@ -5,9 +5,8 @@ defmodule MBTA.Api.Behaviour do
 
   @callback get_json(String.t()) :: JsonApi.t() | {:error, any}
   @callback get_json(String.t(), Keyword.t()) :: JsonApi.t() | {:error, any}
-  @callback get_json(String.t(), Keyword.t(), Keyword.t()) :: JsonApi.t() | {:error, any}
 
-  @implementation Application.compile_env!(:dotcom, :mbta_api)
+  @implementation Application.compile_env!(:dotcom, :mbta_api_module)
 
   def get_json(url) do
     @implementation.get_json(url)
@@ -16,8 +15,4 @@ defmodule MBTA.Api.Behaviour do
   def get_json(url, params) do
     @implementation.get_json(url, params)
   end
-
-  def get_json(url, params, opts) do
-    @implementation.get_json(url, params, opts)
-  end
 end