Merge pull request #1138 from code-corps/refactor-github-api-to-use-gateway

Switched the GitHub API to use a simple Gateway module

Author: joshsmith

config/config.exs

@@ -67,7 +67,7 @@ config :code_corps, :corsica_log_level, [rejected: :warn]
 {:ok, pem} = (System.get_env("GITHUB_APP_PEM") || "") |> Base.decode64()
 
 config :code_corps,
-  github: CodeCorps.GitHub.API,
+  github: CodeCorps.GitHub.API.Gateway,
   github_app_id: System.get_env("GITHUB_APP_ID"),
   github_app_client_id: System.get_env("GITHUB_APP_CLIENT_ID"),
   github_app_client_secret: System.get_env("GITHUB_APP_CLIENT_SECRET"),

lib/code_corps/cloudex/cloudex_test.ex

@@ -14,7 +14,7 @@ defmodule CloudexTest do
     end
   end
 
-  @spec upload(String.t) :: Cloudex.UploadedImage.t
+  @spec upload(String.t) :: %Cloudex.UploadedImage{}
   def upload(_url) do
     [ok: %Cloudex.UploadedImage{public_id: fake_cloudinary_id()}]
   end

lib/code_corps/github/api/api.ex

@@ -2,19 +2,37 @@ defmodule CodeCorps.GitHub.API do
   alias CodeCorps.{
     GithubAppInstallation,
     GitHub,
+    GitHub.API.Pagination,
     GitHub.APIError,
     GitHub.HTTPClientError,
     User
   }
 
+  alias HTTPoison.{Error, Response}
+
+  def gateway(), do: Application.get_env(:code_corps, :github)
+
   @spec request(GitHub.method, String.t, GitHub.headers, GitHub.body, list) :: GitHub.response
-  def request(method, url, headers, body, options) do
-    method
-    |> :hackney.request(url, headers, body, options)
+  def request(method, url, body, headers, options) do
+    gateway().request(method, url, body, headers, options)
     |> marshall_response()
   end
 
