From 3c957c006633d2df44f0d23a3131294f9b657d2b Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 13 Oct 2016 17:53:01 +0200
Subject: [PATCH 1/6] Added tests for IssuePolicy

---
 spec/policies/issue_policy_spec.rb | 119 +++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 spec/policies/issue_policy_spec.rb

diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
new file mode 100644
index 00000000000..7591bfd1471
--- /dev/null
+++ b/spec/policies/issue_policy_spec.rb
@@ -0,0 +1,119 @@
+require 'spec_helper'
+
+describe IssuePolicy, models: true do
+  let(:user) { create(:user) }
+
+  describe '#rules' do
+    context 'using a regular issue' do
+      let(:project) { create(:project, :public) }
+      let(:issue) { create(:issue, project: project) }
+      let(:policies) { described_class.abilities(user, issue).to_set }
+
+      context 'with a regular user' do
+        it 'includes the read_issue permission' do
+          expect(policies).to include(:read_issue)
+        end
+
+        it 'does not include the admin_issue permission' do
+          expect(policies).not_to include(:admin_issue)
+        end
+
+        it 'does not include the update_issue permission' do
+          expect(policies).not_to include(:update_issue)
+        end
+      end
+
+      context 'with a user that is a project reporter' do
+        before do
+          project.team << [user, :reporter]
+        end
+
+        it 'includes the read_issue permission' do
+          expect(policies).to include(:read_issue)
+        end
+
+        it 'includes the admin_issue permission' do
+          expect(policies).to include(:admin_issue)
+        end
+
+        it 'includes the update_issue permission' do
+          expect(policies).to include(:update_issue)
+        end
+      end
+
+      context 'with a user that is a project guest' do
+        before do
+          project.team << [user, :guest]
+        end
+
+        it 'includes the read_issue permission' do
+          expect(policies).to include(:read_issue)
+        end
+
+        it 'does not include the admin_issue permission' do
+          expect(policies).not_to include(:admin_issue)
+        end
+
+        it 'does not include the update_issue permission' do
+          expect(policies).not_to include(:update_issue)
+        end
+      end
+    end
+
+    context 'using a confidential issue' do
+      let(:issue) { create(:issue, :confidential) }
+
+      context 'with a regular user' do
+        let(:policies) { described_class.abilities(user, issue).to_set }
+
+        it 'does not include the read_issue permission' do
+          expect(policies).not_to include(:read_issue)
+        end
+
+        it 'does not include the admin_issue permission' do
+          expect(policies).not_to include(:admin_issue)
+        end
+
+        it 'does not include the update_issue permission' do
+          expect(policies).not_to include(:update_issue)
+        end
+      end
+
+      context 'with a user that is a project member' do
+        let(:policies) { described_class.abilities(user, issue).to_set }
+
+        before do
+          issue.project.team << [user, :reporter]
+        end
+
+        it 'includes the read_issue permission' do
+          expect(policies).to include(:read_issue)
+        end
+
+        it 'includes the admin_issue permission' do
+          expect(policies).to include(:admin_issue)
+        end
+
+        it 'includes the update_issue permission' do
+          expect(policies).to include(:update_issue)
+        end
+      end
+
+      context 'without a user' do
+        let(:policies) { described_class.abilities(nil, issue).to_set }
+
+        it 'does not include the read_issue permission' do
+          expect(policies).not_to include(:read_issue)
+        end
+
+        it 'does not include the admin_issue permission' do
+          expect(policies).not_to include(:admin_issue)
+        end
+
+        it 'does not include the update_issue permission' do
+          expect(policies).not_to include(:update_issue)
+        end
+      end
+    end
+  end
+end

From 467b346f0684053081d7e762df1a2b5df5888543 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 13 Oct 2016 17:53:47 +0200
Subject: [PATCH 2/6] Add User#projects_with_reporter_access_limited_to

This method can be used to retrieve a list of projects for a user that
said user has reporter access to. This list is then be reduced down to
a specific set of projects. This allows you to reduce a list of projects
to a list of projects you have reporter access to in an efficient
manner.
---
 app/models/user.rb       | 10 ++++++++++
 spec/models/user_spec.rb | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/app/models/user.rb b/app/models/user.rb
