Refactor user syncing into separate step and clean up some queries

Author: joshsmith

config/dev.exs

@@ -26,7 +26,8 @@ config :code_corps, CodeCorpsWeb.Endpoint,
   ]
 
 # Do not include metadata nor timestamps in development logs
-config :logger, :console, format: "[$level] $message\n"
+config :logger,
+  :console, format: "[$level] $message\n"
 
 # Set a higher stacktrace during development. Avoid configuring such
 # in production as building large stacktraces may be expensive.

lib/code_corps/github/sync/comment/comment/comment.ex

@@ -24,7 +24,6 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
     User
   }
   alias CodeCorps.GitHub.Sync.Comment.Comment.Changeset, as: CommentChangeset
-  alias CodeCorps.GitHub.Sync.User.User, as: UserSyncer
   alias Ecto.Changeset
 
   @type outcome :: {:ok, list(Comment.t)} |
@@ -52,16 +51,15 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
       github_repo: %GithubRepo{
         github_comments: github_comments
       }
-    } = project_github_repo |> Repo.preload([:project, github_repo: [github_comments: [:github_issue, :github_user]]])
+    } = project_github_repo |> Repo.preload([:project, github_repo: [github_comments: [:github_issue, github_user: [:user]]]])
 
     github_comments
     |> Enum.map(&find_or_create_comment(&1, project_github_repo))
     |> ResultAggregator.aggregate
   end
 
-  defp find_or_create_comment(%GithubComment{github_user: %GithubUser{} = github_user} = github_comment, %ProjectGithubRepo{} = project_github_repo) do
-    with {:ok, %User{} = user} <- UserSyncer.create_or_update_user(github_user),
-         %Task{} = task <- find_task(github_comment, project_github_repo),
+  defp find_or_create_comment(%GithubComment{github_user: %GithubUser{user:  %User{} = user}} = github_comment, %ProjectGithubRepo{} = project_github_repo) do
+    with %Task{} = task <- find_task(github_comment, project_github_repo),
          {:ok, %Comment{} = comment} <- sync(task, github_comment, user)
     do
       {:ok, comment}
@@ -85,7 +83,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
     task
     |> find_or_init_comment(github_comment)
     |> CommentChangeset.build_changeset(github_comment, task, user)
