diff --git a/config/config.exs b/config/config.exs index 44c1ffbb7..8c4468814 100644 --- a/config/config.exs +++ b/config/config.exs @@ -68,6 +68,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_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"), diff --git a/config/test.exs b/config/test.exs index 8e77f6fc0..a50964ae2 100644 --- a/config/test.exs +++ b/config/test.exs @@ -61,4 +61,6 @@ pem = case System.get_env("GITHUB_APP_PEM") do encoded_pem -> encoded_pem |> Base.decode64! end -config :code_corps, github_app_pem: pem +config :code_corps, + github: CodeCorps.GitHub.SuccessAPI, + github_app_pem: pem diff --git a/lib/code_corps/github.ex b/lib/code_corps/github.ex deleted file mode 100644 index dd9a8887b..000000000 --- a/lib/code_corps/github.ex +++ /dev/null @@ -1,228 +0,0 @@ -defmodule CodeCorps.GitHub do - - alias CodeCorps.Github.JWT - - @client_id Application.get_env(:code_corps, :github_app_client_id) - @client_secret Application.get_env(:code_corps, :github_app_client_secret) - - @base_access_token_params %{ - client_id: @client_id, - client_secret: @client_secret - } - - defmodule APIErrorObject do - @moduledoc """ - Represents an error object from the GitHub API. - - Used in some `APIError`s when the API's JSON response contains an - `errors` key. - - The full details of error objects can be found in the - [GitHub API documentation](https://developer.github.com/v3/#client-errors). - """ - - @type t :: %__MODULE__{} - - defstruct [:code, :field, :resource] - - def new(opts) do - struct(__MODULE__, opts) - end - end - - defmodule APIError do - @moduledoc """ - Represents a client error from the GitHub API. - - You can read more about client errors in the - [GitHub API documentation](https://developer.github.com/v3/#client-errors). - """ - - defstruct [:documentation_url, :errors, :message, :status_code] - - @type t :: %__MODULE__{ - documentation_url: String.t | nil, - errors: list | nil, - message: String.t | nil, - status_code: pos_integer | nil - } - - @spec new({integer, map}) :: t - def new({status_code, %{"message" => message, "errors" => errors}}) do - errors = Enum.into(errors, [], fn error -> convert_error(error) end) - - %__MODULE__{ - errors: errors, - message: message, - status_code: status_code - } - end - def new({status_code, %{"message" => message, "documentation_url" => documentation_url}}) do - %__MODULE__{ - documentation_url: documentation_url, - message: message, - status_code: status_code - } - end - def new({status_code, %{"message" => message}}) do - %__MODULE__{ - message: message, - status_code: status_code - } - end - - @spec convert_error(map) :: APIErrorObject.t - defp convert_error(%{"code" => code, "field" => field, "resource" => resource}) do - APIErrorObject.new([code: code, field: field, resource: resource]) - end - end - - defmodule HTTPClientError do - defstruct [:reason, message: """ - The GitHub HTTP client encountered an error while communicating with - the GitHub API. - """] - - @type t :: %__MODULE__{} - - def new(opts) do - struct(__MODULE__, opts) - end - end - - @type method :: :get | :post | :put | :delete | :patch - @type headers :: %{String.t => String.t} | %{} - @type body :: {:multipart, list} | map - @typep http_success :: {:ok, integer, [{String.t, String.t}], String.t} - @typep http_failure :: {:error, term} - - @type api_error_struct :: APIError.t | HTTPClientError.t | Poison.DecodeError.t - - @spec get_base_url() :: String.t - defp get_base_url() do - Application.get_env(:code_corps, :github_base_url) || "https://api.github.com/" - end - - @spec get_token_url() :: String.t - defp get_token_url() do - Application.get_env(:code_corps, :github_oauth_url) || "https://github.com/login/oauth/access_token" - end - - @spec use_pool?() :: boolean - defp use_pool?() do - Application.get_env(:code_corps, :github_api_use_connection_pool) - end - - @spec add_default_headers(headers) :: headers - defp add_default_headers(existing_headers) do - Map.merge(%{"Accept" => "application/vnd.github.machine-man-preview+json"}, existing_headers) - end - - @spec add_access_token_header(headers, String.t | nil) :: headers - defp add_access_token_header(existing_headers, nil), do: existing_headers - defp add_access_token_header(existing_headers, access_token) do - Map.put(existing_headers, "Authorization", "token #{access_token}") - end - - @spec add_jwt_header(headers) :: headers - defp add_jwt_header(existing_headers) do - Map.put(existing_headers, "Authorization", "Bearer #{JWT.generate}") - end - - @spec add_default_options(list) :: list - defp add_default_options(opts) do - [:with_body | opts] - end - - @spec build_access_token_params(String.t, String.t) :: map - defp build_access_token_params(code, state) do - @base_access_token_params - |> Map.put(:code, code) - |> Map.put(:state, state) - end - - @doc """ - A low level utility function to make a direct request to the GitHub API. - """ - @spec request(body, method, String.t, headers, list) :: {:ok, map} | {:error, api_error_struct} - def request(body, method, endpoint, headers, opts) do - {access_token, opts} = Keyword.pop(opts, :access_token) - - base_url = get_base_url() - req_url = base_url <> endpoint - req_body = body |> Poison.encode! - req_headers = - headers - |> add_default_headers() - |> add_access_token_header(access_token) - |> Map.to_list() - - req_opts = - opts - |> add_default_options() - - method - |> :hackney.request(req_url, req_headers, req_body, req_opts) - |> handle_response() - end - - @doc """ - A low level utility function to fetch a GitHub user's OAuth access token - """ - @spec user_access_token_request(String.t, String.t) :: {:ok, map} | {:error, api_error_struct} - def user_access_token_request(code, state) do - req_url = get_token_url() - req_body = code |> build_access_token_params(state) |> Poison.encode! - req_headers = - %{"Accept" => "application/json", "Content-Type" => "application/json"} - |> add_default_headers() - |> Map.to_list() - - req_opts = - [] - |> add_default_options() - - :hackney.request(:post, req_url, req_headers, req_body, req_opts) - |> handle_response() - end - - @doc """ - A low level utility function to make an authenticated request to the - GitHub API on behalf of a GitHub App or integration - """ - @spec authenticated_integration_request(body, method, String.t, headers, list) :: {:ok, map} | {:error, api_error_struct} - def authenticated_integration_request(body, method, endpoint, headers, opts) do - base_url = get_base_url() - req_url = base_url <> endpoint - req_body = body |> Poison.encode! - req_headers = - headers - |> add_default_headers() - |> add_jwt_header() - |> Map.to_list() - - req_opts = - opts - |> add_default_options() - - :hackney.request(method, req_url, req_headers, req_body, req_opts) - |> handle_response() - end - - @spec handle_response(http_success | http_failure) :: {:ok, map} | {:error, api_error_struct} - defp handle_response({:ok, status, _headers, body}) when status in 200..299 do - body |> Poison.decode - end - defp handle_response({:ok, 404, _headers, body}) do - {:error, APIError.new({404, %{"message" => body}})} - end - defp handle_response({:ok, status, _headers, body}) when status in 400..599 do - case body |> Poison.decode do - {:ok, json} -> {:error, APIError.new({status, json})} - {:error, error} -> {:error, error} - end - end - defp handle_response({:error, reason}) do - {:error, HTTPClientError.new(reason: reason)} - end -end diff --git a/lib/code_corps/github/api.ex b/lib/code_corps/github/api.ex new file mode 100644 index 000000000..986812749 --- /dev/null +++ b/lib/code_corps/github/api.ex @@ -0,0 +1,41 @@ +defmodule CodeCorps.GitHub.API do + alias CodeCorps.{ + GitHub, + GitHub.APIError, + GitHub.HTTPClientError + } + + @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) + |> marshall_response() + end + + @typep http_success :: {:ok, integer, [{String.t, String.t}], String.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 + case body |> Poison.decode do + {:ok, json} -> + {:ok, json} + {:error, {_decoding_error, _decoding_value}} -> + marshall_response({:error, :body_decoding_error}) + end + end + defp marshall_response({:ok, 404, _headers, body}) do + {:error, APIError.new({404, %{"message" => body}})} + end + defp marshall_response({:ok, status, _headers, body}) when status in 400..599 do + case body |> Poison.decode do + {:ok, json} -> + {:error, APIError.new({status, json})} + {:error, {_decoding_error, _decoding_value}} -> + marshall_response({:error, :body_decoding_error}) + end + end + defp marshall_response({:error, reason}) do + {:error, HTTPClientError.new(reason: reason)} + end +end diff --git a/lib/code_corps/github/bypass.ex b/lib/code_corps/github/bypass.ex deleted file mode 100644 index 697d123fe..000000000 --- a/lib/code_corps/github/bypass.ex +++ /dev/null @@ -1,12 +0,0 @@ -defmodule CodeCorps.GitHub.Bypass do - - def setup(bypass) do - Application.put_env(:code_corps, :github_oauth_url, "http://localhost:#{bypass.port}") - Application.put_env(:code_corps, :github_base_url, "http://localhost:#{bypass.port}/") - end - - def teardown do - Application.delete_env(:code_corps, :github_base_url) - Application.delete_env(:code_corps, :github_oauth_url) - end -end diff --git a/lib/code_corps/github/github.ex b/lib/code_corps/github/github.ex new file mode 100644 index 000000000..924e19287 --- /dev/null +++ b/lib/code_corps/github/github.ex @@ -0,0 +1,166 @@ +defmodule CodeCorps.GitHub do + alias CodeCorps.GitHub.Headers + + @client_id Application.get_env(:code_corps, :github_app_client_id) + @client_secret Application.get_env(:code_corps, :github_app_client_secret) + + @base_access_token_params %{ + client_id: @client_id, + client_secret: @client_secret + } + + defmodule APIErrorObject do + @moduledoc """ + Represents an error object from the GitHub API. + + Used in some `APIError`s when the API's JSON response contains an + `errors` key. + + The full details of error objects can be found in the + [GitHub API documentation](https://developer.github.com/v3/#client-errors). + """ + + @type t :: %__MODULE__{} + + defstruct [:code, :field, :resource] + + def new(opts) do + struct(__MODULE__, opts) + end + end + + defmodule APIError do + @moduledoc """ + Represents a client error from the GitHub API. + + You can read more about client errors in the + [GitHub API documentation](https://developer.github.com/v3/#client-errors). + """ + + defstruct [:documentation_url, :errors, :message, :status_code] + + @type t :: %__MODULE__{ + documentation_url: String.t | nil, + errors: list | nil, + message: String.t | nil, + status_code: pos_integer | nil + } + + @spec new({integer, map}) :: t + def new({status_code, %{"message" => message, "errors" => errors}}) do + errors = Enum.into(errors, [], fn error -> convert_error(error) end) + + %__MODULE__{ + errors: errors, + message: message, + status_code: status_code + } + end + def new({status_code, %{"message" => message, "documentation_url" => documentation_url}}) do + %__MODULE__{ + documentation_url: documentation_url, + message: message, + status_code: status_code + } + end + def new({status_code, %{"message" => message}}) do + %__MODULE__{ + message: message, + status_code: status_code + } + end + + @spec convert_error(map) :: APIErrorObject.t + defp convert_error(%{"code" => code, "field" => field, "resource" => resource}) do + APIErrorObject.new([code: code, field: field, resource: resource]) + end + end + + defmodule HTTPClientError do + defstruct [:reason, message: """ + The GitHub HTTP client encountered an error while communicating with + the GitHub API. + """] + + @type t :: %__MODULE__{} + + def new(opts) do + struct(__MODULE__, opts) + end + end + + + @type method :: :get | :post | :put | :delete | :patch + + @type body :: {:multipart, list} | map + @type headers :: %{String.t => String.t} | %{} + @type response :: {:ok, map} | {:error, api_error_struct} + @type api_error_struct :: APIError.t | HTTPClientError.t + + @doc """ + A low level utility function to make a direct request to the GitHub API. + """ + @spec request(method, String.t, headers, body, list) :: response + def request(method, endpoint, headers, body, options) do + api().request( + method, + api_url_for(endpoint), + headers |> Headers.user_request(options), + body |> Poison.encode!, + options |> add_default_options() + ) + end + + @doc """ + A low level utility function to make an authenticated request to the + GitHub API on behalf of a GitHub App or integration + """ + @spec integration_request(method, String.t, headers, body, list) :: response + def integration_request(method, endpoint, headers, body, options) do + api().request( + method, + api_url_for(endpoint), + headers |> Headers.integration_request, + body |> Poison.encode!, + options |> add_default_options() + ) + end + + @token_url "https://github.com/login/oauth/access_token" + + @doc """ + A low level utility function to fetch a GitHub user's OAuth access token + """ + @spec user_access_token_request(String.t, String.t) :: response + def user_access_token_request(code, state) do + api().request( + :post, + @token_url, + Headers.access_token_request, + code |> build_access_token_params(state) |> Poison.encode!, + [] |> add_default_options() + ) + end + + @spec api :: module + defp api, do: Application.get_env(:code_corps, :github) + + @api_url "https://api.github.com/" + + @spec api_url_for(String.t) :: String.t + defp api_url_for(endpoint) when is_binary(endpoint) do + @api_url |> URI.merge(endpoint) |> URI.to_string + end + + @spec add_default_options(list) :: list + defp add_default_options(opts) do + [:with_body | opts] + end + + @spec build_access_token_params(String.t, String.t) :: map + defp build_access_token_params(code, state) do + @base_access_token_params + |> Map.put(:code, code) + |> Map.put(:state, state) + end +end diff --git a/lib/code_corps/github/headers.ex b/lib/code_corps/github/headers.ex new file mode 100644 index 000000000..d8e8d6ec9 --- /dev/null +++ b/lib/code_corps/github/headers.ex @@ -0,0 +1,46 @@ +defmodule CodeCorps.GitHub.Headers do + alias CodeCorps.Github.JWT + + @typep header :: {String.t, String.t} + @type t :: list(header) + + @spec user_request(%{String.t => String.t} | %{}, list) :: t + def user_request(%{} = headers, options) do + headers + |> add_default_headers() + |> add_access_token_header(options) + |> Map.to_list() + end + + @spec integration_request(%{String.t => String.t} | %{}) :: t + def integration_request(%{} = headers) do + headers + |> add_default_headers() + |> add_jwt_header() + |> Map.to_list() + end + + @spec access_token_request :: t + def access_token_request do + %{"Accept" => "application/json", "Content-Type" => "application/json"} + |> add_default_headers() + |> Map.to_list() + end + + @spec add_default_headers(%{String.t => String.t}) :: %{String.t => String.t} + defp add_default_headers(%{} = headers) do + Map.merge(%{"Accept" => "application/vnd.github.machine-man-preview+json"}, headers) + end + + @spec add_access_token_header(%{String.t => String.t}, list) :: %{String.t => String.t} + defp add_access_token_header(%{} = headers, [access_token: nil]), do: headers + defp add_access_token_header(%{} = headers, [access_token: access_token]) do + Map.put(headers, "Authorization", "token #{access_token}") + end + defp add_access_token_header(headers, []), do: headers + + @spec add_jwt_header(%{String.t => String.t}) :: %{String.t => String.t} + defp add_jwt_header(%{} = headers) do + Map.put(headers, "Authorization", "Bearer #{JWT.generate}") + end +end diff --git a/lib/code_corps/github/installation.ex b/lib/code_corps/github/installation.ex index cbbc2afcb..811e021ec 100644 --- a/lib/code_corps/github/installation.ex +++ b/lib/code_corps/github/installation.ex @@ -61,7 +61,7 @@ defmodule CodeCorps.GitHub.Installation do def refresh_token(%GithubAppInstallation{github_id: installation_id} = installation) do endpoint = "installations/#{installation_id}/access_tokens" with {:ok, %{"token" => token, "expires_at" => expires_at}} <- - GitHub.authenticated_integration_request(%{}, :post, endpoint, %{}, []), + GitHub.integration_request(:post, endpoint, %{}, %{}, []), {:ok, %GithubAppInstallation{}} <- update_token(installation, token, expires_at) do diff --git a/lib/code_corps/github/request.ex b/lib/code_corps/github/request.ex index 3f5ccb4a2..5a92dfc49 100644 --- a/lib/code_corps/github/request.ex +++ b/lib/code_corps/github/request.ex @@ -1,13 +1,6 @@ defmodule CodeCorps.GitHub.Request do - alias CodeCorps.GitHub - @spec retrieve(String.t, Keyword.t) :: {:ok, map} | {:error, GitHub.api_error_struct} - def retrieve(endpoint, opts), do: retrieve(%{}, endpoint, opts) - - @spec retrieve(map, String.t, Keyword.t) :: {:ok, map} | {:error, GitHub.api_error_struct} - def retrieve(changes, endpoint, opts) do - changes - |> GitHub.request(:get, endpoint, %{}, opts) - end + @spec retrieve(String.t, Keyword.t) :: GitHub.response + def retrieve(endpoint, opts), do: GitHub.request(:get, endpoint, %{}, %{}, opts) end diff --git a/lib/code_corps/github/user.ex b/lib/code_corps/github/user.ex index a21a1ec86..4a4938b63 100644 --- a/lib/code_corps/github/user.ex +++ b/lib/code_corps/github/user.ex @@ -78,8 +78,8 @@ defmodule CodeCorps.GitHub.User do @spec me(String.t, Keyword.t) :: {:ok, map} | {:error, GitHub.api_error_struct} def me(access_token, opts \\ []) do case GitHub.Request.retrieve(@single_endpoint, opts ++ [access_token: access_token]) do - {:ok, %{"error" => error}} -> {:error, error} - {:ok, %{} = user_payload} -> {:ok, user_payload} + {:ok, response} -> {:ok, response} + {:error, error} -> {:error, error} end end end diff --git a/mix.exs b/mix.exs index 90fa75c4d..926712b91 100644 --- a/mix.exs +++ b/mix.exs @@ -48,7 +48,7 @@ defmodule CodeCorps.Mixfile do {:gettext, "~> 0.12"}, {:cowboy, "~> 1.0"}, {:benchfella, "~> 0.3.0", only: :dev}, - {:bypass, "~> 0.6", only: :test}, + {:bypass, "~> 0.8.1", only: :test}, {:canary, "~> 1.1"}, # Authorization {:cloudex, "~> 0.1.10"}, {:comeonin, "~> 2.0"}, diff --git a/mix.lock b/mix.lock index d92e922f2..9455c281b 100644 --- a/mix.lock +++ b/mix.lock @@ -3,7 +3,7 @@ "base64url": {:hex, :base64url, "0.0.1", "36a90125f5948e3afd7be97662a1504b934dd5dac78451ca6e9abf85a10286be", [:rebar], []}, "benchfella": {:hex, :benchfella, "0.3.4", "41d2c017b361ece5225b5ba2e3b30ae53578c57c6ebc434417b4f1c2c94cf4f3", [:mix], []}, "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []}, - "bypass": {:hex, :bypass, "0.6.0", "fd0a8004fada4464e2ba98497755310b892a097f2fd975f4f787cf264066a335", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, optional: false]}, {:plug, "~> 1.0", [hex: :plug, optional: false]}]}, + "bypass": {:hex, :bypass, "0.8.1", "16d409e05530ece4a72fabcf021a3e5c7e15dcc77f911423196a0c551f2a15ca", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, optional: false]}, {:plug, "~> 1.0", [hex: :plug, optional: false]}]}, "canada": {:hex, :canada, "1.0.1", "da96d0ff101a0c2a6cc7b07d92b8884ff6508f058781d3679999416feacf41c5", [:mix], []}, "canary": {:hex, :canary, "1.1.0", "3599012f5393c2fdb18c9129853a9fc6cd115ebfdcc0725af7b52398b9ed5c7b", [:mix], [{:canada, "~> 1.0.0", [hex: :canada, optional: false]}, {:ecto, ">= 1.1.0", [hex: :ecto, optional: false]}, {:plug, "~> 1.0", [hex: :plug, optional: false]}]}, "certifi": {:hex, :certifi, "0.7.0", "861a57f3808f7eb0c2d1802afeaae0fa5de813b0df0979153cbafcd853ababaf", [:rebar3], []}, diff --git a/test/lib/code_corps/github/adapters/comment_test.exs b/test/lib/code_corps/github/adapters/comment_test.exs index e02fc1fd8..fb7e1c31d 100644 --- a/test/lib/code_corps/github/adapters/comment_test.exs +++ b/test/lib/code_corps/github/adapters/comment_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Adapters.CommentTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Adapters.Comment diff --git a/test/lib/code_corps/github/adapters/github_app_installation_test.exs b/test/lib/code_corps/github/adapters/github_app_installation_test.exs index 18c4c6dc6..45d14e442 100644 --- a/test/lib/code_corps/github/adapters/github_app_installation_test.exs +++ b/test/lib/code_corps/github/adapters/github_app_installation_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Adapters.GithubAppInstallationTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Adapters.GithubAppInstallation diff --git a/test/lib/code_corps/github/adapters/github_repo_test.exs b/test/lib/code_corps/github/adapters/github_repo_test.exs index 4638a4b7b..0f4a8fd88 100644 --- a/test/lib/code_corps/github/adapters/github_repo_test.exs +++ b/test/lib/code_corps/github/adapters/github_repo_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Adapters.GithubRepoTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Adapters.GithubRepo diff --git a/test/lib/code_corps/github/adapters/task_test.exs b/test/lib/code_corps/github/adapters/task_test.exs index 2aa3fc9f1..719082223 100644 --- a/test/lib/code_corps/github/adapters/task_test.exs +++ b/test/lib/code_corps/github/adapters/task_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Adapters.TaskTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Adapters.Task diff --git a/test/lib/code_corps/github/adapters/user_test.exs b/test/lib/code_corps/github/adapters/user_test.exs index ba57dc376..f36843944 100644 --- a/test/lib/code_corps/github/adapters/user_test.exs +++ b/test/lib/code_corps/github/adapters/user_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Adapters.UserTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Adapters.User diff --git a/test/lib/code_corps/github/api_test.exs b/test/lib/code_corps/github/api_test.exs new file mode 100644 index 000000000..e3af6a578 --- /dev/null +++ b/test/lib/code_corps/github/api_test.exs @@ -0,0 +1,90 @@ +defmodule CodeCorps.GitHub.APITest do + @moduledoc false + + use ExUnit.Case + + alias CodeCorps.GitHub.{API, APIError, HTTPClientError} + alias Plug.Conn + + @port 12345 + @endpoint "http://localhost" |> URI.merge("") |> Map.put(:port, @port) |> URI.to_string + @body %{"bar" => "baz"} |> Poison.encode! + + defp url_for(path) do + URI.merge(@endpoint, path) |> URI.to_string + end + + setup do + bypass = Bypass.open(port: @port) + {:ok, bypass: bypass} + end + + describe "request/5" do + test "handles a 200..299 response", %{bypass: bypass} do + Bypass.expect(bypass, "GET", "/foo", fn %Conn{req_headers: headers} = conn -> + assert {"foo", "bar"} in headers + conn |> Conn.resp(200, @body) + end) + + {:ok, response} = API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body]) + assert response == (@body |> Poison.decode!) + end + + test "handles a decode error for a 200..299 response", %{bypass: bypass} do + Bypass.expect(bypass, "GET", "/foo", fn %Conn{req_headers: headers} = conn -> + assert {"foo", "bar"} in headers + conn |> Conn.resp(200, "foo") + end) + + {:error, response} = API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body]) + assert response == HTTPClientError.new([reason: :body_decoding_error]) + end + + test "handles a 404 response", %{bypass: bypass} do + Bypass.expect(bypass, "GET", "/foo", fn %Conn{req_headers: headers} = conn -> + assert {"foo", "bar"} in headers + conn |> Conn.resp(404, @body) + end) + + {:error, response} = API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body]) + assert response == APIError.new({404, %{"message" => @body}}) + end + + @generic_error %{"message" => "foo"} |> Poison.encode! + + test "handles a 400 response", %{bypass: bypass} do + Bypass.expect(bypass, "GET", "/foo", fn %Conn{req_headers: headers} = conn -> + assert {"foo", "bar"} in headers + conn |> Conn.resp(400, @generic_error) + end) + + {:error, response} = API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body]) + assert response == APIError.new({400, @generic_error |> Poison.decode!}) + end + + test "handles a decode error for a 400..599 response", %{bypass: bypass} do + Bypass.expect(bypass, "GET", "/foo", fn %Conn{req_headers: headers} = conn -> + assert {"foo", "bar"} in headers + conn |> Conn.resp(400, "foo") + end) + + {:error, response} = API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body]) + assert response == HTTPClientError.new([reason: :body_decoding_error]) + end + + test "handles a network error", %{bypass: bypass} do + bypass |> Bypass.down + # bypass is simulating a connection timeout error, so we need to set a + # small value for hackney's request to avoid test hanging + {:error, %HTTPClientError{reason: reason}} = + API.request(:get, url_for("/foo"), [{"foo", "bar"}], @body, [:with_body, connect_timeout: 15]) + bypass |> Bypass.up + + # bypass wil localy throw a :connect_timeout when down, + # but it will be an _econnrefused on circle + # we just care that a client error is handled by creating the + # proper struct + assert reason in [:connect_timeout, :econnrefused] + end + end +end diff --git a/test/lib/code_corps/github/event/common/repo_finder_test.exs b/test/lib/code_corps/github/event/common/repo_finder_test.exs index 908ee60f5..f8c32b5a8 100644 --- a/test/lib/code_corps/github/event/common/repo_finder_test.exs +++ b/test/lib/code_corps/github/event/common/repo_finder_test.exs @@ -1,7 +1,7 @@ defmodule CodeCorps.GitHub.Event.Common.RepoFinderTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Event.Common.RepoFinder diff --git a/test/lib/code_corps/github/event/installation/changeset_builder_test.exs b/test/lib/code_corps/github/event/installation/changeset_builder_test.exs index 196db3aa2..f0ba6f1a3 100644 --- a/test/lib/code_corps/github/event/installation/changeset_builder_test.exs +++ b/test/lib/code_corps/github/event/installation/changeset_builder_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Installation.ChangesetBuilderTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers import Ecto.Changeset alias CodeCorps.{ diff --git a/test/lib/code_corps/github/event/installation/repos_test.exs b/test/lib/code_corps/github/event/installation/repos_test.exs index 2fca216a5..08efe0c04 100644 --- a/test/lib/code_corps/github/event/installation/repos_test.exs +++ b/test/lib/code_corps/github/event/installation/repos_test.exs @@ -2,10 +2,8 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do @moduledoc false use CodeCorps.DbAccessCase - use CodeCorps.GitHubCase - import CodeCorps.Factories - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GithubAppInstallation, @@ -16,20 +14,34 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do alias CodeCorps.GitHub.Adapters.GithubRepo, as: GithubRepoAdapter - @access_token "v1.1f699f1069f60xxx" - @expires_at Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601() - @access_token_create_response %{"token" => @access_token, "expires_at" => @expires_at} + defmodule BadRepoRequest do + def request(:get, "https://api.github.com/installation/repositories", _, _, _) do + body = load_endpoint_fixture("forbidden") + {:error, CodeCorps.GitHub.APIError.new({404, %{"message" => body}})} + end + def request(method, endpoint, headers, body, options) do + CodeCorps.GitHub.SuccessAPI.request(method, endpoint, headers, body, options) + end + end - @installation_repositories load_endpoint_fixture("installation_repositories") - @forbidden load_endpoint_fixture("forbidden") + defmodule InvalidRepoRequest do + def request(:get, "https://api.github.com/installation/repositories", _, _, _) do + payload = + "installation_repositories" + |> load_endpoint_fixture + |> Map.put("repositories", [%{}]) + {:ok, payload} + end + def request(method, endpoint, headers, body, options) do + CodeCorps.GitHub.SuccessAPI.request(method, endpoint, headers, body, options) + end + end + # from fixture + @installation_repositories load_endpoint_fixture("installation_repositories") @app_github_id 2 describe "process_async/1" do - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@app_github_id}/access_tokens" => {200, @access_token_create_response} - } test "syncs repos by performing a diff using payload as master list, asynchronously" do installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps") @@ -66,10 +78,6 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do end describe "process/1" do - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@app_github_id}/access_tokens" => {200, @access_token_create_response} - } test "syncs repos by performing a diff using payload as master list" do installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps") @@ -93,25 +101,23 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do assert GithubRepo |> Repo.aggregate(:count, :id) == 2 end - @tag bypass: %{ - "/installation/repositories" => {403, @forbidden}, - "/installations/#{@app_github_id}/access_tokens" => {200, @access_token_create_response} - } test "returns installation as errored if api error" do installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps") - {:error, %GithubAppInstallation{state: state}, %CodeCorps.GitHub.APIError{}} = Repos.process(installation) + + with_mock_api(BadRepoRequest) do + {:error, %GithubAppInstallation{state: state}, %CodeCorps.GitHub.APIError{}} = Repos.process(installation) + end + assert state == "errored" end - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories |> Map.put("repositories", [%{}])}, - "/installations/#{@app_github_id}/access_tokens" => {200, @access_token_create_response} - } test "returns installation as errored if error creating repos" do installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps") - {:error, %GithubAppInstallation{state: state}, _changesets} - = installation |> Repo.preload(:github_repos) |> Repos.process() + with_mock_api(InvalidRepoRequest) do + {:error, %GithubAppInstallation{state: state}, _changesets} + = installation |> Repo.preload(:github_repos) |> Repos.process() + end assert state == "errored" end diff --git a/test/lib/code_corps/github/event/installation/validator_test.exs b/test/lib/code_corps/github/event/installation/validator_test.exs index 142719918..4057193f9 100644 --- a/test/lib/code_corps/github/event/installation/validator_test.exs +++ b/test/lib/code_corps/github/event/installation/validator_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Installation.ValidatorTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Event.Installation.Validator diff --git a/test/lib/code_corps/github/event/installation_repositories_test.exs b/test/lib/code_corps/github/event/installation_repositories_test.exs index 520ff3fd8..594fa8f20 100644 --- a/test/lib/code_corps/github/event/installation_repositories_test.exs +++ b/test/lib/code_corps/github/event/installation_repositories_test.exs @@ -1,15 +1,9 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do @moduledoc false - use CodeCorps.{ - DbAccessCase, - GitHubCase - } + use CodeCorps.DbAccessCase - import CodeCorps.{ - Factories, - TestHelpers.GitHub - } + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GithubRepo, diff --git a/test/lib/code_corps/github/event/installation_test.exs b/test/lib/code_corps/github/event/installation_test.exs index 3299838de..1271dd604 100644 --- a/test/lib/code_corps/github/event/installation_test.exs +++ b/test/lib/code_corps/github/event/installation_test.exs @@ -1,9 +1,9 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do @moduledoc false - use CodeCorps.{DbAccessCase, GitHubCase} + use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GithubAppInstallation, @@ -12,12 +12,7 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do Repo } - @access_token "v1.1f699f1069f60xxx" - @expires_at Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601() - @access_token_create_response %{"token" => @access_token, "expires_at" => @expires_at} - @installation_created load_event_fixture("installation_created") - @installation_repositories load_endpoint_fixture("installation_repositories") describe "handle/2" do test "returns error if payload is wrong" do @@ -42,10 +37,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do end describe "handle/2 for Installation::created" do - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_created["installation"]["id"]}/access_tokens" => {200, @access_token_create_response} - } test "creates installation for unmatched user if no user, syncs repos" do payload = %{"installation" => %{"id" => installation_github_id}} = @installation_created event = build(:github_event, action: "created", type: "installation") @@ -65,10 +56,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do assert Repo.one(GithubAppInstallation).state == "processed" end - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_created["installation"]["id"]}/access_tokens" => {200, @access_token_create_response} - } test "creates installation if user matched but installation unmatched, syncs repos" do %{"sender" => %{"id" => user_github_id}} = payload = @installation_created event = build(:github_event, action: "created", type: "installation") @@ -90,10 +77,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do assert Repo.one(GithubAppInstallation).state == "processed" end - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_created["installation"]["id"]}/access_tokens" => {200, @access_token_create_response} - } test "updates installation, if both user and installation matched, syncs repos" do %{"sender" => %{"id" => user_github_id}, "installation" => %{"id" => installation_github_id}} = payload = @installation_created event = build(:github_event, action: "created", type: "installation") @@ -120,10 +103,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do assert Repo.one(GithubAppInstallation).state == "processed" end - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_created["installation"]["id"]}/access_tokens" => {200, @access_token_create_response} - } test "updates installation if there is an installation, but no user, syncs repos" do %{"installation" => %{"id" => installation_github_id}, "sender" => %{"id" => sender_github_id}} = payload = @installation_created insert(:github_app_installation, github_id: installation_github_id) diff --git a/test/lib/code_corps/github/event/issue_comment/changeset_builder_test.exs b/test/lib/code_corps/github/event/issue_comment/changeset_builder_test.exs index f9725965b..863890540 100644 --- a/test/lib/code_corps/github/event/issue_comment/changeset_builder_test.exs +++ b/test/lib/code_corps/github/event/issue_comment/changeset_builder_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.ChangesetBuilderTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers import Ecto.Changeset alias CodeCorps.{ diff --git a/test/lib/code_corps/github/event/issue_comment/comment_deleter_test.exs b/test/lib/code_corps/github/event/issue_comment/comment_deleter_test.exs index cfdfb95f5..8205d106d 100644 --- a/test/lib/code_corps/github/event/issue_comment/comment_deleter_test.exs +++ b/test/lib/code_corps/github/event/issue_comment/comment_deleter_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.CommentDeleterTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.IssueComment.CommentDeleter, diff --git a/test/lib/code_corps/github/event/issue_comment/comment_syncer_test.exs b/test/lib/code_corps/github/event/issue_comment/comment_syncer_test.exs index 2246c46da..19ba806d7 100644 --- a/test/lib/code_corps/github/event/issue_comment/comment_syncer_test.exs +++ b/test/lib/code_corps/github/event/issue_comment/comment_syncer_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.CommentSyncerTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.IssueComment.CommentSyncer, diff --git a/test/lib/code_corps/github/event/issue_comment/user_linker_test.exs b/test/lib/code_corps/github/event/issue_comment/user_linker_test.exs index ecce12596..d20ab5651 100644 --- a/test/lib/code_corps/github/event/issue_comment/user_linker_test.exs +++ b/test/lib/code_corps/github/event/issue_comment/user_linker_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.UserLinkerTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.IssueComment.UserLinker, diff --git a/test/lib/code_corps/github/event/issue_comment/validator_test.exs b/test/lib/code_corps/github/event/issue_comment/validator_test.exs index d6b69f043..ad7892949 100644 --- a/test/lib/code_corps/github/event/issue_comment/validator_test.exs +++ b/test/lib/code_corps/github/event/issue_comment/validator_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.ValidatorTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Event.IssueComment.Validator diff --git a/test/lib/code_corps/github/event/issue_comment_test.exs b/test/lib/code_corps/github/event/issue_comment_test.exs index 5aae1450a..411738d55 100644 --- a/test/lib/code_corps/github/event/issue_comment_test.exs +++ b/test/lib/code_corps/github/event/issue_comment_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ Comment, diff --git a/test/lib/code_corps/github/event/issues/changeset_builder_test.exs b/test/lib/code_corps/github/event/issues/changeset_builder_test.exs index 2e254321b..c1accf050 100644 --- a/test/lib/code_corps/github/event/issues/changeset_builder_test.exs +++ b/test/lib/code_corps/github/event/issues/changeset_builder_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Issues.ChangesetBuilderTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers import Ecto.Changeset alias CodeCorps.{ diff --git a/test/lib/code_corps/github/event/issues/task_syncer_test.exs b/test/lib/code_corps/github/event/issues/task_syncer_test.exs index 32c99f289..c17204054 100644 --- a/test/lib/code_corps/github/event/issues/task_syncer_test.exs +++ b/test/lib/code_corps/github/event/issues/task_syncer_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncerTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.Issues.TaskSyncer, diff --git a/test/lib/code_corps/github/event/issues/user_linker_test.exs b/test/lib/code_corps/github/event/issues/user_linker_test.exs index a9e22e223..b29b81624 100644 --- a/test/lib/code_corps/github/event/issues/user_linker_test.exs +++ b/test/lib/code_corps/github/event/issues/user_linker_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Issues.UserLinkerTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.Issues.UserLinker, diff --git a/test/lib/code_corps/github/event/issues/validator_test.exs b/test/lib/code_corps/github/event/issues/validator_test.exs index 4f89081eb..9a5e68435 100644 --- a/test/lib/code_corps/github/event/issues/validator_test.exs +++ b/test/lib/code_corps/github/event/issues/validator_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.Issues.ValidatorTest do use ExUnit.Case, async: true - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GitHub.Event.Issues.Validator diff --git a/test/lib/code_corps/github/event/issues_test.exs b/test/lib/code_corps/github/event/issues_test.exs index 5e5b35cb1..1f7f81383 100644 --- a/test/lib/code_corps/github/event/issues_test.exs +++ b/test/lib/code_corps/github/event/issues_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do use CodeCorps.DbAccessCase - import CodeCorps.{Factories, TestHelpers.GitHub} + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GitHub.Event.Issues, diff --git a/test/lib/code_corps/github/event_test.exs b/test/lib/code_corps/github/event_test.exs index 40f8ed5b5..1d5f2d0de 100644 --- a/test/lib/code_corps/github/event_test.exs +++ b/test/lib/code_corps/github/event_test.exs @@ -3,8 +3,6 @@ defmodule CodeCorps.GitHub.EventTest do use CodeCorps.DbAccessCase - import CodeCorps.Factories - alias CodeCorps.{ GithubEvent, GitHub.Event diff --git a/test/lib/code_corps/github/github_test.exs b/test/lib/code_corps/github/github_test.exs new file mode 100644 index 000000000..30eac7696 --- /dev/null +++ b/test/lib/code_corps/github/github_test.exs @@ -0,0 +1,122 @@ +defmodule CodeCorps.GitHubTest do + @moduledoc false + + use CodeCorps.DbAccessCase + import CodeCorps.GitHub.TestHelpers + alias CodeCorps.GitHub + + defmodule BasicSuccessAPI do + def request(method, url, headers, body, options) do + send(self(), {method, url, headers, body, options}) + {:ok, "foo"} + end + end + + defmodule BasicErrorAPI do + def request(method, url, headers, body, options) do + send(self(), {method, url, headers, body, options}) + {:error, "bar"} + end + end + + describe "request/5" do + test "properly calls api and returns a successful response" do + with_mock_api(BasicSuccessAPI) do + assert {:ok, "foo"} == GitHub.request(:get, "foo", %{}, %{}, []) + end + + assert_received({ + :get, + "https://api.github.com/foo", + [{"Accept", "application/vnd.github.machine-man-preview+json"}], + "{}", + [:with_body] + }) + end + + test "properly calls api and returns an error response" do + with_mock_api(BasicErrorAPI) do + assert {:error, "bar"} = GitHub.request(:get, "bar", %{}, %{}, []) + end + + assert_received({ + :get, + "https://api.github.com/bar", + [{"Accept", "application/vnd.github.machine-man-preview+json"}], + "{}", + [:with_body] + }) + end + end + + describe "user_access_token_request/2" do + test "properly calls api and returns a successful response" do + with_mock_api(BasicSuccessAPI) do + assert {:ok, "foo"} == GitHub.user_access_token_request("foo_code", "foo_state") + end + + assert_received({ + :post, + "https://github.com/login/oauth/access_token", + [{"Accept", "application/json"}, {"Content-Type", "application/json"}], + body_text, + [:with_body] + }) + + body = body_text |> Poison.decode! + assert body["state"] == "foo_state" + assert body["code"] == "foo_code" + assert body |> Map.has_key?("client_secret") + assert body |> Map.has_key?("client_id") + end + + test "properly calls api and returns an error response" do + with_mock_api(BasicErrorAPI) do + assert {:error, "bar"} == GitHub.user_access_token_request("foo_code", "foo_state") + end + + assert_received({ + :post, "https://github.com/login/oauth/access_token", + [{"Accept", "application/json"}, {"Content-Type", "application/json"}], + body_text, + [:with_body] + }) + + body = body_text |> Poison.decode! + assert body["state"] == "foo_state" + assert body["code"] == "foo_code" + assert body |> Map.has_key?("client_secret") + assert body |> Map.has_key?("client_id") + end + end + + describe "integration_request/5" do + test "properly calls api and returns a successful response" do + with_mock_api(BasicSuccessAPI) do + assert {:ok, "foo"} == GitHub.integration_request(:get, "foo", %{}, %{}, []) + end + + assert_received({ + :get, + "https://api.github.com/foo", + [{"Accept", "application/vnd.github.machine-man-preview+json"}, {"Authorization", "Bearer" <> _}], + "{}", + [:with_body] + }) + end + + test "properly calls api and returns an error response" do + with_mock_api(BasicErrorAPI) do + assert {:error, "bar"} = GitHub.integration_request(:get, "bar", %{}, %{}, []) + end + + assert_received({ + :get, + "https://api.github.com/bar", + [{"Accept", "application/vnd.github.machine-man-preview+json"}, {"Authorization", "Bearer" <> _}], + "{}", + [:with_body] + }) + end + end +end diff --git a/test/lib/code_corps/github/headers_test.exs b/test/lib/code_corps/github/headers_test.exs new file mode 100644 index 000000000..d6fd05794 --- /dev/null +++ b/test/lib/code_corps/github/headers_test.exs @@ -0,0 +1,77 @@ +defmodule CodeCorps.GitHub.HeadersTest do + @moduledoc false + + use ExUnit.Case, async: true + + alias CodeCorps.GitHub.Headers + + describe "access_token_request/0" do + test "works" do + assert Headers.access_token_request == [ + {"Accept", "application/json"}, + {"Content-Type", "application/json"} + ] + end + end + + describe "integration_request/1" do + test "returns defaults when provided a blank map" do + headers = Headers.integration_request(%{}) + + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + end + + test "merges with provided map" do + headers = Headers.integration_request(%{"foo" => "bar"}) + + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + assert {"foo", "bar"} in headers + end + + test "prioritizes keys in provided map" do + headers = Headers.integration_request(%{"foo" => "bar", "Accept" => "baz"}) + + assert {"Accept", "baz"} in headers + assert {"foo", "bar"} in headers + end + + test "adds a jwt to the headers" do + headers = Headers.integration_request(%{}) + + assert headers |> Enum.find(fn {key, _value} -> key == "Authorization" end) + end + end + + describe "user_request/2" do + test "returns defaults when provided a blank map" do + headers = Headers.user_request(%{}, []) + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + end + + test "merges with provided map" do + headers = Headers.user_request(%{"foo" => "bar"}, []) + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + assert {"foo", "bar"} in headers + end + + test "prioritizes keys in provided map" do + headers = Headers.user_request(%{"foo" => "bar", "Accept" => "baz"}, []) + assert {"Accept", "baz"} in headers + assert {"foo", "bar"} in headers + end + + test "adds access token if key is present in opts and not nil" do + headers = Headers.user_request(%{"foo" => "bar"}, [access_token: "foo_bar"]) + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + assert {"foo", "bar"} in headers + assert {"Authorization", "token foo_bar"} in headers + end + + test "does not add access token if key is present in opts but is nil" do + headers = Headers.user_request(%{"foo" => "bar"}, [access_token: nil]) + assert {"Accept", "application/vnd.github.machine-man-preview+json"} in headers + assert {"foo", "bar"} in headers + refute headers |> Enum.find(fn {key, _value} -> key == "Authorization" end) + end + end +end diff --git a/test/lib/code_corps/github/installation_test.exs b/test/lib/code_corps/github/installation_test.exs index 2924d2915..534c2b85a 100644 --- a/test/lib/code_corps/github/installation_test.exs +++ b/test/lib/code_corps/github/installation_test.exs @@ -2,9 +2,8 @@ defmodule CodeCorps.GitHub.InstallationTest do @moduledoc false use CodeCorpsWeb.ApiCase - use CodeCorps.GitHubCase - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GithubAppInstallation, @@ -12,20 +11,13 @@ defmodule CodeCorps.GitHub.InstallationTest do } @access_token "v1.1f699f1069f60xxx" - @installation_github_id 1 @expires_at Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601() - @access_token_create_response %{"token" => @access_token, "expires_at" => @expires_at} @installation_repositories load_endpoint_fixture("installation_repositories") - @installations_access_tokens load_endpoint_fixture("installations_access_tokens") - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_github_id}/access_tokens" => {200, @access_token_create_response} - } describe "repositories/1" do test "makes a request to get the repositories for the authenticated installation" do - installation = %GithubAppInstallation{github_id: @installation_github_id, access_token: @access_token, access_token_expires_at: @expires_at} + installation = %GithubAppInstallation{access_token: @access_token, access_token_expires_at: @expires_at} assert Installation.repositories(installation) == {:ok, @installation_repositories |> Map.get("repositories")} end @@ -42,20 +34,24 @@ defmodule CodeCorps.GitHub.InstallationTest do assert Installation.get_access_token(installation) == {:ok, @access_token} end - @tag bypass: %{"/installations/#{@installation_github_id}/access_tokens" => {200, @installations_access_tokens}} test "returns a new token if expires time has passed" do expires_at = Timex.now() |> Timex.shift(hours: -1) - installation = insert(:github_app_installation, github_id: @installation_github_id, access_token: "old-access-token", access_token_expires_at: expires_at) + installation = insert( + :github_app_installation, + access_token: "old-access-token", access_token_expires_at: expires_at, + github_id: 1) assert Installation.get_access_token(installation) == {:ok, @access_token} end - @tag bypass: %{"/installations/#{@installation_github_id}/access_tokens" => {200, @installations_access_tokens}} test "returns a new token if token and expires time are nil" do - installation = insert(:github_app_installation, github_id: @installation_github_id, access_token: nil, access_token_expires_at: nil) + installation = insert( + :github_app_installation, + access_token: nil, access_token_expires_at: nil, + github_id: 1) assert Installation.get_access_token(installation) == {:ok, @access_token} end diff --git a/test/lib/code_corps/github/user_test.exs b/test/lib/code_corps/github/user_test.exs index e0071aef0..e4d664ad7 100644 --- a/test/lib/code_corps/github/user_test.exs +++ b/test/lib/code_corps/github/user_test.exs @@ -1,22 +1,12 @@ defmodule CodeCorps.GitHub.UserTest do @moduledoc false - use CodeCorps.ModelCase - use CodeCorps.GitHubCase + use CodeCorps.DbAccessCase + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{GitHub, GithubAppInstallation, User} - @user_data %{ - "avatar_url" => "foo_url", - "email" => "foo_email", - "id" => 123, - "login" => "foo_login" - } - - @token_data %{"access_token" => "foo_auth_token"} - describe "connect/2" do - @tag bypass: %{"/user" => {200, @user_data}, "/" => {200, @token_data}} test "posts to github, associates user and installations, returns updated user" do user = insert(:user) @@ -44,13 +34,23 @@ defmodule CodeCorps.GitHub.UserTest do refute Repo.get(GithubAppInstallation, installation_3.id).user_id == returned_user.id end - @error_data %{"error" => "Not Found"} + defmodule NotFoundRequest do + def request(:get, "https://api.github.com/user", _, _, _) do + {:error, GitHub.APIError.new({404, %{"message" => "{\"error\":\"Not Found\"}"}})} + end + def request(method, endpoint, headers, body, options) do + CodeCorps.GitHub.SuccessAPI.request(method, endpoint, headers, body, options) + end + end - @tag bypass: %{"/" => {404, @error_data}} test "posts to github, returns error if reply is not ok" do user = insert(:user) + error = GitHub.APIError.new({404, %{"message" => "{\"error\":\"Not Found\"}"}}) - assert {:error, error} == GitHub.User.connect(user, "foo_code", "foo_state") + + with_mock_api(NotFoundRequest) do + assert {:error, error } == GitHub.User.connect(user, "foo_code", "foo_state") + end end end end diff --git a/test/lib/code_corps/github/webhook/handler_test.exs b/test/lib/code_corps/github/webhook/handler_test.exs index 3e15c5520..7fa24e2be 100644 --- a/test/lib/code_corps/github/webhook/handler_test.exs +++ b/test/lib/code_corps/github/webhook/handler_test.exs @@ -2,10 +2,8 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do @moduledoc false use CodeCorps.DbAccessCase - use CodeCorps.GitHubCase - import CodeCorps.TestHelpers.GitHub - import CodeCorps.Factories + import CodeCorps.GitHub.TestHelpers alias CodeCorps.{ GithubEvent, @@ -14,7 +12,6 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do } describe "handle" do - test "handles issues 'opened' event" do %{"repository" => %{"id" => github_repo_id}} = payload = load_event_fixture("issues_opened") @@ -160,20 +157,8 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do assert event.type == "installation_repositories" end - @access_token "v1.1f699f1069f60xxx" - @installation_created_payload load_event_fixture("installation_created") - @installation_github_id @installation_created_payload["installation"]["id"] - - @expires_at Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601() - @access_token_create_response %{"token" => @access_token, "expires_at" => @expires_at} - - @installation_repositories load_endpoint_fixture("installation_repositories") - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_github_id}/access_tokens" => {200, @access_token_create_response} - } test "handles installation 'created' event" do assert Handler.handle("installation", "abc-123", @installation_created_payload) diff --git a/test/lib/code_corps/github_test.exs b/test/lib/code_corps/github_test.exs deleted file mode 100644 index a9c4a8109..000000000 --- a/test/lib/code_corps/github_test.exs +++ /dev/null @@ -1,50 +0,0 @@ -defmodule CodeCorps.GitHubTest do - @moduledoc false - - use CodeCorps.ModelCase - use CodeCorps.GitHubCase - - alias CodeCorps.GitHub - - describe "request/5" do - @tag bypass: %{"/foo" => {200, %{"bar" => "baz"}}} - test "handles a successful response" do - {:ok, response} = GitHub.request(%{}, :get, "foo", %{}, []) - assert response == %{"bar" => "baz"} - end - - @tag bypass: %{"/foo" => {404, %{"bar" => "baz"}}} - test "handles an error response" do - {:error, response} = GitHub.request(%{}, :get, "foo", %{}, []) - assert response == CodeCorps.GitHub.APIError.new({404, %{"message" => %{"bar" => "baz"} |> Poison.encode!}}) - end - end - - describe "user_access_token_request/2" do - @tag bypass: %{"/" => {200, %{"bar" => "baz"}}} - test "handles a successful response" do - {:ok, response} = GitHub.user_access_token_request("foo_code", "foo_state") - assert response == %{"bar" => "baz"} - end - - @tag bypass: %{"/" => {404, %{"bar" => "baz"}}} - test "handles an error response" do - {:error, response} = GitHub.user_access_token_request("foo_code", "foo_state") - assert response == CodeCorps.GitHub.APIError.new({404, %{"message" => %{"bar" => "baz"} |> Poison.encode!}}) - end - end - - describe "authenticated_integration_request/5" do - @tag bypass: %{"/foo" => {200, %{"bar" => "baz"}}} - test "handles a successful response" do - {:ok, response} = GitHub.authenticated_integration_request(%{}, :get, "foo", %{}, []) - assert response == %{"bar" => "baz"} - end - - @tag bypass: %{"/foo" => {404, %{"bar" => "baz"}}} - test "handles an error response" do - {:error, response} = GitHub.authenticated_integration_request(%{}, :get, "foo", %{}, []) - assert response == CodeCorps.GitHub.APIError.new({404, %{"message" => %{"bar" => "baz"} |> Poison.encode!}}) - end - end -end diff --git a/test/lib/code_corps_web/controllers/github_events_controller_test.exs b/test/lib/code_corps_web/controllers/github_events_controller_test.exs index 48b223dfe..bc181ce76 100644 --- a/test/lib/code_corps_web/controllers/github_events_controller_test.exs +++ b/test/lib/code_corps_web/controllers/github_events_controller_test.exs @@ -2,12 +2,9 @@ defmodule CodeCorpsWeb.GitHubEventsControllerTest do @moduledoc false use CodeCorpsWeb.ConnCase - use CodeCorps.{ - BackgroundProcessingCase, - GitHubCase - } + use CodeCorps.BackgroundProcessingCase - import CodeCorps.TestHelpers.GitHub + import CodeCorps.GitHub.TestHelpers alias CodeCorps.GithubEvent @@ -26,17 +23,6 @@ defmodule CodeCorpsWeb.GitHubEventsControllerTest do |> put_req_header("x-github-delivery", id) end - @access_token "v1.1f699f1069f60xxx" - @expires_at Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601() - @access_token_create_response %{"token" => @access_token, "expires_at" => @expires_at} - - @installation_created load_event_fixture("installation_created") - @installation_repositories load_endpoint_fixture("installation_repositories") - - @tag bypass: %{ - "/installation/repositories" => {200, @installation_repositories}, - "/installations/#{@installation_created["installation"]["id"]}/access_tokens" => {200, @access_token_create_response} - } test "responds with 200 for a supported event", %{conn: conn} do path = conn |> github_events_path(:create) diff --git a/test/lib/code_corps_web/controllers/user_controller_test.exs b/test/lib/code_corps_web/controllers/user_controller_test.exs index f59b1df56..f3640bee7 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/test/lib/code_corps_web/controllers/user_controller_test.exs @@ -2,10 +2,8 @@ defmodule CodeCorpsWeb.UserControllerTest do @moduledoc false use CodeCorpsWeb.ApiCase, resource_name: :user - use CodeCorps.GitHubCase - alias CodeCorps.User - alias CodeCorps.Repo + alias CodeCorps.{User, Repo} @valid_attrs %{ email: "test@user.com", @@ -247,19 +245,6 @@ defmodule CodeCorpsWeb.UserControllerTest do describe "github_oauth" do - @github_user_data %{ - "avatar_url" => "foo_url", - "email" => "foo_email", - "id" => 123, - "login" => "foo_login" - } - - @github_token_data %{"access_token" => "foo_auth_token"} - - @tag bypass: %{ - "/" => {200, @github_token_data}, - "/user" => {200, @github_user_data} - } test "return the user when current user connects successfully", %{conn: conn} do user = insert(:user) diff --git a/test/support/db_access_case.ex b/test/support/db_access_case.ex index 568dd37ae..e359ceaf0 100644 --- a/test/support/db_access_case.ex +++ b/test/support/db_access_case.ex @@ -6,6 +6,14 @@ defmodule CodeCorps.DbAccessCase do use ExUnit.CaseTemplate + using do + quote do + alias CodeCorps.Repo + + import CodeCorps.Factories + end + end + setup tags do :ok = Ecto.Adapters.SQL.Sandbox.checkout(CodeCorps.Repo) diff --git a/test/support/github/success_api.ex b/test/support/github/success_api.ex new file mode 100644 index 000000000..1b232cdd8 --- /dev/null +++ b/test/support/github/success_api.ex @@ -0,0 +1,66 @@ +defmodule CodeCorps.GitHub.SuccessAPI do + @moduledoc ~S""" + A mocked github API layer which returns a default successful response for all + GitHub API requests. + + All tests in the test environment use this module as a mock for GitHub API + requests by default. + + If certain tests explicitly depend on the data returned by GitHub, they can be + mocked individually using the `CodeCorps.GitHub.TestHelpers.with_mock_api` + macro. + + As support for new GitHub endpoints is added, defaults for those endpoints + should be added here. + + To assert a request has been made to GitHub as a result as an action, the + `assert_received` test helper can be used: + + ``` + assert_received({:get, "https://api.github.com/user", headers, body, options}) + ``` + """ + + import CodeCorps.GitHub.TestHelpers + + defmodule UnhandledGitHubEndpointError do + defexception message: "You have a GitHub API endpoint that's unhandled in tests." + end + + def request(method, url, headers, body, options) do + send(self(), {method, url, headers, body, options}) + + {:ok, mock_response(method, url, headers, body, options)} + end + + defp mock_response(:post, "https://github.com/login/oauth/access_token", _, _, _) do + %{"access_token" => "foo_auth_token"} + end + defp mock_response(method, "https://api.github.com/" <> endpoint, headers, body, options) do + mock_response(method, endpoint |> String.split("/"), headers, body, options) + end + defp mock_response(:get, ["user"], _, _, _) do + %{ + "avatar_url" => "foo_url", + "email" => "foo_email", + "id" => 123, + "login" => "foo_login" + } + end + defp mock_response(_method, ["installation", "repositories"], _, _, _) do + load_endpoint_fixture("installation_repositories") + end + defp mock_response(:post, ["installations", _id, "access_tokens"], _, _, _) do + %{ + "token" => "v1.1f699f1069f60xxx", + "expires_at" => Timex.now() |> Timex.shift(hours: 1) |> DateTime.to_iso8601 + } + end + defp mock_response(method, endpoint, _, _, _) when is_binary(endpoint) do + raise UnhandledGitHubEndpointError, message: "You have an unhandled " <> to_string(method) <> " request to " <> endpoint + end + defp mock_response(method, uri_parts, _, _, _) when is_list uri_parts do + endpoint = uri_parts |> Enum.join("/") + raise UnhandledGitHubEndpointError, message: "You have an unhandled API " <> to_string(method) <> " request to " <> endpoint + end +end diff --git a/test/support/github/test_helpers.ex b/test/support/github/test_helpers.ex new file mode 100644 index 000000000..ca0d9dba9 --- /dev/null +++ b/test/support/github/test_helpers.ex @@ -0,0 +1,36 @@ +defmodule CodeCorps.GitHub.TestHelpers do + @spec load_endpoint_fixture(String.t) :: map + def load_endpoint_fixture(id) do + "./test/fixtures/github/endpoints/#{id}.json" |> File.read! |> Poison.decode! + end + + @spec load_event_fixture(String.t) :: map + def load_event_fixture(id) do + "./test/fixtures/github/events/#{id}.json" |> File.read! |> Poison.decode! + end + + @doc ~S""" + Allows setting a mock Github API module for usage in specific tests + To use it, define a module containing the methods expected to be called, then + pass in the block of code expected to call it into the macro: + ``` + defmodule MyApiModule do + def some_function, do: "foo" + end + with_mock_api(MyApiModule) do + execute_code_calling_api + end + ``` + """ + @spec with_mock_api(module, do: function) :: any + defmacro with_mock_api(mock_module, do: block) do + quote do + old_mock = Application.get_env(:code_corps, :github) + Application.put_env(:code_corps, :github, unquote(mock_module)) + + unquote(block) + + Application.put_env(:code_corps, :github, old_mock) + end + end +end diff --git a/test/support/github_case.ex b/test/support/github_case.ex deleted file mode 100644 index 7bbf9a8b2..000000000 --- a/test/support/github_case.ex +++ /dev/null @@ -1,51 +0,0 @@ -defmodule CodeCorps.GitHubCase do - @moduledoc ~S""" - For use in tests where Bypass is needed. - - The module is mainly used through tags. - - Example: - - ``` - use CodeCorps.GitHubCase - - @tag bypass: %{ - "/path/to/request" => {status_code, data_to_respond_with}, - "/path/to/other/request" => {status_code, data_to_respond_with} - # etc... - } - test "some test which makes a github api request" do - # run test code here, all github API requests will automatically respond - # as specified, if listed in the bypass tag - end - """ - - alias CodeCorps.GitHub - - use ExUnit.CaseTemplate - - setup tags do - bypass = Bypass.open - - GitHub.Bypass.setup(bypass) - - case tags |> Map.get(:bypass) do - nil -> nil - bypass_data -> bypass |> setup_handling(bypass_data) - end - - on_exit fn -> - GitHub.Bypass.teardown() - end - - {:ok, bypass: bypass} - end - - defp setup_handling(bypass, handler_data) do - bypass |> Bypass.expect(fn %Plug.Conn{request_path: path} = conn -> - {status, data} = handler_data |> Map.get(path) - response = data |> Poison.encode! - conn |> Plug.Conn.resp(status, response) - end) - end -end diff --git a/test/support/test_helpers/github.ex b/test/support/test_helpers/github.ex deleted file mode 100644 index 8d31f4687..000000000 --- a/test/support/test_helpers/github.ex +++ /dev/null @@ -1,15 +0,0 @@ -defmodule CodeCorps.TestHelpers.GitHub do - @moduledoc """ - Contains test helpers for testing github features - """ - - @spec load_endpoint_fixture(String.t) :: map - def load_endpoint_fixture(id) do - "./test/fixtures/github/endpoints/#{id}.json" |> File.read! |> Poison.decode! - end - - @spec load_event_fixture(String.t) :: map - def load_event_fixture(id) do - "./test/fixtures/github/events/#{id}.json" |> File.read! |> Poison.decode! - end -end diff --git a/test/test_helper.exs b/test/test_helper.exs index cc1caa880..07876e70f 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,8 +1,8 @@ # Make sure all required plugins start before tests start running # Needs to be called before ExUnit.start {:ok, _} = Application.ensure_all_started(:ex_machina) +{:ok, _} = Application.ensure_all_started(:bypass) ExUnit.start -Application.ensure_all_started(:bypass) Ecto.Adapters.SQL.Sandbox.mode(CodeCorps.Repo, :manual)