index 65e96ee6b2e..c0dffa7b6ea 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -444,6 +444,16 @@ class User < ActiveRecord::Base
     Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
   end
 
+  # Returns the projects this user has reporter (or greater) access to, limited
+  # to at most the given projects.
+  #
+  # This method is useful when you have a list of projects and want to
+  # efficiently check to which of these projects the user has at least reporter
+  # access.
+  def projects_with_reporter_access_limited_to(projects)
+    authorized_projects(Gitlab::Access::REPORTER).where(id: projects)
+  end
+
   def viewable_starred_projects
     starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})",
                            [Project::PUBLIC, Project::INTERNAL])
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ba47479a2e1..3b152e15b61 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1205,4 +1205,40 @@ describe User, models: true do
       expect(user.viewable_starred_projects).not_to include(private_project)
     end
   end
+
+  describe '#projects_with_reporter_access_limited_to' do
+    let(:project1) { create(:project) }
+    let(:project2) { create(:project) }
+    let(:user) { create(:user) }
+
+    before do
+      project1.team << [user, :reporter]
+      project2.team << [user, :guest]
+    end
+
+    it 'returns the projects when using a single project ID' do
+      projects = user.projects_with_reporter_access_limited_to(project1.id)
+
+      expect(projects).to eq([project1])
+    end
+
+    it 'returns the projects when using an Array of project IDs' do
+      projects = user.projects_with_reporter_access_limited_to([project1.id])
+
+      expect(projects).to eq([project1])
+    end
+
+    it 'returns the projects when using an ActiveRecord relation' do
+      projects = user.
+        projects_with_reporter_access_limited_to(Project.select(:id))
+
+      expect(projects).to eq([project1])
+    end
+
+    it 'does not return projects you do not have reporter access to' do
+      projects = user.projects_with_reporter_access_limited_to(project2.id)
+
+      expect(projects).to be_empty
+    end
+  end
 end

From 24261f2dbd50583ac997c3f53f78104a96fa2cd3 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 11 Oct 2016 17:37:19 +0200
Subject: [PATCH 3/6] Add the method ExternalIssue#project_id

This method returns the project's ID, making ExternalIssue slightly more
compatible with Issue (which also defines the "project_id" method).
---
 app/models/external_issue.rb       | 9 +++++++++
 spec/models/external_issue_spec.rb | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index fd9a8c1b8b7..91b508eb325 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -29,6 +29,15 @@ class ExternalIssue
     @project
   end
 
+  def project_id
+    @project.id
+  end
+
+  # Pattern used to extract `JIRA-123` issue references from text
+  def self.reference_pattern
+    @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
+  end
+
   def to_reference(_from_project = nil)
     id
   end
diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb
index ebba6e14578..2debe1289a3 100644
--- a/spec/models/external_issue_spec.rb
+++ b/spec/models/external_issue_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe ExternalIssue, models: true do
-  let(:project) { double('project', to_reference: 'namespace1/project1') }
+  let(:project) { double('project', id: 1, to_reference: 'namespace1/project1') }
   let(:issue)   { described_class.new('EXT-1234', project) }
 
   describe 'modules' do
@@ -36,4 +36,10 @@ describe ExternalIssue, models: true do
       end
     end
   end
+
+  describe '#project_id' do
+    it 'returns the ID of the project' do
+      expect(issue.project_id).to eq(project.id)
+    end
+  end
 end

From 89bb29b247b57e3b4ba053a5fd17f2087ac4414f Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 11 Oct 2016 17:31:19 +0200
Subject: [PATCH 4/6] Flush Housekeeping data from Redis specs

These specs use raw Redis objects which can not use the memory based
caching mechanism used for tests. As such we have to explicitly flush
the data from Redis before/after each spec to ensure no data lingers on.
---
 spec/services/git_push_service_spec.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 45bc44ba172..1c7a3d04b09 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -538,9 +538,16 @@ describe GitPushService, services: true do
     let(:housekeeping) { Projects::HousekeepingService.new(project) }
 
     before do
