From 6ba5296f550ade045187a1bd8425912faed8009b Mon Sep 17 00:00:00 2001 From: Tom Naessens Date: Thu, 30 Jun 2022 21:23:35 +0200 Subject: [PATCH] Add max length option to BranchNamer --- common/lib/dependabot/pull_request_creator.rb | 9 ++-- .../pull_request_creator/branch_namer.rb | 19 ++++++-- .../pull_request_creator/branch_namer_spec.rb | 43 +++++++++++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index 0adad23857..895745e6c4 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -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:, @@ -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) @@ -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 @@ -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 diff --git a/common/lib/dependabot/pull_request_creator/branch_namer.rb b/common/lib/dependabot/pull_request_creator/branch_namer.rb index 3f97fff123..a494f1d56c 100644 --- a/common/lib/dependabot/pull_request_creator/branch_namer.rb +++ b/common/lib/dependabot/pull_request_creator/branch_namer.rb @@ -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 @@ -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 @@ -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) diff --git a/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb b/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb index d3797ca2e2..3b6d8b1170 100644 --- a/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb +++ b/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb @@ -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