Remove Httpoison in the CMS api code and replace with req (#2015)

.env.template

@@ -4,7 +4,7 @@
 # ALGOLIA_SEARCH_KEY=
 # ALGOLIA_WRITE_KEY=
 
-DRUPAL_ROOT=https://live-mbta.pantheonsite.io
+CMS_API_BASE_URL=https://live-mbta.pantheonsite.io
 OPEN_TRIP_PLANNER_URL=http://otp2-local.mbtace.com
 RECAPTCHA_PUBLIC_KEY=6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
 RECAPTCHA_PRIVATE_KEY=6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe

README.md

@@ -23,10 +23,10 @@ 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)
+    - [`ALGOLIA_APP_ID`, `ALGOLIA_SEARCH_KEY`, and `ALGOLIA_WRITE_KEY`](docs/ENVIRONMENT.md#algolia_app_id-algolia_search_key-and-algolia_write_key)
+    - [`CMS_API_BASE_URL`](docs/ENVIRONMENT.md#cms_api_base_url)
     - [`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)
 
 ## Getting Started

config/config.exs

@@ -2,6 +2,8 @@ import Config
 
 config :elixir, ansi_enabled: true
 
+config :dotcom, :cms_api_module, CMS.Api
+
 config :dotcom, :httpoison, HTTPoison
 
 config :dotcom, :mbta_api_module, MBTA.Api

config/runtime.exs

@@ -42,13 +42,6 @@ if config_env() == :dev do
   config :dotcom,
     dev_server?: true,
     webpack_path: webpack_path
-
-  unless System.get_env("DRUPAL_ROOT") do
-    # To see CMS content locally, please read the README on how to setup Kalabox.
-    # These instructions will help you run the CMS locally and configure the
-    # correct endpoint for the drupal root.
-    System.put_env("DRUPAL_ROOT", "http://temp-drupal.invalid")
-  end
 end
 
 # Redis cluster configuration
@@ -107,6 +100,13 @@ else
     ]
 end
 
+config :dotcom, :cms_api,
+  base_url: System.get_env("CMS_API_BASE_URL", System.get_env("DRUPAL_ROOT")),
+  headers: [
+    {"Content-Type", "application/json"},
+    {"Accept", "application/json"}
+  ]
+
 config :dotcom, :mbta_api,
   base_url: System.get_env("MBTA_API_BASE_URL", System.get_env("V3_URL")),
   headers: [
@@ -138,14 +138,6 @@ if config_env() != :test and System.get_env("OPEN_TRIP_PLANNER_URL") != "" do
     otp_url: System.get_env("OPEN_TRIP_PLANNER_URL")
 end
 
-if config_env() != :test do
-  config :dotcom,
-    drupal: [
-      cms_root: System.fetch_env!("DRUPAL_ROOT"),
-      cms_static_path: "/sites/default/files"
-    ]
-end
-
 if config_env() == :prod do
   config :dotcom, alerts_bus_stop_change_bucket: System.get_env("S3_PREFIX_BUSCHANGE")
 

config/test.exs

@@ -4,6 +4,7 @@ config :dotcom, :cache, Dotcom.Cache.TestCache
 
 config :dotcom, :httpoison, HTTPoison.Mock
 
+config :dotcom, :cms_api_module, CMS.Api.Static
 config :dotcom, :mbta_api_module, MBTA.Api.Mock
 config :dotcom, :repo_modules, route_patterns: RoutePatterns.Repo.Mock
 

docs/ENVIRONMENT.md

@@ -32,7 +32,7 @@ for development since requests without an API key have a very low rate limit.
 
 ## Optional
 
-### `DRUPAL_ROOT`
+### `CMS_API_BASE_URL`
 
 The URL for our CMS. You'll need to set this to view any of the static content
 on the site.

integration/health_checks/cms.js

@@ -1,7 +1,7 @@
 const { status200 } = require("../utils");
 
 const options = {
-  baseURL: process.env.DRUPAL_ROOT,
+  baseURL: process.env.CMS_API_BASE_URL || process.env.DRUPAL_ROOT,
   url: "/pantheon_healthcheck",
 };
 

lib/cms/api.ex

@@ -1,31 +1,98 @@
-defmodule CMS.API do
+defmodule CMS.Api do
   @moduledoc """
-  The behaviour for a live HTTP or a static testing API over our content CMS.
+  Handles fetching and caching generic JSON:API responses from the CMS API.
   """
 
-  @type error :: :not_found | :timeout | :invalid_response | {:redirect, integer, Keyword.t()}
-  @type response :: {:ok, map() | [map()]} | {:error, error}
-
-  @type type ::
-          :diversion
-          | :event
-          | :news_entry
-          | :page
-          | :project
-          | :project_update
-
-  @type route_term :: %{
-          id: String.t(),
-          group: String.t(),
-          mode: String.t() | nil
-        }
-
-  @doc """
-  Issues a request for a given path, with optional parameters
-  for the request. Parses the JSON result but does not do anything
-  beyond that. Shouldn't raise an exception; if the HTTP request
-  or JSON decoding fails, returns {:error, message}
-  """
-  @callback view(String.t(), Keyword.t() | map) :: response
-  @callback preview(integer, integer | nil) :: {:ok, [map()]} | {:error, error}
+  @behaviour CMS.Api.Behaviour
+
+  @req Application.compile_env(:dotcom, :req_module)
+
+  @impl CMS.Api.Behaviour
+  def preview(node_id, revision_id) do
+    path = ~s(/cms/revisions/#{node_id})
+
+    case client(vid: revision_id) |> @req.get(url: path) do
+      {:ok, response} -> {:ok, response.body}
+      {:error, reason} -> {:error, reason}
+    end
+  end
+
+  @impl CMS.Api.Behaviour
+  def view(path, params) do
+    params = [
+      {"_format", "json"}
+      | Enum.reduce(params, [], &stringify_params/2)
+    ]
+
+    case client() |> @req.get(url: path, params: params) do
+      {:ok, response} -> {:ok, response.body}
+      {:error, reason} -> {:error, reason}
+    end
+  end
+
+  defp client(headers \\ []) do
+    config = Application.get_env(:dotcom, :cms_api)
+
+    @req.new(
+      base_url: config[:base_url],
+      headers: config[:headers] ++ headers
+    )
+  end
+
+  @type param_key :: String.t() | atom()
+  @type param_list :: [{String.t(), String.t()}]
+  @type param_value :: String.t() | atom() | Keyword.t()
+  @type nested_param :: {safe_key(), String.t()} | map | String.t()
+
+  # Allow only whitelisted, known, nested params.
+  # Note: when redirecting from CMS, nested params will
+  # be shaped as a Map.t() with String.t() keys and values.
+  @type safe_key :: :value | :min | :max | String.t()
+  @safe_keys [:value, :min, :max, "latitude", "longitude", "type"]
+
+  @spec stringify_params({param_key, param_value}, param_list) :: param_list
+  def stringify_params({key, val}, acc) when is_atom(key) do
+    stringify_params({Atom.to_string(key), val}, acc)
+  end
+
+  def stringify_params({key, val}, acc) when is_atom(val) do
+    stringify_params({key, Atom.to_string(val)}, acc)
+  end
+
+  def stringify_params({key, val}, acc) when is_integer(val) do
+    stringify_params({key, Integer.to_string(val)}, acc)
+  end
+
+  def stringify_params({key, val}, acc) when is_binary(key) and is_binary(val) do
+    [{key, val} | acc]
+  end
+
+  def stringify_params({key, val}, acc) when is_binary(key) and (is_map(val) or is_list(val)) do
+    val
+    # drop original param, add new key/vals for nested params
+    |> Enum.reduce(acc, fn nested_param, acc -> list_to_params(key, acc, nested_param) end)
+    # restore original order of nested params
+    |> Enum.reverse()
+  end
+
+  def stringify_params(_, acc) do
+    # drop invalid param
+    acc
+  end
+
+  # Convert nested key values to their own keys, if whitelisted
+  @spec list_to_params(String.t(), param_list, nested_param) :: param_list
+  defp list_to_params(key, acc, {sub_key, sub_val}) when sub_key in @safe_keys do
+    stringify_params({key <> "[#{sub_key}]", sub_val}, acc)
+  end
+
+  # Drupal's View system expects a key[]=value format for multiple values
+  defp list_to_params(key, acc, value) when key in @safe_keys do
+    stringify_params({key <> "[]", value}, acc)
+  end
+
+  # Drop entire param (key[subkey]=val) completely
+  defp list_to_params(_, acc, _) do
+    acc
+  end
 end

lib/cms/api/behaviour.ex

@@ -0,0 +1,31 @@
+defmodule CMS.Api.Behaviour do
+  @moduledoc """
+  The behaviour for a live HTTP or a static testing API over our content CMS.
+  """
+
+  @type error :: :not_found | :timeout | :invalid_response | {:redirect, integer, Keyword.t()}
+  @type response :: {:ok, map() | [map()]} | {:error, error}
+
+  @type type ::
+          :diversion
+          | :event
+          | :news_entry
+          | :page
+          | :project
+          | :project_update
+
+  @type route_term :: %{
+          id: String.t(),
+          group: String.t(),
+          mode: String.t() | nil
+        }
+
+  @doc """
+  Issues a request for a given path, with optional parameters
+  for the request. Parses the JSON result but does not do anything
+  beyond that. Shouldn't raise an exception; if the HTTP request
+  or JSON decoding fails, returns {:error, message}
+  """
+  @callback preview(integer, integer | nil) :: response
+  @callback view(String.t(), Keyword.t() | map) :: response
+end

lib/cms/api/static.ex

@@ -1,10 +1,11 @@
-defmodule CMS.API.Static do
+defmodule CMS.Api.Static do
   @moduledoc """
+  This module provides static responses for the CMS API.
 
-  Emulates Drupal REST API (both native and Views-based)
-
+  Should be removed when we rewrite all tests in this module.
   """
-  @behaviour CMS.API
+
+  @behaviour CMS.Api.Behaviour
 
   alias CMS.Helpers
   alias CMS.Page.NewsEntry