+      # Flush any raw Redis data stored by the housekeeping code.
+      Gitlab::Redis.with { |conn| conn.flushall }
+
       allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
     end
 
+    after do
+      Gitlab::Redis.with { |conn| conn.flushall }
+    end
+
     it 'does not perform housekeeping when not needed' do
       expect(housekeeping).not_to receive(:execute)
 

From f694f94c491452a50035c2ff43c8ba595c0e73aa Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 12 Oct 2016 14:01:34 +0200
Subject: [PATCH 5/6] Added IssueCollection

This class can be used to reduce a list of issues down to a subset based
on user permissions. This class operates in such a way that it can
reduce issues using as few queries as possible, if any at all.
---
 app/models/concerns/issuable.rb       |  5 +++
 app/models/issue_collection.rb        | 40 ++++++++++++++++++
 app/policies/issuable_policy.rb       |  2 +-
 app/policies/issue_policy.rb          |  9 +----
 spec/models/concerns/issuable_spec.rb | 21 ++++++++++
 spec/models/issue_collection_spec.rb  | 58 +++++++++++++++++++++++++++
 6 files changed, 127 insertions(+), 8 deletions(-)
 create mode 100644 app/models/issue_collection.rb
 create mode 100644 spec/models/issue_collection_spec.rb

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 613444e0d70..93a6b3122e0 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -286,6 +286,11 @@ module Issuable
     false
   end
 
+  def assignee_or_author?(user)
+    # We're comparing IDs here so we don't need to load any associations.
+    author_id == user.id || assignee_id == user.id
+  end
+
   def record_metrics
     metrics = self.metrics || create_metrics
     metrics.record!
diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb
new file mode 100644
index 00000000000..df36252a5b1
--- /dev/null
+++ b/app/models/issue_collection.rb
@@ -0,0 +1,40 @@
+# IssueCollection can be used to reduce a list of issues down to a subset.
+#
+# IssueCollection is not meant to be some sort of Enumerable, instead it's meant
+# to take a list of issues and return a new list of issues based on some
+# criteria. For example, given a list of issues you may want to return a list of
+# issues that can be read or updated by a given user.
+class IssueCollection
+  attr_reader :collection
+
+  def initialize(collection)
+    @collection = collection
+  end
+
+  # Returns all the issues that can be updated by the user.
+  def updatable_by_user(user)
+    return collection if user.admin?
+
+    # Given all the issue projects we get a list of projects that the current
+    # user has at least reporter access to.
+    projects_with_reporter_access = user.
+      projects_with_reporter_access_limited_to(project_ids).
+      pluck(:id)
+
+    collection.select do |issue|
+      if projects_with_reporter_access.include?(issue.project_id)
+        true
+      elsif issue.is_a?(Issue)
+        issue.assignee_or_author?(user)
+      else
+        false
+      end
+    end
+  end
+
+  private
+
+  def project_ids
+    @project_ids ||= collection.map(&:project_id).uniq
+  end
+end
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index c253f9a9399..9501e499507 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -4,7 +4,7 @@ class IssuablePolicy < BasePolicy
   end
 
   def rules
-    if @user && (@subject.author == @user || @subject.assignee == @user)
+    if @user && @subject.assignee_or_author?(@user)
       can! :"read_#{action_name}"
       can! :"update_#{action_name}"
     end
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index bd1811a3c54..f3ede58a001 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -8,9 +8,8 @@ class IssuePolicy < IssuablePolicy
 
     if @subject.confidential? && !can_read_confidential?
       cannot! :read_issue
-      cannot! :admin_issue
       cannot! :update_issue
-      cannot! :read_issue
+      cannot! :admin_issue
     end
   end
 
@@ -18,11 +17,7 @@ class IssuePolicy < IssuablePolicy
 
   def can_read_confidential?
     return false unless @user
-    return true if @user.admin?
-    return true if @subject.author == @user
-    return true if @subject.assignee == @user
-    return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER)
 
-    false
+    IssueCollection.new([@subject]).updatable_by_user(@user).any?
   end
 end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index a59d30687f6..a9603074c32 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -341,4 +341,25 @@ describe Issue, "Issuable" do
       expect(Issue.with_label([bug.title, enhancement.title])).to match_array([issue2])
     end
   end
+
+  describe '#assignee_or_author?' do
+    let(:user) { build(:user, id: 1) }
+    let(:issue) { build(:issue) }
+
+    it 'returns true for a user that is assigned to an issue' do
+      issue.assignee = user
+
+      expect(issue.assignee_or_author?(user)).to eq(true)
+    end
+
+    it 'returns true for a user that is the author of an issue' do
+      issue.author = user
+
+      expect(issue.assignee_or_author?(user)).to eq(true)
+    end
+
+    it 'returns false for a user that is not the assignee or author' do
+      expect(issue.assignee_or_author?(user)).to eq(false)
+    end
+  end
 end
diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb
new file mode 100644
index 00000000000..d9ab397c302
--- /dev/null
+++ b/spec/models/issue_collection_spec.rb
@@ -0,0 +1,58 @@
+require 'spec_helper'
+
+describe IssueCollection do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:issue1) { create(:issue, project: project) }
+  let(:issue2) { create(:issue, project: project) }
+  let(:collection) { described_class.new([issue1, issue2]) }
+
+  describe '#collection' do
+    it 'returns the issues in the same order as the input Array' do
+      expect(collection.collection).to eq([issue1, issue2])
+    end
+  end
+
+  describe '#updatable_by_user' do
+    context 'using an admin user' do
+      it 'returns all issues' do
+        user = create(:admin)
+
+        expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
+      end
+    end
+
+    context 'using a user that has no access to the project' do
+      it 'returns no issues when the user is not an assignee or author' do
+        expect(collection.updatable_by_user(user)).to be_empty
+      end
+
+      it 'returns the issues the user is assigned to' do
+        issue1.assignee = user
+
+        expect(collection.updatable_by_user(user)).to eq([issue1])
+      end
+
+      it 'returns the issues for which the user is the author' do
+        issue1.author = user
+
+        expect(collection.updatable_by_user(user)).to eq([issue1])
+      end
+    end
+
+    context 'using a user that has reporter access to the project' do
+      it 'returns the issues of the project' do
+        project.team << [user, :reporter]
+
+        expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
+      end
+    end
+
+    context 'using a user that is the owner of a project' do
+      it 'returns the issues of the project' do
+        expect(collection.updatable_by_user(project.namespace.owner)).
+          to eq([issue1, issue2])
+      end
+    end
+  end
+end

From 509910b89f636f95d2d5a9cd3f38ce8f7f4f47a6 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 7 Oct 2016 15:20:57 +0200
Subject: [PATCH 6/6] Process commits in a separate worker

This moves the code used for processing commits from GitPushService to
its own Sidekiq worker: ProcessCommitWorker.

Using a Sidekiq worker allows us to process multiple commits in
parallel. This in turn will lead to issues being closed faster and cross
references being created faster. Furthermore by isolating this code into
a separate class it's easier to test and maintain the code.

The new worker also ensures it can efficiently check which issues can be
closed, without having to run numerous SQL queries for every issue.
---
 app/models/issue_collection.rb                |   2 +
 app/policies/issue_policy.rb                  |   2 +-
 app/services/git_push_service.rb              |  37 +-----
 app/services/issues/close_service.rb          |  13 +++
 app/workers/process_commit_worker.rb          |  67 +++++++++++
 .../process-commits-using-sidekiq.yml         |   4 +
 config/sidekiq_queues.yml                     |   1 +
 spec/models/issue_collection_spec.rb          |   9 ++
 spec/services/git_push_service_spec.rb        |   9 ++
 spec/services/issues/close_service_spec.rb    |  49 +++++---
 spec/workers/process_commit_worker_spec.rb    | 109 ++++++++++++++++++
 11 files changed, 251 insertions(+), 51 deletions(-)
 create mode 100644 app/workers/process_commit_worker.rb
 create mode 100644 changelogs/unreleased/process-commits-using-sidekiq.yml
 create mode 100644 spec/workers/process_commit_worker_spec.rb

diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb
index df36252a5b1..f0b7d9914c8 100644
--- a/app/models/issue_collection.rb
+++ b/app/models/issue_collection.rb
@@ -32,6 +32,8 @@ class IssueCollection
     end
   end
 
+  alias_method :visible_to, :updatable_by_user
+
   private
 
   def project_ids
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index f3ede58a001..52fa33bc4b0 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -18,6 +18,6 @@ class IssuePolicy < IssuablePolicy
   def can_read_confidential?
     return false unless @user
 
-    IssueCollection.new([@subject]).updatable_by_user(@user).any?
+    IssueCollection.new([@subject]).visible_to(@user).any?
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index e8415862de5..de313095bed 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -105,35 +105,11 @@ class GitPushService < BaseService
   # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
   # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
   def process_commit_messages
-    is_default_branch = is_default_branch?
-
-    authors = Hash.new do |hash, commit|
-      email = commit.author_email
-      next hash[email] if hash.has_key?(email)
-
-      hash[email] = commit_user(commit)
-    end
+    default = is_default_branch?
 
     @push_commits.each do |commit|
-      # Keep track of the issues that will be actually closed because they are on a default branch.
-      # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches)
-      # will also have cross-reference.
-      closed_issues = []
-
-      if is_default_branch
-        # Close issues if these commits were pushed to the project's default branch and the commit message matches the
-        # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
-        # a different branch.
-        closed_issues = commit.closes_issues(current_user)
-        closed_issues.each do |issue|
-          if can?(current_user, :update_issue, issue)
-            Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit: commit)
-          end
-        end
-      end
-
-      commit.create_cross_references!(authors[commit], closed_issues)
-      update_issue_metrics(commit, authors)
+      ProcessCommitWorker.
+        perform_async(project.id, current_user.id, commit.id, default)
     end
   end
 
@@ -176,11 +152,4 @@ class GitPushService < BaseService
   def branch_name
     @branch_name ||= Gitlab::Git.ref_name(params[:ref])
   end
-
-  def update_issue_metrics(commit, authors)
-    mentioned_issues = commit.all_references(authors[commit]).issues
-
-    Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil).
-      update_all(first_mentioned_in_commit_at: commit.committed_date)
-  end
 end
diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb
index 45cca216ccc..ab4c51386a4 100644
--- a/app/services/issues/close_service.rb
+++ b/app/services/issues/close_service.rb
@@ -1,8 +1,21 @@
 module Issues
   class CloseService < Issues::BaseService
+    # Closes the supplied issue if the current user is able to do so.
     def execute(issue, commit: nil, notifications: true, system_note: true)
       return issue unless can?(current_user, :update_issue, issue)
 
+      close_issue(issue,
+                  commit: commit,
+                  notifications: notifications,
+                  system_note: system_note)
+    end
+
+    # Closes the supplied issue without checking if the user is authorized to
+    # do so.
+    #
+    # The code calling this method is responsible for ensuring that a user is
+    # allowed to close the given issue.
+    def close_issue(issue, commit: nil, notifications: true, system_note: true)
       if project.jira_tracker? && project.jira_service.active
         project.jira_service.execute(commit, issue)
         todo_service.close_issue(issue, current_user)
diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb
new file mode 100644
index 00000000000..071741fbacd
--- /dev/null
+++ b/app/workers/process_commit_worker.rb
@@ -0,0 +1,67 @@
+# Worker for processing individiual commit messages pushed to a repository.
+#
+# Jobs for this worker are scheduled for every commit that is being pushed. As a
+# result of this the workload of this worker should be kept to a bare minimum.
+# Consider using an extra worker if you need to add any extra (and potentially
+# slow) processing of commits.
+class ProcessCommitWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+
+  # project_id - The ID of the project this commit belongs to.
+  # user_id - The ID of the user that pushed the commit.
+  # commit_sha - The SHA1 of the commit to process.
+  # default - The data was pushed to the default branch.
+  def perform(project_id, user_id, commit_sha, default = false)
+    project = Project.find_by(id: project_id)
+
+    return unless project
+
+    user = User.find_by(id: user_id)
+
+    return unless user
+
+    commit = find_commit(project, commit_sha)
+
+    return unless commit
+
+    author = commit.author || user
+
+    process_commit_message(project, commit, user, author, default)
+
+    update_issue_metrics(commit, author)
+  end
+
+  def process_commit_message(project, commit, user, author, default = false)
+    closed_issues = default ? commit.closes_issues(user) : []
+
+    unless closed_issues.empty?
+      close_issues(project, user, author, commit, closed_issues)
+    end
+
+    commit.create_cross_references!(author, closed_issues)
+  end
+
+  def close_issues(project, user, author, commit, issues)
+    # We don't want to run permission related queries for every single issue,
+    # therefor we use IssueCollection here and skip the authorization check in
+    # Issues::CloseService#execute.
+    IssueCollection.new(issues).updatable_by_user(user).each do |issue|
+      Issues::CloseService.new(project, author).
+        close_issue(issue, commit: commit)
+    end
+  end
+
+  def update_issue_metrics(commit, author)
+    mentioned_issues = commit.all_references(author).issues
+
+    Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil).
+      update_all(first_mentioned_in_commit_at: commit.committed_date)
+  end
+
+  private
+
+  def find_commit(project, sha)
+    project.commit(sha)
+  end
+end
diff --git a/changelogs/unreleased/process-commits-using-sidekiq.yml b/changelogs/unreleased/process-commits-using-sidekiq.yml
new file mode 100644
index 00000000000..9f596e6a584
--- /dev/null
+++ b/changelogs/unreleased/process-commits-using-sidekiq.yml
@@ -0,0 +1,4 @@
+---
+title: Process commits using a dedicated Sidekiq worker
+merge_request: 6802
+author: 
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index f36fe893fd0..0aec8aedf72 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -21,6 +21,7 @@
   - [post_receive, 5]
   - [merge, 5]
   - [update_merge_requests, 3]
+  - [process_commit, 2]
   - [new_note, 2]
   - [build, 2]
   - [pipeline, 2]
diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb
index d9ab397c302..d742c814680 100644
--- a/spec/models/issue_collection_spec.rb
+++ b/spec/models/issue_collection_spec.rb
@@ -55,4 +55,13 @@ describe IssueCollection do
       end
     end
   end
+
+  describe '#visible_to' do
+    it 'is an alias for updatable_by_user' do
+      updatable_by_user = described_class.instance_method(:updatable_by_user)
+      visible_to = described_class.instance_method(:visible_to)
+
+      expect(visible_to).to eq(updatable_by_user)
+    end
+  end
 end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 1c7a3d04b09..cea7e6429f9 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -302,6 +302,9 @@ describe GitPushService, services: true do
         author_email: commit_author.email
       )
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(commit)
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
@@ -357,6 +360,9 @@ describe GitPushService, services: true do
         committed_date: commit_time
       )
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(commit)
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
@@ -393,6 +399,9 @@ describe GitPushService, services: true do
       allow(project.repository).to receive(:commits_between).
         and_return([closing_commit])
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(closing_commit)
+
       project.team << [commit_author, :master]
     end
 
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 5dfb33f4b28..4465f22a001 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -15,10 +15,39 @@ describe Issues::CloseService, services: true do
   end
 
   describe '#execute' do
+    let(:service) { described_class.new(project, user) }
+
+    it 'checks if the user is authorized to update the issue' do
+      expect(service).to receive(:can?).with(user, :update_issue, issue).
+        and_call_original
+
+      service.execute(issue)
+    end
+
+    it 'does not close the issue when the user is not authorized to do so' do
+      allow(service).to receive(:can?).with(user, :update_issue, issue).
+        and_return(false)
+
+      expect(service).not_to receive(:close_issue)
+      expect(service.execute(issue)).to eq(issue)
+    end
+
+    it 'closes the issue when the user is authorized to do so' do
+      allow(service).to receive(:can?).with(user, :update_issue, issue).
+        and_return(true)
+
+      expect(service).to receive(:close_issue).
+        with(issue, commit: nil, notifications: true, system_note: true)
+
+      service.execute(issue)
+    end
+  end
+
+  describe '#close_issue' do
     context "valid params" do
       before do
         perform_enqueued_jobs do