-    |> commit()
+    |> Repo.insert_or_update()
   end
 
   @doc ~S"""
@@ -110,7 +108,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
     task
     |> find_or_init_comment(payload)
     |> CommentChangeset.build_changeset(payload, github_comment, task, user)
-    |> commit()
+    |> Repo.insert_or_update()
   end
 
   @spec find_or_init_comment(Task.t, Comment.t) :: Comment.t
@@ -136,8 +134,4 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
       %Comment{} = comment -> comment
     end
   end
-
-  @spec commit(Changeset.t) :: {:ok, Comment.t} | {:error, Changeset.t}
-  defp commit(%Changeset{data: %Comment{id: nil}} = changeset), do: changeset |> Repo.insert
-  defp commit(%Changeset{} = changeset), do: changeset |> Repo.update
 end

lib/code_corps/github/sync/issue/task/task.ex

@@ -1,4 +1,6 @@
 defmodule CodeCorps.GitHub.Sync.Issue.Task do
+  import Ecto.Query
+
   alias CodeCorps.{
     GithubIssue,
     GithubRepo,
@@ -10,7 +12,6 @@ defmodule CodeCorps.GitHub.Sync.Issue.Task do
     Repo
   }
   alias CodeCorps.GitHub.Sync.Issue.Task.Changeset, as: TaskChangeset
-  alias CodeCorps.GitHub.Sync.User.User, as: UserSyncer
   alias Ecto.Changeset
 
   @type outcome :: {:ok, list(Task.t)} |
@@ -39,58 +40,41 @@ defmodule CodeCorps.GitHub.Sync.Issue.Task do
       github_repo: %GithubRepo{
         github_issues: github_issues
       }
-    } = project_github_repo
+    } = project_github_repo |> Repo.preload([:project, github_repo: [github_issues: [github_user: [:user]]]])
 
     github_issues
     |> Enum.map(&find_or_create_task(&1, project_github_repo))
     |> ResultAggregator.aggregate
   end
 
-  defp find_or_create_task(%GithubIssue{github_user: %GithubUser{} = github_user} = github_issue, %ProjectGithubRepo{} = project_github_repo) do
-    with {:ok, %User{} = user} <- UserSyncer.create_or_update_user(github_user),
-         {:ok, %Task{} = task} <- sync(github_issue, project_github_repo, user)
-    do
-      {:ok, task}
-    else
-      {:error, error} -> {:error, error}
-    end
+  defp find_or_create_task(%GithubIssue{github_user: %GithubUser{user: %User{} = user}} = github_issue, %ProjectGithubRepo{} = project_github_repo) do
+    sync(github_issue, project_github_repo, user)
   end
 
   @spec sync(GithubIssue.t, ProjectGithubRepo.t, User.t) :: {:ok, ProjectGithubRepo.t} | {:error, Changeset.t}
   defp sync(%GithubIssue{} = github_issue, %ProjectGithubRepo{} = project_github_repo, %User{} = user) do
     project_github_repo
     |> find_or_init_task(github_issue)
     |> TaskChangeset.build_changeset(github_issue, project_github_repo, user)
-    |> commit()
+    |> Repo.insert_or_update()
   end
 
   @spec sync(GithubIssue.t, ProjectGithubRepo.t, User.t, map) :: {:ok, ProjectGithubRepo.t} | {:error, Changeset.t}
   defp sync(%GithubIssue{} = github_issue, %ProjectGithubRepo{} = project_github_repo, %User{} = user, %{} = payload) do
     project_github_repo
     |> find_or_init_task(github_issue)
     |> TaskChangeset.build_changeset(payload, github_issue, project_github_repo, user)
-    |> commit()
+    |> Repo.insert_or_update()
   end
 
   @spec find_or_init_task(ProjectGithubRepo.t, GithubIssue.t) :: Task.t
   defp find_or_init_task(
-    %ProjectGithubRepo{project_id: project_id, github_repo_id: github_repo_id},
+    %ProjectGithubRepo{project_id: project_id},
     %GithubIssue{id: github_issue_id}
   ) do
-
-    query_params = [
-      github_issue_id: github_issue_id,
-      github_repo_id: github_repo_id,
-      project_id: project_id
-    ]
-
-    case Task |> Repo.get_by(query_params) do
+    case Task |> Repo.get_by(github_issue_id: github_issue_id, project_id: project_id) do
       nil -> %Task{}
       %Task{} = task -> task
     end
   end
-
-  @spec commit(Changeset.t) :: {:ok, Task.t} | {:error, Changeset.t}
-  defp commit(%Changeset{data: %Task{id: nil}} = changeset), do: changeset |> Repo.insert
-  defp commit(%Changeset{} = changeset), do: changeset |> Repo.update
 end

lib/code_corps/github/sync/sync.ex

@@ -123,7 +123,6 @@ defmodule CodeCorps.GitHub.Sync do
     |> Repo.update
   end
 
-
   @count_fields [:syncing_comments_count, :syncing_issues_count, :syncing_pull_requests_count]
 
   defp build_sync_params(sync_state, opts) do
@@ -174,6 +173,8 @@ defmodule CodeCorps.GitHub.Sync do
     with {:ok, project_github_repo} <- project_github_repo |> mark_project_repo("syncing_github_repo"),
          {:ok, %GithubRepo{sync_state: "receiving_webhooks"}} <- repo |> sync_repo(),
          project_github_repo <- Repo.get(ProjectGithubRepo, project_github_repo.id) |> preload_project_github_repo(),
+         {:ok, project_github_repo} <- project_github_repo |> mark_project_repo("syncing_users"),
+         {:ok, _users} <- project_github_repo |> Sync.User.User.sync_project_github_repo() |> sync_step(:sync_users),
          {:ok, project_github_repo} <- project_github_repo |> mark_project_repo("syncing_tasks"),
          {:ok, _tasks} <- project_github_repo |> Sync.Issue.Task.sync_project_github_repo() |> sync_step(:sync_tasks),
          {:ok, project_github_repo} <- project_github_repo |> mark_project_repo("syncing_comments"),
@@ -183,14 +184,15 @@ defmodule CodeCorps.GitHub.Sync do
       {:ok, project_github_repo}
     else
       {:ok, %GithubRepo{}} -> project_github_repo |> mark_project_repo("errored_syncing_github_repo")
+      {:error, :sync_users} -> repo |> mark_project_repo("errored_syncing_users")
       {:error, :sync_tasks} -> repo |> mark_project_repo("errored_syncing_tasks")
       {:error, :sync_comments} -> repo |> mark_project_repo("errored_syncing_comments")
     end
   end
 
   defp preload_project_github_repo(%ProjectGithubRepo{} = project_github_repo) do
     project_github_repo
-    |> Repo.preload([:project, github_repo: [:github_app_installation, [github_issues: [:github_comments, :github_user]]]])
+    |> Repo.preload([:project, github_repo: [:github_app_installation, [github_comments: [:github_issue, :github_user], github_issues: [:github_comments, :github_user]]]])
   end
 
   @doc false

lib/code_corps/github/sync/user/user.ex

@@ -3,13 +3,56 @@ defmodule CodeCorps.GitHub.Sync.User.User do
   In charge of finding or creating a `User` given a `GithubUser`.
   """
 
