Have `Project#open_branches` return branches that are matched by a wildcard protected branch.

1. The `open_branches` method is used to provide a list of branches while creating a protected branch. 2. It makes sense to include branches which are matched by one or more wildcard protected branches, since the user might want to make exact protected branches from these as well. 3. This also provides a large performance improvement. On my machine, in a project with 5000 branches and 2000 protected branches, the `ProtectedBranches#index` page went from a 40 seconds load time to 4 seconds (10x speedup).

Have `Project#open_branches` return branches that are matched by a wildcard protected branch.
b1c81f84 · Timothy Andrew · d8d5424d · b1c81f84 · b1c81f84 · b1c81f84
Commit b1c81f84 authored Jul 06, 2016 by Timothy Andrew
Hide whitespace changes
Inline Side-by-side

Showing with 8 additions and 4 deletions

project.rb app/models/project.rb +5 -1

protected_branch.rb app/models/protected_branch.rb +1 -1

project_spec.rb spec/models/project_spec.rb +2 -2

No files found.
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -802,8 +802,12 @@ class Project < ActiveRecord::Base
    @repo_exists = false
  end

+  # Branches that are not _exactly_ matched by a protected branch.
  def open_branches
-    repository.branches.reject { |branch| self.protected_branch?(branch.name) }
+    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)

--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -34,7 +34,7 @@ class ProtectedBranch < ActiveRecord::Base

  # Checks if this protected branch contains a wildcard
  def wildcard?
-    self.name.include?('*')
+    self.name && self.name.include?('*')
  end

  protected

--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -438,12 +438,12 @@ describe Project, models: true do
    it { expect(project.open_branches.map(&:name)).to include('feature') }
    it { expect(project.open_branches.map(&:name)).not_to include('master') }

-    it "does not include branches matching a protected branch wildcard" do
+    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)).not_to include('feature')
+      expect(Project.find(project.id).open_branches.map(&:name)).to include('feature')
    end
  end