Skip to content

Commit

Permalink
Refactor webhook handler to take only the payload
Browse files Browse the repository at this point in the history
  • Loading branch information
joshsmith committed Oct 11, 2017
1 parent b6a6b60 commit 020888d
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 144 deletions.
10 changes: 4 additions & 6 deletions lib/code_corps/github/event/handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ defmodule CodeCorps.GitHub.Event.Handler do
Default behavior for all GitHub webhook event handlers.
"""

alias CodeCorps.GithubEvent

@doc ~S"""
The only entry point a GitHub webhook event handler function should contain.
Receives a `CodeCorps.GithubEvent` record and a payload, returns an `:ok`
tuple if the process was successful, or an `:error` tuple, where the second
element is an atom, if it failed.
Receives the GitHub payload, returns an `:ok` tuple if the process was
successful, or an `:error` tuple, where the second element is an atom, if it
failed.
"""
@callback handle(GithubEvent.t, map) :: {:ok, any} | {:error, atom}
@callback handle(map) :: {:ok, any} | {:error, atom}
end
5 changes: 2 additions & 3 deletions lib/code_corps/github/event/installation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.Installation do

alias CodeCorps.{
GithubAppInstallation,
GithubEvent,
GitHub.Event.Installation,
Repo,
User
Expand Down Expand Up @@ -48,8 +47,8 @@ defmodule CodeCorps.GitHub.Event.Installation do
If a step in the process failes, an `:error` tuple will be returned, where the
second element is an atom indicating which step of the process failed.
"""
@spec handle(GithubEvent.t, map) :: outcome
def handle(%GithubEvent{}, payload) do
@spec handle(map) :: outcome
def handle(payload) do
Multi.new
|> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
|> Multi.run(:action, fn _ -> payload |> validate_action() end)
Expand Down
5 changes: 2 additions & 3 deletions lib/code_corps/github/event/installation_repositories.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositories do