-  defdelegate get_all(url, headers, opts), to: CodeCorps.GitHub.EagerAPI
+  @spec get_all(String.t, GitHub.headers, list) :: GitHub.response
+  def get_all(url, headers, options) do
+    {:ok, %Response{request_url: request_url, headers: response_headers}} =
+      gateway().request(:head, url, "", headers, options)
+
+    response_headers
+    |> Pagination.retrieve_total_pages()
+    |> Pagination.to_page_numbers()
+    |> Enum.map(&Pagination.add_page_param(options, &1))
+    |> Enum.map(&gateway().request(:get, url, "", headers, &1))
+    |> Enum.map(&marshall_response/1)
+    |> Enum.map(&Tuple.to_list/1)
+    |> Enum.map(&List.last/1)
+    |> List.flatten
+  end
 
   @doc """
   Get access token headers for a given `CodeCorps.User` and
@@ -47,29 +65,32 @@ defmodule CodeCorps.GitHub.API do
     end
   end
 
-  @typep http_success :: {:ok, integer, [{String.t, String.t}], String.t}
+  @typep http_success :: {:ok, Response.t}
   @typep http_failure :: {:error, term}
 
   @spec marshall_response(http_success | http_failure) :: GitHub.response
-  defp marshall_response({:ok, status, _headers, body}) when status in 200..299 do
+  defp marshall_response({:ok, %Response{body: body, status_code: status}}) when status in 200..299 do
     case body |> Poison.decode do
       {:ok, json} ->
         {:ok, json}
       {:error, _value} ->
         {:error, HTTPClientError.new(reason: :body_decoding_error)}
     end
   end
-  defp marshall_response({:ok, 404, _headers, body}) do
+  defp marshall_response({:ok, %Response{body: body, status_code: 404}}) do
     {:error, APIError.new({404, %{"message" => body}})}
   end
-  defp marshall_response({:ok, status, _headers, body}) when status in 400..599 do
+  defp marshall_response({:ok, %Response{body: body, status_code: status}}) when status in 400..599 do
     case body |> Poison.decode do
       {:ok, json} ->
         {:error, APIError.new({status, json})}
       {:error, _value} ->
         {:error, HTTPClientError.new(reason: :body_decoding_error)}
     end
   end
+  defp marshall_response({:error, %Error{reason: reason}}) do
+    {:error, HTTPClientError.new(reason: reason)}
+  end
   defp marshall_response({:error, reason}) do
     {:error, HTTPClientError.new(reason: reason)}
   end

lib/code_corps/github/api/comment.ex

@@ -32,7 +32,7 @@ defmodule CodeCorps.GitHub.API.Comment do
     attrs = comment |> GitHub.Adapters.Comment.to_api
 
     with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
-      GitHub.request(:post, endpoint, %{}, attrs, opts)
+      GitHub.request(:post, endpoint, attrs, %{}, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end
@@ -56,7 +56,7 @@ defmodule CodeCorps.GitHub.API.Comment do
     attrs = comment |> GitHub.Adapters.Comment.to_api
 
     with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
-      GitHub.request(:patch, endpoint, %{}, attrs, opts)
+      GitHub.request(:patch, endpoint, attrs, %{}, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end

lib/code_corps/github/api/eager_api.ex

@@ -0,0 +1,16 @@
+defmodule CodeCorps.GitHub.API.Gateway do
+  @moduledoc ~S"""
+  The gate through which all communication with the GitHub API must go through.
+
+  The purpose of this module is to centralize the most basic GitHub API request,
+  so the module can be injected into tests easily, giving full control to what
+  the tested response is.
+  """
+
+  alias CodeCorps.GitHub
+
+  @spec request(GitHub.method, String.t, GitHub.body, GitHub.headers, list) :: GitHub.response
+  def request(method, url, body, headers, options) do
+    HTTPoison.request(method, url, body, headers, options)
+  end
+end

lib/code_corps/github/api/gateway.ex

@@ -41,7 +41,7 @@ defmodule CodeCorps.GitHub.API.Issue do
     attrs = task |> GitHub.Adapters.Issue.to_api
 
     with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
-      GitHub.request(:post, endpoint, %{}, attrs, opts)
+      GitHub.request(:post, endpoint, attrs, %{}, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end
@@ -63,7 +63,7 @@ defmodule CodeCorps.GitHub.API.Issue do
     attrs = task |> GitHub.Adapters.Issue.to_api
 
     with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
-      GitHub.request(:patch, endpoint, %{}, attrs, opts)
+      GitHub.request(:patch, endpoint, attrs, %{}, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end

lib/code_corps/github/api/issue.ex

@@ -0,0 +1,92 @@
+defmodule CodeCorps.GitHub.API.Pagination do
+  @moduledoc ~S"""
+  Used to parse and build pagination data when fetching multiple pages from the GitHub API
+  """
+
+  @doc ~S"""
+  Parses a collection of response headers and determines the record page count for a GitHub endpoint.
+
+  The response the headers are retrieved from is usually generated using a
+  `:head` request.
+
+  The value of a "Link" header is used to determine the page count.
+
+  If the "Link" header is not present in the collection, the count is assumed 1
+
+  If the "Link" header is present, we use regex to parse the pagination info
+  from its value.
+
+  The format of the header is as follows:
+
+  ```
+  {"Link", '<https://api.github.com/search/code?q=addClass+user%3Amozilla&page=15>; rel="next",
+            <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=34>; rel="last",
+            <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=1>; rel="first",
+            <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=13>; rel="prev"'
+  ```
+
+  The page count is determind by locating the `rel="last"` url and extracting
+  the `page` query parameter from it.
+  """
+  @spec retrieve_total_pages(list) :: integer
+  def retrieve_total_pages(headers) do
+    headers
+    |> List.keyfind("Link", 0, nil)
+    |> extract_total_pages
+  end
+
+  @spec extract_total_pages(nil | String.t) :: integer
+  defp extract_total_pages(nil), do: 1
+  defp extract_total_pages({"Link", value} = _header) do
+    value
+    |> String.split(", ")
+    |> Enum.map(fn link ->
+      rel = get_rel(link)
+      page = get_page(link)
+      {rel, page}
+    end)
+    |> Enum.into(%{})
+    |> Map.get("last")
+  end
+
+  @spec get_rel(String.t) :: String.t
+  defp get_rel(link) when is_binary(link) do
+    # Searches for `rel=`
+    Regex.run(~r{rel="([a-z]+)"}, link) |> List.last()
+  end
+
+  @spec get_page(String.t) :: integer
+  defp get_page(link) when is_binary(link) do
+    # Searches for the following variations:
+    # ```
+    # ?page={match}>
+    # ?page={match}&...
+    # &page={match}>
+    # &page={match}&...
+    # ```
+    Regex.run(~r{[&/?]page=([^>&]+)}, link) |> List.last |> String.to_integer
+  end
+
+  @doc ~S"""
+  From the specified page count, generates a list of integers, `1..count`
+  """
+  @spec to_page_numbers(integer) :: list(integer)
+  def to_page_numbers(total) when is_integer(total), do: 1..total
+
+  @doc ~S"""
+  Adds a `page` query parameter to an `options` `Keyword` list.
+
+  For `HTTPPoison`, query parameters go under the `params` key of the `options`
+  argument, so this function also ensures the `params` key is present.
+  """
+  @spec add_page_param(Keyword.t, integer) :: Keyword.t
+  def add_page_param(options, page) when is_list(options) when is_integer(page) do
+    params =
+      options
+      |> Keyword.get(:params, [])
+      |> Keyword.put(:page, page)
+
+    options
+    |> Keyword.put(:params, params)
+  end
+end

lib/code_corps/github/api/pagination.ex

@@ -4,7 +4,7 @@ defmodule CodeCorps.GitHub.API.User do
   """
 
   alias CodeCorps.{Accounts, GitHub, User}
-  alias Ecto.{Changeset}
+  alias Ecto.Changeset
 
 
   @single_endpoint "user"

lib/code_corps/github/api/user.ex

@@ -12,7 +12,7 @@ defmodule CodeCorps.GitHub.Event.Installation.MatchedUser do
     User
   }
 
-  @typep process_outcome :: {:ok, GithubAppInstallation.t} | {:error, Changeset.t}
+  @typep process_outcome :: {:ok, GithubAppInstallation.t} | {:error, Ecto.Changeset.t}
   @type outcome :: process_outcome | {:error, :too_many_unprocessed_installations}
 
   @doc """