Extracted ProtectableDropdown to clean up Project#open_branches

Makes it clear this is only used in dropdowns, instead of cluttering up Project class. Since we only care about branch names, it is also possible to refactor out a lot of the set/reject logic.

A benchmark on Array/Set subtraction favoured using Arrays. This was with 5000 ‘branches’ and 2000 ‘protections’ to ensure a similar comparison to the commit which introduced using Set for intersection.

Comparison:
   array subtraction:      485.8 i/s
     set subtraction:      128.7 i/s - 3.78x slower

5 files changed, 57 insertions, 46 deletions

diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb
index 5160ee5e1e4..a87927fe1ec 100644
--- a/app/controllers/projects/settings/repository_controller.rb
+++ b/app/controllers/projects/settings/repository_controller.rb
@@ -4,8 +4,7 @@ module Projects
       before_action :authorize_admin_project!
 
       def show
-        @deploy_keys = DeployKeysPresenter
-          .new(@project, current_user: current_user)
+        @deploy_keys = DeployKeysPresenter.new(@project, current_user: current_user)
 
         define_protected_refs
       end
@@ -38,27 +37,17 @@ module Projects
         }
       end
 
-      #TODO: Move to Protections::TagMatcher.new(project).unprotected
-      def unprotected_tags
-        exact_protected_tag_names = @project.protected_tags.reject(&:wildcard?).map(&:name)
-        tag_names = @project.repository.tags.map(&:name)
-        non_open_tag_names = Set.new(exact_protected_tag_names).intersection(Set.new(tag_names))
-        @project.repository.tags.reject { |tag| non_open_tag_names.include? tag.name }
+      def protectable_tags_for_dropdown
+        { open_tags: ProtectableDropdown.new(@project, :tags).hash }
       end
 
-      def unprotected_tags_hash
-        tags = unprotected_tags.map { |tag| { text: tag.name, id: tag.name, title: tag.name } }
-        { open_tags: tags }
-      end
-
-      def open_branches
-        branches = @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } }
-        { open_branches: branches }
+      def protectable_branches_for_dropdown
+        { open_branches: ProtectableDropdown.new(@project, :branches).hash }
       end
 
       def load_gon_index
-        gon.push(open_branches)
-        gon.push(unprotected_tags_hash)
+        gon.push(protectable_tags_for_dropdown)
+        gon.push(protectable_branches_for_dropdown)
         gon.push(access_levels_options)
       end
     end
diff --git a/app/models/project.rb b/app/models/project.rb
index 970de324a5b..4175bfab0a9 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -865,15 +865,6 @@ class Project < ActiveRecord::Base
     @repo_exists = false
   end
 
-  # Branches that are not _exactly_ matched by a protected branch.
-  #TODO: Move to Protections::BranchMatcher.new(project).unprotecte
-  def open_branches
-    exact_protected_branch_names = protected_branches.reject(&:wildcard?).map(&:name)
-    branch_names = repository.branches.map(&:name)
-    non_open_branch_names = Set.new(exact_protected_branch_names).intersection(Set.new(branch_names))
-    repository.branches.reject { |branch| non_open_branch_names.include? branch.name }
-  end
-
   def root_ref?(branch)
     repository.root_ref == branch
   end
diff --git a/app/models/protectable_dropdown.rb b/app/models/protectable_dropdown.rb
new file mode 100644
index 00000000000..c9b2b213cd2
--- /dev/null
+++ b/app/models/protectable_dropdown.rb
@@ -0,0 +1,26 @@
+class ProtectableDropdown
+  def initialize(project, ref_type)
+    @project = project
+    @ref_type = ref_type
+  end
+
+  # Tags/branches which are yet to be individually protected
+  def protectable_ref_names
+    non_wildcard_protections = protections.reject(&:wildcard?)
+    refs.map(&:name) - non_wildcard_protections.map(&:name)
+  end
+
+  def hash
+    protectable_ref_names.map { |ref_name| { text: ref_name, id: ref_name, title: ref_name } }
+  end
+
+  private
+
+  def refs
+    @project.repository.public_send(@ref_type)
+  end
+
+  def protections
+    @project.public_send("protected_#{@ref_type}")
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f68631ebe06..cc06949974e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -702,25 +702,6 @@ describe Project, models: true do
     end
   end
 
-  describe '#open_branches' do
-    let(:project) { create(:project, :repository) }
-
-    before do
-      project.protected_branches.create(name: 'master')
-    end
-
-    it { expect(project.open_branches.map(&:name)).to include('feature') }
-    it { expect(project.open_branches.map(&:name)).not_to include('master') }
-
-    it "includes branches matching a protected branch wildcard" do
-      expect(project.open_branches.map(&:name)).to include('feature')
-
-      create(:protected_branch, name: 'feat*', project: project)
-
-      expect(Project.find(project.id).open_branches.map(&:name)).to include('feature')
-    end
-  end
-
   describe '#star_count' do
     it 'counts stars from multiple users' do
       user1 = create :user
diff --git a/spec/models/protectable_dropdown_spec.rb b/spec/models/protectable_dropdown_spec.rb
new file mode 100644
index 00000000000..7f8ef7195e5
--- /dev/null
+++ b/spec/models/protectable_dropdown_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe ProtectableDropdown, models: true do
+  let(:project) { create(:project, :repository) }
+  let(:subject) { described_class.new(project, :branches) }
+
+  describe '#protectable_ref_names' do
+    before do
+      project.protected_branches.create(name: 'master')
+    end
+
+    it { expect(subject.protectable_ref_names).to include('feature') }
+    it { expect(subject.protectable_ref_names).not_to include('master') }
+
+    it "includes branches matching a protected branch wildcard" do
+      expect(subject.protectable_ref_names).to include('feature')
+
+      create(:protected_branch, name: 'feat*', project: project)
+
+      subject = described_class.new(project.reload, :branches)
+      expect(subject.protectable_ref_names).to include('feature')
+    end
+  end
+end