+  import Ecto.Query
+
   alias CodeCorps.{
     Accounts,
+    GithubComment,
+    GithubIssue,
+    GithubRepo,
     GithubUser,
+    GitHub.Utils.ResultAggregator,
+    ProjectGithubRepo,
     Repo,
     User
   }
 
+  def sync_project_github_repo(%ProjectGithubRepo{github_repo: %GithubRepo{} = _} = project_github_repo) do
+    %ProjectGithubRepo{
+      github_repo: %GithubRepo{
+        github_comments: github_comments,
+        github_issues: github_issues
+      }
+    } = project_github_repo
+
+    comment_users = find_users_for_comments(github_comments)
+    issue_users = find_users_for_issues(github_issues)
+
+    comment_users
+    |> Enum.concat(issue_users)
+    |> Enum.uniq()
+    |> Enum.map(&create_or_update_user/1)
+    |> ResultAggregator.aggregate
+  end
+
+  defp find_users_for_comments(github_comments) do
+    github_comment_ids = Enum.map(github_comments, fn c -> c.id end)
+    query = from gu in GithubUser,
+      distinct: gu.id,
+      join: gc in GithubComment, on: gu.id == gc.github_user_id, where: gc.id in ^github_comment_ids
+
+    query |> Repo.all
+  end
+
+  defp find_users_for_issues(github_issues) do
+    github_issue_ids = Enum.map(github_issues, fn i -> i.id end)
+    query = from gu in GithubUser,
+      distinct: gu.id,
+      join: gi in GithubIssue, on: gu.id == gi.github_user_id, where: gi.id in ^github_issue_ids
+
+    query |> Repo.all
+  end
+
   @doc ~S"""
   Creates or updates a `User` given a `GithubUser`.
   """

lib/code_corps/model/github_user.ex

@@ -9,6 +9,8 @@ defmodule CodeCorps.GithubUser do
     field :type, :string
     field :username, :string
 
+    has_one :user, CodeCorps.User
+
     timestamps()
   end
 

lib/code_corps/model/project_github_repo.ex

@@ -42,6 +42,7 @@ defmodule CodeCorps.ProjectGithubRepo do
     ~w{
       unsynced
       syncing_github_repo errored_syncing_github_repo
+      syncing_users errored_syncing_users
       syncing_tasks errored_syncing_tasks
       syncing_comments errored_syncing_comments
       synced

lib/code_corps/model/task.ex

@@ -3,7 +3,7 @@ defmodule CodeCorps.Task do
 
   import EctoOrdered
 
-  alias CodeCorps.{Task, Services.MarkdownRendererService}
+  alias CodeCorps.{Repo, Services.MarkdownRendererService, Task}
   alias Ecto.Changeset
 
   @type t :: %__MODULE__{}

priv/repo/migrations/20171104013543_add_indexes_for_syncing.exs

@@ -0,0 +1,7 @@
+defmodule CodeCorps.Repo.Migrations.AddIndexesForSyncing do
+  use Ecto.Migration
+
+  def change do
+    create index(:tasks, [:github_issue_id, :project_id])
+  end
+end

priv/repo/structure.sql

@@ -3114,6 +3114,13 @@ CREATE INDEX tasks_archived_index ON tasks USING btree (archived);
 CREATE INDEX tasks_github_issue_id_index ON tasks USING btree (github_issue_id);
 
 
+--
+-- Name: tasks_github_issue_id_project_id_index; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX tasks_github_issue_id_project_id_index ON tasks USING btree (github_issue_id, project_id);
+
+
 --
 -- Name: tasks_github_repo_id_index; Type: INDEX; Schema: public; Owner: -
 --
@@ -3837,5 +3844,5 @@ ALTER TABLE ONLY users
 -- PostgreSQL database dump complete
 --
 
-INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309);
+INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309), (20171104013543);