f51af49676
1. The main implementation is in the `ProtectedBranch` model. The
wildcard is converted to a Regex and compared. This has been tested
thoroughly.
- While `Project#protected_branch?` is the main entry point,
`project#open_branches` and
`project#developers_can_push_to_protected_branch?`
have also been modified to work with wildcard protected branches.
- The regex is memoized (within the `ProtectedBranch` instance)
2. Improve the performance of `Project#protected_branch?`
- This method is called from `Project#open_branches` once _per branch_
in the project, to check if that branch is protected or not.
- Before, `#protected_branch?` was making a database call every
time it was invoked (in the above case, that amounts to once
per branch), which is expensive.
- This commit caches the list of protected branches in memory, which
reduces the number of database calls down to 1.
- A downside to this approach is that `#protected_branch?` _could_
return a stale value (due to the caching), but this is
an acceptable tradeoff.
3. Remove the (now) unused `Project#protected_branch_names` method.
- This was previously used to check for protected branch status.
140 lines
6.5 KiB
Ruby
140 lines
6.5 KiB
Ruby
require 'spec_helper'
|
|
|
|
describe ProtectedBranch, models: true do
|
|
subject { build_stubbed(:protected_branch) }
|
|
|
|
describe 'Associations' do
|
|
it { is_expected.to belong_to(:project) }
|
|
end
|
|
|
|
describe "Mass assignment" do
|
|
end
|
|
|
|
describe 'Validation' do
|
|
it { is_expected.to validate_presence_of(:project) }
|
|
it { is_expected.to validate_presence_of(:name) }
|
|
end
|
|
|
|
describe "#matches?" do
|
|
context "when the protected branch setting is not a wildcard" do
|
|
let(:protected_branch) { build(:protected_branch, name: "production/some-branch") }
|
|
|
|
it "returns true for branch names that are an exact match" do
|
|
expect(protected_branch.matches?("production/some-branch")).to be true
|
|
end
|
|
|
|
it "returns false for branch names that are not an exact match" do
|
|
expect(protected_branch.matches?("staging/some-branch")).to be false
|
|
end
|
|
end
|
|
|
|
context "when the protected branch name contains wildcard(s)" do
|
|
context "when there is a single '*'" do
|
|
let(:protected_branch) { build(:protected_branch, name: "production/*") }
|
|
|
|
it "returns true for branch names matching the wildcard" do
|
|
expect(protected_branch.matches?("production/some-branch")).to be true
|
|
expect(protected_branch.matches?("production/")).to be true
|
|
end
|
|
|
|
it "returns false for branch names not matching the wildcard" do
|
|
expect(protected_branch.matches?("staging/some-branch")).to be false
|
|
expect(protected_branch.matches?("production")).to be false
|
|
end
|
|
end
|
|
|
|
context "when the wildcard contains regex symbols other than a '*'" do
|
|
let(:protected_branch) { build(:protected_branch, name: "pro.duc.tion/*") }
|
|
|
|
it "returns true for branch names matching the wildcard" do
|
|
expect(protected_branch.matches?("pro.duc.tion/some-branch")).to be true
|
|
end
|
|
|
|
it "returns false for branch names not matching the wildcard" do
|
|
expect(protected_branch.matches?("production/some-branch")).to be false
|
|
expect(protected_branch.matches?("proXducYtion/some-branch")).to be false
|
|
end
|
|
end
|
|
|
|
context "when there are '*'s at either end" do
|
|
let(:protected_branch) { build(:protected_branch, name: "*/production/*") }
|
|
|
|
it "returns true for branch names matching the wildcard" do
|
|
expect(protected_branch.matches?("gitlab/production/some-branch")).to be true
|
|
expect(protected_branch.matches?("/production/some-branch")).to be true
|
|
expect(protected_branch.matches?("gitlab/production/")).to be true
|
|
expect(protected_branch.matches?("/production/")).to be true
|
|
end
|
|
|
|
it "returns false for branch names not matching the wildcard" do
|
|
expect(protected_branch.matches?("gitlabproductionsome-branch")).to be false
|
|
expect(protected_branch.matches?("production/some-branch")).to be false
|
|
expect(protected_branch.matches?("gitlab/production")).to be false
|
|
expect(protected_branch.matches?("production")).to be false
|
|
end
|
|
end
|
|
|
|
context "when there are arbitrarily placed '*'s" do
|
|
let(:protected_branch) { build(:protected_branch, name: "pro*duction/*/gitlab/*") }
|
|
|
|
it "returns true for branch names matching the wildcard" do
|
|
expect(protected_branch.matches?("production/some-branch/gitlab/second-branch")).to be true
|
|
expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/second-branch")).to be true
|
|
expect(protected_branch.matches?("proXYZduction/gitlab/gitlab/gitlab")).to be true
|
|
expect(protected_branch.matches?("proXYZduction//gitlab/")).to be true
|
|
expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/")).to be true
|
|
expect(protected_branch.matches?("proXYZduction//gitlab/some-branch")).to be true
|
|
end
|
|
|
|
it "returns false for branch names not matching the wildcard" do
|
|
expect(protected_branch.matches?("production/some-branch/not-gitlab/second-branch")).to be false
|
|
expect(protected_branch.matches?("prodXYZuction/some-branch/gitlab/second-branch")).to be false
|
|
expect(protected_branch.matches?("proXYZduction/gitlab/some-branch/gitlab")).to be false
|
|
expect(protected_branch.matches?("proXYZduction/gitlab//")).to be false
|
|
expect(protected_branch.matches?("proXYZduction/gitlab/")).to be false
|
|
expect(protected_branch.matches?("proXYZduction//some-branch/gitlab")).to be false
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#matching" do
|
|
context "for direct matches" do
|
|
it "returns a list of protected branches matching the given branch name" do
|
|
production = create(:protected_branch, name: "production")
|
|
staging = create(:protected_branch, name: "staging")
|
|
|
|
expect(ProtectedBranch.matching("production")).to include(production)
|
|
expect(ProtectedBranch.matching("production")).not_to include(staging)
|
|
end
|
|
|
|
it "accepts a list of protected branches to search from, so as to avoid a DB call" do
|
|
production = build(:protected_branch, name: "production")
|
|
staging = build(:protected_branch, name: "staging")
|
|
|
|
expect(ProtectedBranch.matching("production")).to be_empty
|
|
expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).to include(production)
|
|
expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).not_to include(staging)
|
|
end
|
|
end
|
|
|
|
context "for wildcard matches" do
|
|
it "returns a list of protected branches matching the given branch name" do
|
|
production = create(:protected_branch, name: "production/*")
|
|
staging = create(:protected_branch, name: "staging/*")
|
|
|
|
expect(ProtectedBranch.matching("production/some-branch")).to include(production)
|
|
expect(ProtectedBranch.matching("production/some-branch")).not_to include(staging)
|
|
end
|
|
|
|
it "accepts a list of protected branches to search from, so as to avoid a DB call" do
|
|
production = build(:protected_branch, name: "production/*")
|
|
staging = build(:protected_branch, name: "staging/*")
|
|
|
|
expect(ProtectedBranch.matching("production/some-branch")).to be_empty
|
|
expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).to include(production)
|
|
expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).not_to include(staging)
|
|
end
|
|
end
|
|
end
|
|
end
|