-          described_class.new(project, user).execute(issue)
+          described_class.new(project, user).close_issue(issue)
         end
       end
 
@@ -41,24 +70,12 @@ describe Issues::CloseService, services: true do
       end
     end
 
-    context 'current user is not authorized to close issue' do
-      before do
-        perform_enqueued_jobs do
-          described_class.new(project, guest).execute(issue)
-        end
-      end
-
-      it 'does not close the issue' do
-        expect(issue).to be_open
-      end
-    end
-
     context 'when issue is not confidential' do
       it 'executes issue hooks' do
         expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
         expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks)
 
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
     end
 
@@ -69,14 +86,14 @@ describe Issues::CloseService, services: true do
         expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
         expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
 
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
     end
 
     context 'external issue tracker' do
       before do
         allow(project).to receive(:default_issues_tracker?).and_return(false)
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
 
       it { expect(issue).to be_valid }
diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb
new file mode 100644
index 00000000000..3e4fee42240
--- /dev/null
+++ b/spec/workers/process_commit_worker_spec.rb
@@ -0,0 +1,109 @@
+require 'spec_helper'
+
+describe ProcessCommitWorker do
+  let(:worker) { described_class.new }
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public) }
+  let(:issue) { create(:issue, project: project, author: user) }
+  let(:commit) { project.commit }
+
+  describe '#perform' do
+    it 'does not process the commit when the project does not exist' do
+      expect(worker).not_to receive(:close_issues)
+
+      worker.perform(-1, user.id, commit.id)
+    end
+
+    it 'does not process the commit when the user does not exist' do
+      expect(worker).not_to receive(:close_issues)
+
+      worker.perform(project.id, -1, commit.id)
+    end
+
+    it 'does not process the commit when the commit no longer exists' do
+      expect(worker).not_to receive(:close_issues)
+
+      worker.perform(project.id, user.id, 'this-should-does-not-exist')
+    end
+
+    it 'processes the commit message' do
+      expect(worker).to receive(:process_commit_message).and_call_original
+
+      worker.perform(project.id, user.id, commit.id)
+    end
+
+    it 'updates the issue metrics' do
+      expect(worker).to receive(:update_issue_metrics).and_call_original
+
+      worker.perform(project.id, user.id, commit.id)
+    end
+  end
+
+  describe '#process_commit_message' do
+    context 'when pushing to the default branch' do
+      it 'closes issues that should be closed per the commit message' do
+        allow(commit).to receive(:safe_message).
+          and_return("Closes #{issue.to_reference}")
+
+        expect(worker).to receive(:close_issues).
+          with(project, user, user, commit, [issue])
+
+        worker.process_commit_message(project, commit, user, user, true)
+      end
+    end
+
+    context 'when pushing to a non-default branch' do
+      it 'does not close any issues' do
+        allow(commit).to receive(:safe_message).
+          and_return("Closes #{issue.to_reference}")
+
+        expect(worker).not_to receive(:close_issues)
+
+        worker.process_commit_message(project, commit, user, user, false)
+      end
+    end
+
+    it 'creates cross references' do
+      expect(commit).to receive(:create_cross_references!)
+
+      worker.process_commit_message(project, commit, user, user)
+    end
+  end
+
+  describe '#close_issues' do
+    context 'when the user can update the issues' do
+      it 'closes the issues' do
+        worker.close_issues(project, user, user, commit, [issue])
+
+        issue.reload
+
+        expect(issue.closed?).to eq(true)
+      end
+    end
+
+    context 'when the user can not update the issues' do
+      it 'does not close the issues' do
+        other_user = create(:user)
+
+        worker.close_issues(project, other_user, other_user, commit, [issue])
+
+        issue.reload
+
+        expect(issue.closed?).to eq(false)
+      end
+    end
+  end
+
+  describe '#update_issue_metrics' do
+    it 'updates any existing issue metrics' do
+      allow(commit).to receive(:safe_message).
+        and_return("Closes #{issue.to_reference}")
+
+      worker.update_issue_metrics(commit, user)
+
+      metric = Issue::Metrics.first
+
+      expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date)
+    end
+  end
+end