Merge branch 'check-protected-branches' into 'master'

Cleaned up/tweaked Project#open_branches

See commit c825404572040a3a45cb9e2b3d3e7d64900f66c9 for the details of the changes and https://gitlab.com/gitlab-org/gitlab-ce/issues/14280#note_4973648 for more information.

See merge request !3985

CHANGELOG

@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.8.0 (unreleased)
+  - Project#open_branches has been cleaned up and no longer loads entire records into memory.
   - Make build status canceled if any of the jobs was canceled and none failed
   - Remove future dates from contribution calendar graph.
   - Use ActionDispatch Remote IP for Akismet checking

app/models/project.rb

@@ -735,19 +735,17 @@ class Project < ActiveRecord::Base
   end
 
   def open_branches
-    all_branches = repository.branches
+    # We're using a Set here as checking values in a large Set is faster than
+    # checking values in a large Array.
+    protected_set = Set.new(protected_branch_names)
 
-    if protected_branches.present?
+    repository.branches.reject do |branch|
-      all_branches.reject! do |branch|
+      protected_set.include?(branch.name)
-        protected_branches_names.include?(branch.name)
     end
   end
 
-    all_branches
+  def protected_branch_names
-  end
+    @protected_branch_names ||= protected_branches.pluck(:name)
-
-  def protected_branches_names
-    @protected_branches_names ||= protected_branches.map(&:name)
   end
 
   def root_ref?(branch)
@@ -764,7 +762,7 @@ class Project < ActiveRecord::Base
 
   # Check if current branch name is marked as protected in the system
   def protected_branch?(branch_name)
-    protected_branches_names.include?(branch_name)
+    protected_branches.where(name: branch_name).any?
   end
 
   def developers_can_push_to_protected_branch?(branch_name)

spec/models/project_spec.rb

@@ -798,4 +798,18 @@ describe Project, models: true do
       end
     end
   end
+
+  describe '#protected_branch?' do
+    let(:project) { create(:empty_project) }
+
+    it 'returns true when a branch is a protected branch' do
+      project.protected_branches.create!(name: 'foo')
+
+      expect(project.protected_branch?('foo')).to eq(true)
+    end
+
+    it 'returns false when a branch is not a protected branch' do
+      expect(project.protected_branch?('foo')).to eq(false)
+    end
+  end
 end