alias CodeCorps.{
GithubAppInstallation,
GithubEvent,
GithubRepo,
GitHub.Event.Common.ResultAggregator,
GitHub.Event.InstallationRepositories,
Expand Down Expand Up @@ -43,8 +42,8 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositories do
`CodeCorps.ProjectGithubRepo` records, they are deleted automatically,
due to `on_delete: :delete_all` set at the database level.
"""
@spec handle(GithubEvent.t, map) :: outcome
def handle(%GithubEvent{}, payload) do
@spec handle(map) :: outcome
def handle(payload) do
Multi.new
|> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
|> Multi.run(:action, fn _ -> payload |> validate_action() end)
Expand Down
5 changes: 2 additions & 3 deletions lib/code_corps/github/event/issue_comment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.IssueComment do

alias CodeCorps.{
Comment,
GithubEvent,
GitHub.Event.Common.RepoFinder,
GitHub.Event.Issues,
GitHub.Event.Issues.TaskSyncer,
Expand Down Expand Up @@ -50,8 +49,8 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
If it fails, it will instead return an `:error` tuple, where the second
element is the atom indicating a reason.
"""
@spec handle(GithubEvent.t, map) :: outcome
def handle(%GithubEvent{}, payload) do
@spec handle(map) :: outcome
def handle(payload) do
Multi.new
|> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
|> Multi.run(:action, fn _ -> payload |> validate_action() end)
Expand Down
5 changes: 2 additions & 3 deletions lib/code_corps/github/event/issues.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.Issues do
@behaviour CodeCorps.GitHub.Event.Handler

alias CodeCorps.{
GithubEvent,
GitHub.Event.Common.RepoFinder,
GitHub.Event.Issues.TaskSyncer,
GitHub.Event.Issues.UserLinker,
Expand Down Expand Up @@ -45,8 +44,8 @@ defmodule CodeCorps.GitHub.Event.Issues do
If it fails, it will instead return an `:error` tuple, where the second
element is the atom indicating a reason.
"""
@spec handle(GithubEvent.t, map) :: outcome
def handle(%GithubEvent{}, %{} = payload) do
@spec handle(map) :: outcome
def handle(payload) do
Multi.new
|> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
|> Multi.run(:action, fn _ -> payload |> validate_action() end)
Expand Down
12 changes: 6 additions & 6 deletions lib/code_corps/github/webhook/handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ defmodule CodeCorps.GitHub.Webhook.Handler do
def handle(type, id, payload) do
with %{} = params <- build_params(type, id, payload),
{:ok, %GithubEvent{} = event} <- params |> create_event(),
{:ok, %GithubEvent{status: "processing"} = processing_event} <- event |> Event.start_processing
{:ok, %GithubEvent{status: "processing"} = event} <- event |> Event.start_processing
do
processing_event |> do_handle(payload) |> Event.stop_processing(event)
payload |> do_handle(type) |> Event.stop_processing(event)
end
end

Expand All @@ -47,8 +47,8 @@ defmodule CodeCorps.GitHub.Webhook.Handler do
end
end

defp do_handle(%GithubEvent{type: "installation"} = event, payload), do: Installation.handle(event, payload)
defp do_handle(%GithubEvent{type: "installation_repositories"} = event, payload), do: InstallationRepositories.handle(event, payload)
defp do_handle(%GithubEvent{type: "issue_comment"} = event, payload), do: IssueComment.handle(event, payload)
defp do_handle(%GithubEvent{type: "issues"} = event, payload), do: Issues.handle(event, payload)
defp do_handle(payload, "installation"), do: Installation.handle(payload)
defp do_handle(payload, "installation_repositories"), do: InstallationRepositories.handle(payload)
defp do_handle(payload, "issue_comment"), do: IssueComment.handle(payload)
defp do_handle(payload, "issues"), do: Issues.handle(payload)
end
42 changes: 15 additions & 27 deletions test/lib/code_corps/github/event/installation_repositories_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
Repo
}

describe "handle/2" do
describe "handle/1" do
@payload load_event_fixture("installation_repositories_added")

test "marks event as errored if invalid action" do
payload = @payload |> Map.put("action", "foo")
event = build(:github_event, action: "foo", type: "installation_repositories")
assert {:error, :unexpected_action} == InstallationRepositories.handle(event, payload)
assert {:error, :unexpected_action} == InstallationRepositories.handle(payload)
end

test "marks event as errored if invalid payload" do
event = build(:github_event, action: "added", type: "installation_repositories")
payload = @payload |> Map.delete("action")
assert {:error, :unexpected_payload} ==
InstallationRepositories.handle(event, payload)
InstallationRepositories.handle(payload)
end
end

describe "handle/2 for InstallationRepositories::added" do
describe "handle/1 for InstallationRepositories::added" do
@payload load_event_fixture("installation_repositories_added")

test "creates repos" do
Expand All @@ -40,8 +38,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do

%{id: installation_id} = insert(:github_app_installation, github_id: installation_github_id)

event = build(:github_event, action: "added", type: "installation_repositories")
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)

github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"])
assert github_repo_1
Expand All @@ -63,8 +60,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
installation = insert(:github_app_installation, github_id: installation_github_id)
preinserted_repo = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"])

event = build(:github_event, action: "added", type: "installation_repositories")
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)

github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"])
assert github_repo_1.id == preinserted_repo.id
Expand All @@ -78,23 +74,20 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
end

test "marks event as errored if invalid instalation payload" do
event = insert(:github_event, action: "added", type: "installation_repositories")
assert {:error, :unexpected_payload} == InstallationRepositories.handle(event, @payload |> Map.put("installation", "foo"))
assert {:error, :unexpected_payload} == InstallationRepositories.handle(@payload |> Map.put("installation", "foo"))
end

test "marks event as errored if invalid repo payload" do
event = insert(:github_event, action: "added", type: "installation_repositories")
insert(:github_app_installation, github_id: @payload["installation"]["id"])
assert {:error, :unexpected_payload} == InstallationRepositories.handle(event, @payload |> Map.put("repositories_added", ["foo"]))
assert {:error, :unexpected_payload} == InstallationRepositories.handle(@payload |> Map.put("repositories_added", ["foo"]))
end

test "marks event as errored if no installation" do
event = insert(:github_event, action: "added", type: "installation_repositories")
assert {:error, :unmatched_installation} == InstallationRepositories.handle(event, @payload)
assert {:error, :unmatched_installation} == InstallationRepositories.handle(@payload)
end
end

describe "handle/2 for InstallationRepositories::removed" do
describe "handle/1 for InstallationRepositories::removed" do
@payload load_event_fixture("installation_repositories_removed")

test "deletes github repos and associated project github repos" do
Expand All @@ -108,8 +101,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
insert(:project_github_repo, project: project, github_repo: github_repo_1)
insert(:github_repo, github_app_installation: installation, github_id: repo_2_payload["id"])

event = build(:github_event, action: "removed", type: "installation_repositories")
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
{:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)

assert Repo.aggregate(GithubRepo, :count, :id) == 0
assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0
Expand All @@ -125,31 +117,27 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
github_repo_1 = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"])
insert(:project_github_repo, project: project, github_repo: github_repo_1)

event = build(:github_event, action: "removed", type: "installation_repositories")
{:ok, [%GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
{:ok, [%GithubRepo{}]} = InstallationRepositories.handle(@payload)

assert Repo.aggregate(GithubRepo, :count, :id) == 0
assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0
end

test "marks event as errored if invalid instalation payload" do
event = build(:github_event, action: "removed", type: "installation_repositories")
payload = @payload |> Map.put("installation", "foo")
assert {:error, :unexpected_payload} ==
InstallationRepositories.handle(event, payload)
InstallationRepositories.handle(payload)
end

test "marks event as errored if invalid repo payload" do
event = build(:github_event, action: "removed", type: "installation_repositories")
insert(:github_app_installation, github_id: @payload["installation"]["id"])
payload = @payload |> Map.put("repositories_removed", ["foo"])
assert {:error, :unexpected_payload} ==
InstallationRepositories.handle(event, payload)
InstallationRepositories.handle(payload)
end

test "marks event as errored if no installation" do
event = build(:github_event, action: "added", type: "installation_repositories")
assert {:error, :unmatched_installation} == InstallationRepositories.handle(event, @payload)
assert {:error, :unmatched_installation} == InstallationRepositories.handle(@payload)
end
end
end
36 changes: 12 additions & 24 deletions test/lib/code_corps/github/event/installation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -44,56 +44,47 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
@bad_sender_payload @installation_created |> Map.put("sender", "foo")
@bad_installation_payload @installation_created |> Map.put("installation", "foo")

describe "handle/2" do
describe "handle/1" do
test "returns error if action of the event is wrong" do
event = build(:github_event, action: "foo", type: "installation")
assert {:error, :unexpected_action} ==
Installation.handle(event, @bad_action_payload)
Installation.handle(@bad_action_payload)
end

test "returns error if payload is wrong" do
event = build(:github_event, action: "created", type: "installation")
assert {:error, :unexpected_payload} == Installation.handle(event, %{})
assert {:error, :unexpected_payload} == Installation.handle(%{})
end

test "returns error if user payload is wrong" do
event = build(:github_event, action: "created", type: "installation")
assert {:error, :unexpected_payload} ==
Installation.handle(event, @bad_sender_payload)
Installation.handle(@bad_sender_payload)
end

test "returns error if installation payload is wrong" do
event = build(:github_event, action: "created", type: "installation")
assert {:error, :unexpected_payload} ==
Installation.handle(event, @bad_installation_payload)
Installation.handle(@bad_installation_payload)
end

test "returns installation as errored if api error" do
event = build(:github_event, action: "created", type: "installation")

with_mock_api(BadRepoRequest) do
assert {:error, :github_api_error_on_syncing_repos}
= Installation.handle(event, @installation_created)
= Installation.handle(@installation_created)
end
end

test "returns installation as errored if error creating repos" do
event = build(:github_event, action: "created", type: "installation")

with_mock_api(InvalidRepoRequest) do
assert {:error, :validation_error_on_syncing_existing_repos} ==
Installation.handle(event, @installation_created)
Installation.handle(@installation_created)
end
end
end

describe "handle/2 for Installation::created" do
describe "handle/1 for Installation::created" do
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")

{:ok, %GithubAppInstallation{} = installation}
= Installation.handle(event, payload)
= Installation.handle(payload)

assert installation.github_id == installation_github_id
assert installation.origin == "github"
Expand All @@ -105,12 +96,11 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do

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")

user = insert(:user, github_id: user_github_id)

{:ok, %GithubAppInstallation{} = installation}
= Installation.handle(event, payload)
= Installation.handle(payload)

assert installation.github_id == (payload |> get_in(["installation", "id"]))
assert installation.origin == "github"
Expand All @@ -123,7 +113,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do

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")

user = insert(:user, github_id: user_github_id)
insert(
Expand All @@ -134,7 +123,7 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
)

{:ok, %GithubAppInstallation{} = installation}
= Installation.handle(event, payload)
= Installation.handle(payload)

assert installation.origin == "codecorps"
assert installation.state == "processed"
Expand All @@ -148,10 +137,9 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
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)
event = build(:github_event, action: "created", type: "installation")

{:ok, %GithubAppInstallation{} = installation}
= Installation.handle(event, payload)
= Installation.handle(payload)

assert installation.origin == "codecorps"
assert installation.state == "processed"
Expand Down

0 comments on commit 020888d

Please sign in to comment.