Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add max length option to BranchNamer #5338

Merged
merged 1 commit into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions common/lib/dependabot/pull_request_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def initialize(cause, pull_request)
:custom_labels, :author_details, :signature_key,
:commit_message_options, :vulnerabilities_fixed,
:reviewers, :assignees, :milestone, :branch_name_separator,
:branch_name_prefix, :github_redirection_service,
:branch_name_prefix, :branch_name_max_length, :github_redirection_service,
:custom_headers, :provider_metadata

def initialize(source:, base_commit:, dependencies:, files:, credentials:,
Expand All @@ -57,7 +57,8 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:,
commit_message_options: {}, vulnerabilities_fixed: {},
reviewers: nil, assignees: nil, milestone: nil,
branch_name_separator: "/", branch_name_prefix: "dependabot",
label_language: false, automerge_candidate: false,
branch_name_max_length: nil, label_language: false,
automerge_candidate: false,
github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE,
custom_headers: nil, require_up_to_date_base: false,
provider_metadata: {}, message: nil)
Expand All @@ -78,6 +79,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:,
@vulnerabilities_fixed = vulnerabilities_fixed
@branch_name_separator = branch_name_separator
@branch_name_prefix = branch_name_prefix
@branch_name_max_length = branch_name_max_length
@label_language = label_language
@automerge_candidate = automerge_candidate
@github_redirection_service = github_redirection_service
Expand Down Expand Up @@ -232,7 +234,8 @@ def branch_namer
files: files,
target_branch: source.branch,
separator: branch_name_separator,
prefix: branch_name_prefix
prefix: branch_name_prefix,
max_length: branch_name_max_length
)
end

Expand Down
19 changes: 15 additions & 4 deletions common/lib/dependabot/pull_request_creator/branch_namer.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
# frozen_string_literal: true

require "digest"

require "dependabot/metadata_finders"
require "dependabot/pull_request_creator"

module Dependabot
class PullRequestCreator
class BranchNamer
attr_reader :dependencies, :files, :target_branch, :separator, :prefix
attr_reader :dependencies, :files, :target_branch, :separator, :prefix, :max_length

def initialize(dependencies:, files:, target_branch:, separator: "/",
prefix: "dependabot")
prefix: "dependabot", max_length: nil)
@dependencies = dependencies
@files = files
@target_branch = target_branch
@separator = separator
@prefix = prefix
@max_length = max_length
end

def new_branch_name
Expand All @@ -37,7 +40,15 @@ def new_branch_name
end

# Some users need branch names without slashes
sanitize_ref(File.join(prefixes, @name).gsub("/", separator))
sanitized_name = sanitize_ref(File.join(prefixes, @name).gsub("/", separator))

# Shorten the ref in case users refs have length limits
if @max_length && (sanitized_name.length > @max_length)
sha = Digest::SHA1.hexdigest(sanitized_name)[0, @max_length]
sanitized_name[[@max_length - sha.size, 0].max..] = sha
end

sanitized_name
end

private
Expand Down Expand Up @@ -181,7 +192,7 @@ def requirements_changed?(dependency)

def sanitize_ref(ref)
# This isn't a complete implementation of git's ref validation, but it
# covers most cases that crop up. Its list of allowed charactersr is a
# covers most cases that crop up. Its list of allowed characters is a
# bit stricter than git's, but that's for cosmetic reasons.
ref.
# Remove forbidden characters (those not already replaced elsewhere)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,49 @@
it { is_expected.to eq("dependabot-dummy-business-1.5.0") }
end

context "with a maximum length" do
let(:namer) do
described_class.new(
dependencies: dependencies,
files: files,
target_branch: target_branch,
max_length: max_length
)
end

context "with a maximum length longer than branch name" do
let(:max_length) { 35 }

it { is_expected.to eq("dependabot/dummy/business-1.5.0") }
its(:length) { is_expected.to eq(31) }
end

context "with a maximum length shorter than branch name" do
let(:dependency_name) { "business-and-work-and-desks-and-tables-and-chairs-and-lunch" }

context "with a maximum length longer than sha1 length" do
let(:max_length) { 50 }

it { is_expected.to eq("dependabot#{Digest::SHA1.hexdigest("dependabot/dummy/#{dependency_name}-1.5.0")}") }
its(:length) { is_expected.to eq(50) }
end

context "with a maximum length equal than sha1 length" do
let(:max_length) { 40 }

it { is_expected.to eq(Digest::SHA1.hexdigest("dependabot/dummy/#{dependency_name}-1.5.0")) }
its(:length) { is_expected.to eq(40) }
end

context "with a maximum length shorter than sha1 length" do
let(:max_length) { 20 }

it { is_expected.to eq(Digest::SHA1.hexdigest("dependabot/dummy/#{dependency_name}-1.5.0")[0...20]) }
its(:length) { is_expected.to eq(20) }
end
end
end

context "with multiple dependencies" do
let(:dependencies) { [dependency, dep2] }
let(:dep2) do
Expand Down