7 files changed, 88 insertions, 27 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index bd5735ed82e..2816a68257c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -212,21 +212,39 @@ class Group < Namespace
   end
 
   def user_ids_for_project_authorizations
-    users_with_parents.pluck(:id)
+    members_with_parents.pluck(:user_id)
   end
 
   def members_with_parents
-    GroupMember.active.where(source_id: ancestors.pluck(:id).push(id)).where.not(user_id: nil)
+    # Avoids an unnecessary SELECT when the group has no parents
+    source_ids =
+      if parent_id
+        self_and_ancestors.reorder(nil).select(:id)
+      else
+        id
+      end
+
+    GroupMember
+      .active_without_invites
+      .where(source_id: source_ids)
+  end
+
+  def members_with_descendants
+    GroupMember
+      .active_without_invites
+      .where(source_id: self_and_descendants.reorder(nil).select(:id))
   end
 
   def users_with_parents
-    User.where(id: members_with_parents.select(:user_id))
+    User
+      .where(id: members_with_parents.select(:user_id))
+      .reorder(nil)
   end
 
   def users_with_descendants
-    members_with_descendants = GroupMember.non_request.where(source_id: descendants.pluck(:id).push(id))
-
-    User.where(id: members_with_descendants.select(:user_id))
+    User
+      .where(id: members_with_descendants.select(:user_id))
+      .reorder(nil)
   end
 
   def max_member_access_for_user(user)
diff --git a/app/models/member.rb b/app/models/member.rb
index b26b5017183..ee2cb13697b 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -41,9 +41,20 @@ class Member < ActiveRecord::Base
     is_external_invite = arel_table[:user_id].eq(nil).and(arel_table[:invite_token].not_eq(nil))
     user_is_active = User.arel_table[:state].eq(:active)
 
-    includes(:user).references(:users)
-      .where(is_external_invite.or(user_is_active))
+    user_ok = Arel::Nodes::Grouping.new(is_external_invite).or(user_is_active)
+
+    left_join_users
+      .where(user_ok)
+      .where(requested_at: nil)
+      .reorder(nil)
+  end
+
+  # Like active, but without invites. For when a User is required.
+  scope :active_without_invites, -> do
+    left_join_users
+      .where(users: { state: 'active' })
       .where(requested_at: nil)
+      .reorder(nil)
   end
 
   scope :invite, -> { where.not(invite_token: nil) }
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index f90194041b1..ac08dc0ee1f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -443,7 +443,8 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def reload_diff_if_branch_changed
-    if source_branch_changed? || target_branch_changed?
+    if (source_branch_changed? || target_branch_changed?) &&
+        (source_branch_head && target_branch_head)
       reload_diff
     end
   end
@@ -792,11 +793,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def fetch_ref
-    target_project.repository.fetch_ref(
-      source_project.repository.path_to_repo,
-      "refs/heads/#{source_branch}",
-      ref_path
-    )
+    write_ref
     update_column(:ref_fetched, true)
   end
 
@@ -939,4 +936,17 @@ class MergeRequest < ActiveRecord::Base
 
     true
   end
+
+  private
+
+  def write_ref
+    target_project.repository.with_repo_branch_commit(
+      source_project.repository, source_branch) do |commit|
+        if commit
+          target_project.repository.write_ref(ref_path, commit.sha)
+        else
+          raise Rugged::ReferenceError, 'source repository is empty'
+        end
+      end
+  end
 end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 6073fb94a3f..e7bc1d1b080 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -156,6 +156,14 @@ class Namespace < ActiveRecord::Base
       .base_and_ancestors
   end
 
+  def self_and_ancestors
+    return self.class.where(id: id) unless parent_id
+
+    Gitlab::GroupHierarchy
+      .new(self.class.where(id: id))
+      .base_and_ancestors
+  end
+
   # Returns all the descendants of the current namespace.
   def descendants
     Gitlab::GroupHierarchy
@@ -163,6 +171,12 @@ class Namespace < ActiveRecord::Base
       .base_and_descendants
   end
 
+  def self_and_descendants
+    Gitlab::GroupHierarchy
+      .new(self.class.where(id: id))
+      .base_and_descendants
+  end
+
   def user_ids_for_project_authorizations
     [owner_id]
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 7010664e1c8..0de7da0ddaa 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -196,7 +196,6 @@ class Project < ActiveRecord::Base
   accepts_nested_attributes_for :import_data
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
-  delegate :count, to: :forks, prefix: true
   delegate :members, to: :team, prefix: true
   delegate :add_user, :add_users, to: :team
   delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
@@ -1048,9 +1047,7 @@ class Project < ActiveRecord::Base
   def change_head(branch)
     if repository.branch_exists?(branch)
       repository.before_change_head
-      repository.rugged.references.create('HEAD',
-                                          "refs/heads/#{branch}",
-                                          force: true)
+      repository.write_ref('HEAD', "refs/heads/#{branch}")
       repository.copy_gitattributes(branch)
       repository.after_change_head
       reload_default_branch
@@ -1398,6 +1395,10 @@ class Project < ActiveRecord::Base
   # @deprecated cannot remove yet because it has an index with its name in elasticsearch
   alias_method :path_with_namespace, :full_path
 
+  def forks_count
+    Projects::ForksCountService.new(self).count
+  end
+
   private
 
   def cross_namespace_reference?(from)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 049bebdbe42..a761302b06b 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -224,7 +224,7 @@ class Repository
 
     # This will still fail if the file is corrupted (e.g. 0 bytes)
     begin
-      rugged.references.create(keep_around_ref_name(sha), sha, force: true)
+      write_ref(keep_around_ref_name(sha), sha)
     rescue Rugged::ReferenceError => ex
       Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
     rescue Rugged::OSError => ex
@@ -237,6 +237,10 @@ class Repository
     ref_exists?(keep_around_ref_name(sha))
   end
 
+  def write_ref(ref_path, sha)
+    rugged.references.create(ref_path, sha, force: true)
+  end
+
   def diverging_commit_counts(branch)
     root_ref_hash = raw_repository.rev_parse_target(root_ref).oid
     cache.fetch(:"diverging_commit_counts_#{branch.name}") do
@@ -985,12 +989,10 @@ class Repository
       if start_repository == self
         start_branch_name
       else
-        tmp_ref = "refs/tmp/#{SecureRandom.hex}/head"
-
-        fetch_ref(
+        tmp_ref = fetch_ref(
           start_repository.path_to_repo,
           "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
-          tmp_ref
+          "refs/tmp/#{SecureRandom.hex}/head"
         )
 
         start_repository.commit(start_branch_name).sha
@@ -1021,7 +1023,12 @@ class Repository
 
   def fetch_ref(source_path, source_ref, target_ref)
     args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
-    run_git(args)
+    message, status = run_git(args)
+
+    # Make sure ref was created, and raise Rugged::ReferenceError when not
+    raise Rugged::ReferenceError, message if status != 0
+
+    target_ref
   end
 
   def create_ref(ref, ref_path)
diff --git a/app/models/user.rb b/app/models/user.rb
index a4615436245..2b25736bb26 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -726,9 +726,9 @@ class User < ActiveRecord::Base
   end
 
   def sanitize_attrs
-    %w[username skype linkedin twitter].each do |attr|
-      value = public_send(attr) # rubocop:disable GitlabSecurity/PublicSend
-      public_send("#{attr}=", Sanitize.clean(value)) if value.present? # rubocop:disable GitlabSecurity/PublicSend
+    %i[skype linkedin twitter].each do |attr|
+      value = self[attr]
+      self[attr] = Sanitize.clean(value) if value.present?
     end
   end