Read about our upcoming Code of Conduct on this issue

Commit b8c7bef5 authored by James Edwards-Jones's avatar James Edwards-Jones
Browse files

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
parent 33d6ee1478a1
......@@ -4,8 +4,7 @@ class RepositoryController < Projects::ApplicationController
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 @@ def access_levels_options
}
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
......
......@@ -865,15 +865,6 @@ def repo_exists?
@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
......
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
......@@ -702,25 +702,6 @@
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
......
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
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment